From 056220d2a71dff89900d0c553104e472cade05ba Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Thu, 15 Jun 2023 16:12:33 +0530 Subject: [PATCH] Windows: Nicer error message when file/folder is locked in another program --- src/calibre/db/backend.py | 15 ++-- src/calibre/gui2/__init__.py | 15 ++-- src/calibre/gui2/actions/catalog.py | 10 +-- src/calibre/gui2/actions/delete.py | 10 +-- src/calibre/gui2/convert/metadata.py | 11 +-- src/calibre/gui2/library/models.py | 27 ++++--- src/calibre/gui2/main_window.py | 82 +++++++++++++++++++++- src/calibre/gui2/metadata/basic_widgets.py | 16 +---- src/calibre/gui2/metadata/single.py | 33 +++------ src/calibre/utils/copy_files.py | 14 ++-- 10 files changed, 133 insertions(+), 100 deletions(-) diff --git a/src/calibre/db/backend.py b/src/calibre/db/backend.py index ff6380fce8..38e61ab47e 100644 --- a/src/calibre/db/backend.py +++ b/src/calibre/db/backend.py @@ -41,7 +41,8 @@ from calibre.ptempfile import PersistentTemporaryFile, TemporaryFile from calibre.utils import pickle_binary_string, unpickle_binary_string from calibre.utils.config import from_json, prefs, to_json, tweaks from calibre.utils.copy_files import ( - copy_files, copy_tree, rename_files, windows_check_if_files_in_use, + copy_files, copy_tree, rename_files, + windows_check_if_files_in_use, ) from calibre.utils.date import EPOCH, parse_date, utcfromtimestamp, utcnow from calibre.utils.filenames import ( @@ -414,11 +415,10 @@ def rmtree_with_retry(path, sleep_time=1): try: shutil.rmtree(path) except OSError as e: - if not iswindows: - raise if e.errno == errno.ENOENT and not os.path.exists(path): return - time.sleep(sleep_time) # In case something has temporarily locked a file + if iswindows: + time.sleep(sleep_time) # In case something has temporarily locked a file shutil.rmtree(path) @@ -1577,12 +1577,7 @@ class DB: except OSError: if iswindows: time.sleep(0.2) - try: - f = open(path, 'rb') - except OSError as e: - # Ensure the path that caused this error is reported - raise Exception(f'Failed to open {path!r} with error: {e}') - + f = open(path, 'rb') with f: if hasattr(dest, 'write'): if report_file_size is not None: diff --git a/src/calibre/gui2/__init__.py b/src/calibre/gui2/__init__.py index 89754a0245..1201565548 100644 --- a/src/calibre/gui2/__init__.py +++ b/src/calibre/gui2/__init__.py @@ -1099,6 +1099,7 @@ class Application(QApplication): if not args: args = sys.argv[:1] args = [args[0]] + sys.excepthook = simple_excepthook QNetworkProxyFactory.setUseSystemConfiguration(True) setup_to_run_webengine() if iswindows: @@ -1474,6 +1475,9 @@ def open_local_file(path): _ea_lock = Lock() +def simple_excepthook(t, v, tb): + return sys.__excepthook__(t, v, tb) + def ensure_app(headless=True): global _store_app @@ -1492,7 +1496,6 @@ def ensure_app(headless=True): set_image_allocation_limit() if headless and has_headless: _store_app.headless = True - import traceback # This is needed because as of PyQt 5.4 if sys.execpthook == # sys.__excepthook__ PyQt will abort the application on an @@ -1501,14 +1504,8 @@ def ensure_app(headless=True): # or running a headless browser, we circumvent this as I really # dont feel like going through all the code and making sure no # unhandled exceptions ever occur. All the actual GUI apps already - # override sys.except_hook with a proper error handler. - - def eh(t, v, tb): - try: - traceback.print_exception(t, v, tb, file=sys.stderr) - except: - pass - sys.excepthook = eh + # override sys.excepthook with a proper error handler. + sys.excepthook = simple_excepthook return _store_app diff --git a/src/calibre/gui2/actions/catalog.py b/src/calibre/gui2/actions/catalog.py index 499faf1a73..ba147c8039 100644 --- a/src/calibre/gui2/actions/catalog.py +++ b/src/calibre/gui2/actions/catalog.py @@ -5,7 +5,7 @@ __license__ = 'GPL v3' __copyright__ = '2010, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import re, os, shutil, errno +import re, os, shutil from qt.core import QModelIndex @@ -99,11 +99,5 @@ class GenerateCatalogAction(InterfaceAction): try: shutil.copyfile(job.catalog_file_path, destination) except OSError as err: - if getattr(err, 'errno', None) == errno.EACCES: # Permission denied - import traceback - error_dialog(self.gui, _('Permission denied'), - _('Could not open %s. Is it being used by another' - ' program?')%destination, det_msg=traceback.format_exc(), - show=True) - return + err.locking_violation_msg = _('Could not open the catalog output file.') raise diff --git a/src/calibre/gui2/actions/delete.py b/src/calibre/gui2/actions/delete.py index 724e6bf585..52af7717a5 100644 --- a/src/calibre/gui2/actions/delete.py +++ b/src/calibre/gui2/actions/delete.py @@ -5,8 +5,6 @@ __license__ = 'GPL v3' __copyright__ = '2010, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import errno -import os from collections import Counter from functools import partial from qt.core import QDialog, QModelIndex, QObject, QTimer @@ -458,13 +456,7 @@ class DeleteAction(InterfaceAction): try: view.model().delete_books_by_id(to_delete_ids) except OSError as err: - if err.errno == errno.EACCES: - import traceback - fname = os.path.basename(getattr(err, 'filename', 'file') or 'file') - return error_dialog(self.gui, _('Permission denied'), - _('Could not access %s. Is it being used by another' - ' program? Click "Show details" for more information.')%fname, det_msg=traceback.format_exc(), - show=True) + err.locking_violation_msg = _('Could not change on-disk location of this book\'s files.') raise self.library_ids_deleted2(to_delete_ids, next_id=next_id, can_undo=True) else: diff --git a/src/calibre/gui2/convert/metadata.py b/src/calibre/gui2/convert/metadata.py index 8de7926d68..dd7caf624f 100644 --- a/src/calibre/gui2/convert/metadata.py +++ b/src/calibre/gui2/convert/metadata.py @@ -5,7 +5,7 @@ __license__ = 'GPL v3' __copyright__ = '2009, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import os, re, errno +import os, re from qt.core import QPixmap, QApplication @@ -243,13 +243,8 @@ class MetadataWidget(Widget, Ui_Form): if self.cover_changed and self.cover_data is not None: self.db.set_cover(self.book_id, self.cover_data) except OSError as err: - if getattr(err, 'errno', None) == errno.EACCES: # Permission denied - import traceback - fname = getattr(err, 'filename', None) or '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 + err.locking_violation_msg = _('Failed to change on disk location of this book\'s files.') + raise publisher = self.publisher.text().strip() if publisher != db.field_for('publisher', self.book_id): db.set_field('publisher', {self.book_id:publisher}) diff --git a/src/calibre/gui2/library/models.py b/src/calibre/gui2/library/models.py index c7beff58b2..4f7a50af4b 100644 --- a/src/calibre/gui2/library/models.py +++ b/src/calibre/gui2/library/models.py @@ -5,11 +5,11 @@ __license__ = 'GPL v3' __copyright__ = '2010, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import errno import functools import numbers import os import re +import sys import time import traceback from collections import defaultdict, namedtuple @@ -20,14 +20,13 @@ from qt.core import ( ) from calibre import ( - fit_image, force_unicode, human_readable, isbytestring, prepare_string_for_xml, - strftime, + fit_image, human_readable, isbytestring, prepare_string_for_xml, strftime, ) from calibre.constants import DEBUG, config_dir, dark_link_color, filesystem_encoding from calibre.db.search import CONTAINS_MATCH, EQUALS_MATCH, REGEXP_MATCH, _match from calibre.ebooks.metadata import authors_to_string, fmt_sidx, string_to_authors from calibre.ebooks.metadata.book.formatter import SafeFormat -from calibre.gui2 import error_dialog +from calibre.gui2 import error_dialog, simple_excepthook from calibre.gui2.library import DEFAULT_SORT from calibre.library.caches import force_to_bool from calibre.library.coloring import color_row_key @@ -641,10 +640,11 @@ class BooksModel(QAbstractTableModel): # {{{ return try: data = self.get_book_display_info(idx) - except Exception: - import traceback - error_dialog(None, _('Unhandled error'), _( - 'Failed to read book data from calibre library. Click "Show details" for more information'), det_msg=traceback.format_exc(), show=True) + except Exception as e: + if sys.excepthook is simple_excepthook or sys.excepthook is sys.__excepthook__: + return # ignore failures during startup/shutdown + e.locking_violation_msg = _('Failed to read cover file for this book from the calibre library.') + raise else: if emit_signal: self.new_bookdisplay_data.emit(data) @@ -1257,13 +1257,10 @@ class BooksModel(QAbstractTableModel): # {{{ return self._set_data(index, value) except OSError as err: import traceback - if getattr(err, 'errno', None) == errno.EACCES: # Permission denied - fname = getattr(err, 'filename', None) - p = 'Locked file: %s\n\n'%force_unicode(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=p+force_unicode(traceback.format_exc()), show=True) + traceback.print_exc() + det_msg = traceback.format_exc() + gui = get_gui() + if gui.show_possible_sharing_violation(err, det_msg): return False error_dialog(get_gui(), _('Failed to set data'), _('Could not set data, click "Show details" to see why.'), diff --git a/src/calibre/gui2/main_window.py b/src/calibre/gui2/main_window.py index a6d02665b5..b61b83fd90 100644 --- a/src/calibre/gui2/main_window.py +++ b/src/calibre/gui2/main_window.py @@ -2,7 +2,7 @@ __license__ = 'GPL v3' __copyright__ = '2008, Kovid Goyal ' -import gc +import gc, os import sys import weakref from qt.core import ( @@ -11,6 +11,7 @@ from qt.core import ( ) from calibre import as_unicode, prepare_string_for_xml, prints +from calibre.constants import iswindows from calibre.gui2 import error_dialog from calibre.utils.config import OptionParser from polyglot.io import PolyglotStringIO @@ -133,6 +134,78 @@ class MainWindow(QMainWindow): def set_exception_handler(self): sys.excepthook = ExceptionHandler(self) + def show_possible_sharing_violation(self, e: Exception, det_msg: str = '') -> bool: + if not iswindows or not isinstance(e, OSError): + return False + from calibre_extensions import winutil + import errno + if not (e.winerror == winutil.ERROR_SHARING_VIOLATION or e.errno == errno.EACCES or isinstance(e, PermissionError)): + return False + msg = getattr(e, 'locking_violation_msg', '') + if msg: + msg = msg.strip() + ' ' + fname = e.filename + + def no_processes_found() -> bool: + is_folder = fname and os.path.isdir(fname) + w = _('folder') if is_folder else _('file') + if e.winerror == winutil.ERROR_SHARING_VIOLATION: + if fname: + dmsg = _('The {0} "{1}" is opened in another program, so calibre cannot access it.').format(w, fname) + else: + dmsg = _('A {} is open in another program so calibre cannot access it.').format(w) + if is_folder: + dmsg += _('This is usually caused by leaving Windows explorer or a similar file manager open' + ' to a folder in the calibre library. Close Windows explorer and retry.') + else: + dmsg += _('This is usually caused by software such as antivirus or file sync (aka DropBox and similar)' + ' accessing files in the calibre library folder at the same time as calibre. Try excluding' + ' the calibre library folder from such software.') + error_dialog(self, _('Cannot open file or folder as it is in use'), msg + dmsg, det_msg=det_msg, show=True) + return True + if msg: + if fname: + dmsg = _('Permission was denied by the operating system when calibre tried to access the file: "{0}".').format(fname) + else: + dmsg = _('Permission was denied by the operating system when calibre tried to access a file.') + dmsg += ' ' + _('This means either that the permissions on the file or its parent folder are incorrect or the file is' + ' open in another program.') + error_dialog(self, _('Cannot open file or folder'), msg + dmsg, det_msg=det_msg, show=True) + return True + return False + + if not hasattr(winutil, 'get_processes_using_files'): + return no_processes_found() # running from source + if not e.filename and not e.filename2: + return no_processes_found() + if e.filename and isinstance(e.filename, str): + if os.path.isdir(e.filename): + return no_processes_found() + try: + p = winutil.get_processes_using_files(e.filename) + except OSError: + return no_processes_found() + if not p and e.filename2 and isinstance(e.filename2, str): + if os.path.isdir(e.filename2): + return no_processes_found() + try: + p = winutil.get_processes_using_files(e.filename2) + except OSError: + return no_processes_found() + fname = e.filename2 + if not p: + return no_processes_found() + + path_map = {x['path']: x for x in p} + is_folder = fname and os.path.isdir(fname) + w = _('folder') if is_folder else _('file') + dmsg = _('Could not open the {0}: "{1}". It is already opened in the following programs:').format(w, fname) + '
' + for path, x in path_map.items(): + dmsg += '
' + prepare_string_for_xml(f'{x["app_name"]}: {path}') + msg = prepare_string_for_xml(msg) + error_dialog(self, _('Cannot open file or folder as it is in use'), '

' + msg + dmsg, det_msg=det_msg, show=True) + return True + def unhandled_exception(self, exc_type, value, tb): if exc_type is KeyboardInterrupt: return @@ -148,10 +221,15 @@ class MainWindow(QMainWindow): if getattr(value, 'locking_debug_msg', None): prints(value.locking_debug_msg, file=sio) fe = sio.getvalue() + prints(fe, file=sys.stderr) + try: + if self.show_possible_sharing_violation(value, det_msg=fe): + return + except Exception: + traceback.print_exc() msg = '%s:'%exc_type.__name__ + prepare_string_for_xml(as_unicode(value)) error_dialog(self, _('Unhandled exception'), msg, det_msg=fe, show=True) - prints(fe, file=sys.stderr) except BaseException: pass except: diff --git a/src/calibre/gui2/metadata/basic_widgets.py b/src/calibre/gui2/metadata/basic_widgets.py index 43c06eca8a..9d85133ce3 100644 --- a/src/calibre/gui2/metadata/basic_widgets.py +++ b/src/calibre/gui2/metadata/basic_widgets.py @@ -54,16 +54,6 @@ from calibre.utils.localization import ngettext from polyglot.builtins import iteritems -def show_locked_file_error(parent, err): - import traceback - fname = getattr(err, 'filename', None) - p = 'Locked file: %s\n\n'%fname if fname else '' - error_dialog(parent, _('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) - - def save_dialog(parent, title, msg, det_msg=''): d = QMessageBox(parent) d.setWindowTitle(title) @@ -388,9 +378,9 @@ class AuthorsEdit(EditWithComplete, ToMetadataMixin): if d == QMessageBox.StandardButton.Yes: try: self.commit(self.db, self.id_) - except PermissionError as err: - show_locked_file_error(self, err) - return + except OSError as e: + e.locking_violation_msg = _('Could not change on-disk location of this book\'s files.') + raise self.db.commit() self.original_val = self.current_val else: diff --git a/src/calibre/gui2/metadata/single.py b/src/calibre/gui2/metadata/single.py index 600cb37503..4d0fdd1cab 100644 --- a/src/calibre/gui2/metadata/single.py +++ b/src/calibre/gui2/metadata/single.py @@ -5,7 +5,6 @@ __license__ = 'GPL v3' __copyright__ = '2011, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import errno import os from datetime import datetime from functools import partial @@ -26,7 +25,7 @@ from calibre.gui2.metadata.basic_widgets import ( AuthorsEdit, AuthorSortEdit, BuddyLabel, CommentsEdit, Cover, DateEdit, FormatsManager, IdentifiersEdit, LanguagesEdit, PubdateEdit, PublisherEdit, RatingEdit, RightClickButton, SeriesEdit, SeriesIndexEdit, TagsEdit, TitleEdit, - TitleSortEdit, show_locked_file_error, + TitleSortEdit ) from calibre.gui2.metadata.single_download import FullFetch from calibre.gui2.widgets2 import CenteredToolButton @@ -448,17 +447,9 @@ class MetadataSingleDialogBase(QDialog): if ext in ('pdf', 'cbz', 'cbr'): return self.choose_cover_from_pages(ext) try: - mi, ext = self.formats_manager.get_selected_format_metadata(self.db, - self.book_id) - except OSError as err: - if getattr(err, 'errno', None) == 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 + mi, ext = self.formats_manager.get_selected_format_metadata(self.db, self.book_id) + except OSError as e: + e.locking_violation_msg = _('Could not read from book file.') raise if mi is None: return @@ -608,18 +599,16 @@ class MetadataSingleDialogBase(QDialog): return True self.comments_edit_state_at_apply = {w:w.tab for w in self.comments_edit_state_at_apply} for widget in self.basic_metadata_widgets: + if hasattr(widget, 'validate_for_commit'): + title, msg, det_msg = widget.validate_for_commit() + if title is not None: + error_dialog(self, title, msg, det_msg=det_msg, show=True) + return False try: - if hasattr(widget, 'validate_for_commit'): - title, msg, det_msg = widget.validate_for_commit() - if title is not None: - error_dialog(self, title, msg, det_msg=det_msg, show=True) - return False widget.commit(self.db, self.book_id) self.books_to_refresh |= getattr(widget, 'books_to_refresh', set()) - except OSError as err: - if getattr(err, 'errno', None) == errno.EACCES: # Permission denied - show_locked_file_error(self, err) - return False + except OSError as e: + e.locking_violation_msg = _('Could not change on-disk location of this book\'s files.') raise for widget in getattr(self, 'custom_metadata_widgets', []): self.books_to_refresh |= widget.commit(self.book_id) diff --git a/src/calibre/utils/copy_files.py b/src/calibre/utils/copy_files.py index 84b226abd5..705b08db9e 100644 --- a/src/calibre/utils/copy_files.py +++ b/src/calibre/utils/copy_files.py @@ -1,7 +1,6 @@ #!/usr/bin/env python # License: GPLv3 Copyright: 2023, Kovid Goyal -import errno import os import shutil import stat @@ -63,6 +62,16 @@ class UnixFileCopier: os.unlink(src_path) +def windows_lock_path_and_callback(path: str, f: Callable) -> None: + is_folder = os.path.isdir(path) + flags = winutil.FILE_FLAG_BACKUP_SEMANTICS if is_folder else winutil.FILE_FLAG_SEQUENTIAL_SCAN + h = winutil.create_file(make_long_path_useable(path), winutil.GENERIC_READ, 0, winutil.OPEN_EXISTING, flags) + try: + f() + finally: + h.close() + + class WindowsFileCopier: ''' @@ -112,9 +121,6 @@ class WindowsFileCopier: if retry_on_sharing_violation: time.sleep(WINDOWS_SLEEP_FOR_RETRY_TIME) return self._open_file(path, False, is_folder) - err = IOError(errno.EACCES, _('File {} is open in another program').format(path)) - err.filename = path - raise err from e raise def open_all_handles(self) -> None: