From f4485b5e02d1829fa3e0a1b3260a631b57e06826 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Mon, 29 Oct 2012 11:13:55 +0530 Subject: [PATCH] Windows: Do not change title/author if book files are in use, to prevent (a harmless) mismatch between title/author anf on disk path --- src/calibre/gui2/library/models.py | 10 +++- src/calibre/gui2/metadata/basic_widgets.py | 66 ++++++++++++---------- src/calibre/gui2/metadata/single.py | 10 ++-- src/calibre/library/database2.py | 27 ++++++++- 4 files changed, 75 insertions(+), 38 deletions(-) diff --git a/src/calibre/gui2/library/models.py b/src/calibre/gui2/library/models.py index 641ac23611..49e5c497fe 100644 --- a/src/calibre/gui2/library/models.py +++ b/src/calibre/gui2/library/models.py @@ -871,12 +871,18 @@ class BooksModel(QAbstractTableModel): # {{{ try: return self._set_data(index, value) except (IOError, OSError) as err: + import traceback if getattr(err, 'errno', None) == errno.EACCES: # Permission denied - import traceback + fname = getattr(err, 'filename', None) + p = 'Locked file: %s\n\n'%fname if fname else '' error_dialog(get_gui(), _('Permission denied'), _('Could not change the on disk location of this' ' book. Is it open in another program?'), - det_msg=traceback.format_exc(), show=True) + det_msg=p+traceback.format_exc(), show=True) + return False + error_dialog(get_gui(), _('Failed to set data'), + _('Could not set data, click Show Details to see why.'), + det_msg=traceback.format_exc(), show=True) except: import traceback traceback.print_exc() diff --git a/src/calibre/gui2/metadata/basic_widgets.py b/src/calibre/gui2/metadata/basic_widgets.py index 0da9b1bcf4..409088d315 100644 --- a/src/calibre/gui2/metadata/basic_widgets.py +++ b/src/calibre/gui2/metadata/basic_widgets.py @@ -91,22 +91,26 @@ class TitleEdit(EnLineEdit): def commit(self, db, id_): title = self.current_val - try: - if self.COMMIT: - getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False) - else: - getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False, - commit=False) - except (IOError, OSError) as err: - if getattr(err, 'errno', -1) == errno.EACCES: # Permission denied - import traceback - fname = err.filename if err.filename else 'file' - error_dialog(self, _('Permission denied'), - _('Could not open %s. Is it being used by another' - ' program?')%fname, det_msg=traceback.format_exc(), - show=True) - return False - raise + if title != self.original_val: + # Only try to commit if changed. This allow setting of other fields + # to work even if some of the book files are opened in windows. + try: + if self.COMMIT: + getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False) + else: + getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False, + commit=False) + except (IOError, OSError) as err: + if getattr(err, 'errno', None) == errno.EACCES: # Permission denied + import traceback + fname = getattr(err, 'filename', None) + p = 'Locked file: %s\n\n'%fname if fname else '' + error_dialog(self, _('Permission denied'), + _('Could not change the on disk location of this' + ' book. Is it open in another program?'), + det_msg=p+traceback.format_exc(), show=True) + return False + raise return True @dynamic_property @@ -262,19 +266,23 @@ class AuthorsEdit(EditWithComplete): def commit(self, db, id_): authors = self.current_val - try: - self.books_to_refresh |= db.set_authors(id_, authors, notify=False, - allow_case_change=True) - except (IOError, OSError) as err: - if getattr(err, 'errno', -1) == errno.EACCES: # Permission denied - import traceback - fname = err.filename if err.filename else 'file' - error_dialog(self, _('Permission denied'), - _('Could not open "%s". Is it being used by another' - ' program?')%fname, det_msg=traceback.format_exc(), - show=True) - return False - raise + if authors != self.original_val: + # Only try to commit if changed. This allow setting of other fields + # to work even if some of the book files are opened in windows. + try: + self.books_to_refresh |= db.set_authors(id_, authors, notify=False, + allow_case_change=True) + except (IOError, OSError) as err: + if getattr(err, 'errno', None) == errno.EACCES: # Permission denied + import traceback + fname = getattr(err, 'filename', None) + p = 'Locked file: %s\n\n'%fname if fname else '' + error_dialog(self, _('Permission denied'), + _('Could not change the on disk location of this' + ' book. Is it open in another program?'), + det_msg=p+traceback.format_exc(), show=True) + return False + raise return True @dynamic_property diff --git a/src/calibre/gui2/metadata/single.py b/src/calibre/gui2/metadata/single.py index 54067f4c0f..af56e2e657 100644 --- a/src/calibre/gui2/metadata/single.py +++ b/src/calibre/gui2/metadata/single.py @@ -322,6 +322,7 @@ class MetadataSingleDialogBase(ResizableDialog): ' program?')%fname, det_msg=traceback.format_exc(), show=True) return + raise if mi is None: return cdata = None @@ -444,11 +445,12 @@ class MetadataSingleDialogBase(ResizableDialog): except (IOError, OSError) as err: if getattr(err, 'errno', None) == errno.EACCES: # Permission denied import traceback - fname = err.filename if err.filename else 'file' + fname = getattr(err, 'filename', None) + p = 'Locked file: %s\n\n'%fname if fname else '' error_dialog(self, _('Permission denied'), - _('Could not open %s. Is it being used by another' - ' program?')%fname, det_msg=traceback.format_exc(), - show=True) + _('Could not change the on disk location of this' + ' book. Is it open in another program?'), + det_msg=p+traceback.format_exc(), show=True) return False raise for widget in getattr(self, 'custom_metadata_widgets', []): diff --git a/src/calibre/library/database2.py b/src/calibre/library/database2.py index d4cdf8cc1f..5952e11e57 100644 --- a/src/calibre/library/database2.py +++ b/src/calibre/library/database2.py @@ -2205,13 +2205,14 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns): def set(self, row, column, val, allow_case_change=False): ''' - Convenience method for setting the title, authors, publisher or rating + Convenience method for setting the title, authors, publisher, tags or + rating ''' id = self.data[row][0] - col = {'title':1, 'authors':2, 'publisher':3, 'rating':4, 'tags':7}[column] + col = self.FIELD_MAP[column] books_to_refresh = set() - self.data.set(row, col, val) + set_args = (row, col, val) if column == 'authors': val = string_to_authors(val) books_to_refresh |= self.set_authors(id, val, notify=False, @@ -2227,6 +2228,7 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns): books_to_refresh |= \ self.set_tags(id, [x.strip() for x in val.split(',') if x.strip()], append=False, notify=False, allow_case_change=allow_case_change) + self.data.set(*set_args) self.data.refresh_ids(self, [id]) self.set_path(id, True) self.notify('metadata', [id]) @@ -2474,6 +2476,23 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns): self.clean_standard_field('authors', commit=True) return books_to_refresh + def windows_check_if_files_in_use(self, book_id): + ''' + Raises an EACCES IOError if any of the files in the folder of book_id + are opened in another program on windows. + ''' + if iswindows: + path = self.path(book_id, index_is_id=True) + if path: + spath = os.path.join(self.library_path, *path.split('/')) + wam = None + if os.path.exists(spath): + try: + wam = WindowsAtomicFolderMove(spath) + finally: + if wam is not None: + wam.close_handles() + def set_authors(self, id, authors, notify=True, commit=True, allow_case_change=False): ''' @@ -2482,6 +2501,7 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns): :param authors: A list of authors. ''' + self.windows_check_if_files_in_use(id) books_to_refresh = self._set_authors(id, authors, allow_case_change=allow_case_change) self.dirtied(set([id])|books_to_refresh, commit=False) @@ -2532,6 +2552,7 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns): Note that even if commit is False, the db will still be committed to because this causes the location of files to change ''' + self.windows_check_if_files_in_use(id) if not self._set_title(id, title): return self.set_path(id, index_is_id=True)