From 74e549838cfda733d6f3be12db3829afbe1255ea Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 26 Jun 2022 18:38:13 +0200 Subject: [PATCH 1/7] feat(import): Add support for reading skipped paths from logfile Fixes #4379. --- beets/ui/commands.py | 45 +++++++++++++++++++++++++++++++++++++++--- docs/changelog.rst | 3 +++ docs/guides/tagger.rst | 5 ++++- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 3a3374013..b0921728d 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -79,6 +79,26 @@ def _do_query(lib, query, album, also_items=True): return items, albums +def _paths_from_logfile(path): + """Parse the logfile and yield skipped paths to pass to the `import` + command. + """ + with open(path, mode="r", encoding="utf-8") as fp: + for line in fp: + verb, sep, paths = line.rstrip("\n").partition(" ") + if not sep: + raise ValueError("malformed line in logfile") + + # Ignore informational lines that don't need to be re-imported. + if verb in {"import", "duplicate-keep", "duplicate-replace"}: + continue + + if verb not in {"asis", "skip", "duplicate-skip"}: + raise ValueError(f"unknown verb {verb} found in logfile") + + yield os.path.commonpath(paths.split("; ")) + + # fields: Shows a list of available fields for queries and format strings. def _print_keys(query): @@ -914,11 +934,30 @@ def import_files(lib, paths, query): query. """ # Check the user-specified directories. + paths_to_import = [] for path in paths: - if not os.path.exists(syspath(normpath(path))): + normalized_path = syspath(normpath(path)) + if not os.path.exists(normalized_path): raise ui.UserError('no such file or directory: {}'.format( displayable_path(path))) + # Read additional paths from logfile. + if os.path.isfile(normalized_path): + try: + paths_to_import.extend(list( + _paths_from_logfile(normalized_path))) + except ValueError: + # We could raise an error here, but it's possible that the user + # tried to import a media file instead of a logfile. In that + # case, logging a warning would be confusing, so we skip this + # here. + pass + else: + # Don't re-add the log file to the list of paths to import. + continue + + paths_to_import.append(paths) + # Check parameter consistency. if config['import']['quiet'] and config['import']['timid']: raise ui.UserError("can't be both quiet and timid") @@ -939,11 +978,11 @@ def import_files(lib, paths, query): config['import']['quiet']: config['import']['resume'] = False - session = TerminalImportSession(lib, loghandler, paths, query) + session = TerminalImportSession(lib, loghandler, paths_to_import, query) session.run() # Emit event. - plugins.send('import', lib=lib, paths=paths) + plugins.send('import', lib=lib, paths=paths_to_import) def import_func(lib, opts, args): diff --git a/docs/changelog.rst b/docs/changelog.rst index dbee12764..a5086b6b9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,9 @@ New features: :bug:`1840` :bug:`4302` * Added a ``-P`` (or ``--disable-plugins``) flag to specify one/multiple plugin(s) to be disabled at startup. +* :ref:`import-options`: Add support for re-running the importer on paths in + log files that were created with the ``-l`` (or ``--logfile``) argument. + :bug:`4379` :bug:`4387` Bug fixes: diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index d890f5c08..5184f433f 100644 --- a/docs/guides/tagger.rst +++ b/docs/guides/tagger.rst @@ -80,6 +80,8 @@ all of these limitations. Now that that's out of the way, let's tag some music. +.. _import-options: + Options ------- @@ -101,7 +103,8 @@ command-line options you should know: * ``beet import -l LOGFILE``: write a message to ``LOGFILE`` every time you skip an album or choose to take its tags "as-is" (see below) or the album is skipped as a duplicate; this lets you come back later and reexamine albums - that weren't tagged successfully + that weren't tagged successfully. Run ``beet import LOGFILE`` rerun the + importer on such paths from the logfile. * ``beet import -q``: quiet mode. Never prompt for input and, instead, conservatively skip any albums that need your opinion. The ``-ql`` combination From 6d74ffb833c4edca1c0573e9728f8c31395adbc8 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 27 Jun 2022 19:00:40 +0200 Subject: [PATCH 2/7] feat(import): Move logfile import logic to `--from-logfile` argument As requested here: https://github.com/beetbox/beets/pull/4387#pullrequestreview-1020040380 --- beets/ui/commands.py | 79 +++++++++++++++++++++++++++--------------- docs/guides/tagger.rst | 4 +-- docs/reference/cli.rst | 4 ++- 3 files changed, 56 insertions(+), 31 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index b0921728d..1bcb1de1d 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -933,31 +933,6 @@ def import_files(lib, paths, query): """Import the files in the given list of paths or matching the query. """ - # Check the user-specified directories. - paths_to_import = [] - for path in paths: - normalized_path = syspath(normpath(path)) - if not os.path.exists(normalized_path): - raise ui.UserError('no such file or directory: {}'.format( - displayable_path(path))) - - # Read additional paths from logfile. - if os.path.isfile(normalized_path): - try: - paths_to_import.extend(list( - _paths_from_logfile(normalized_path))) - except ValueError: - # We could raise an error here, but it's possible that the user - # tried to import a media file instead of a logfile. In that - # case, logging a warning would be confusing, so we skip this - # here. - pass - else: - # Don't re-add the log file to the list of paths to import. - continue - - paths_to_import.append(paths) - # Check parameter consistency. if config['import']['quiet'] and config['import']['timid']: raise ui.UserError("can't be both quiet and timid") @@ -978,11 +953,11 @@ def import_files(lib, paths, query): config['import']['quiet']: config['import']['resume'] = False - session = TerminalImportSession(lib, loghandler, paths_to_import, query) + session = TerminalImportSession(lib, loghandler, paths, query) session.run() # Emit event. - plugins.send('import', lib=lib, paths=paths_to_import) + plugins.send('import', lib=lib, paths=paths) def import_func(lib, opts, args): @@ -999,7 +974,25 @@ def import_func(lib, opts, args): else: query = None paths = args - if not paths: + + # The paths from the logfiles go into a separate list to allow handling + # errors differently from user-specified paths. + paths_from_logfiles = [] + for logfile in opts.from_logfiles or []: + try: + paths_from_logfiles.extend( + list(_paths_from_logfile(syspath(normpath(logfile)))) + ) + except ValueError as err: + raise ui.UserError('malformed logfile {}'.format( + util.displayable_path(logfile), + )) from err + except IOError as err: + raise ui.UserError('unreadable logfile {}'.format( + util.displayable_path(logfile), + )) from err + + if not paths and not paths_from_logfiles: raise ui.UserError('no path specified') # On Python 2, we used to get filenames as raw bytes, which is @@ -1008,6 +1001,31 @@ def import_func(lib, opts, args): # filename. paths = [p.encode(util.arg_encoding(), 'surrogateescape') for p in paths] + paths_from_logfiles = [p.encode(util.arg_encoding(), 'surrogateescape') + for p in paths_from_logfiles] + + # Check the user-specified directories. + for path in paths: + if not os.path.exists(syspath(normpath(path))): + raise ui.UserError('no such file or directory: {}'.format( + displayable_path(path))) + + # Check the directories from the logfiles, but don't throw an error in + # case those paths don't exist. Maybe some of those paths have already + # been imported and moved separately, so logging a warning should + # suffice. + for path in paths_from_logfiles: + if not os.path.exists(syspath(normpath(path))): + log.warning('No such file or directory: {}'.format( + displayable_path(path))) + continue + + paths.append(path) + + # If all paths were read from a logfile, and none of them exist, throw + # an error + if not paths: + raise ui.UserError('none of the paths are importable') import_files(lib, paths, query) @@ -1100,6 +1118,11 @@ import_cmd.parser.add_option( metavar='ID', help='restrict matching to a specific metadata backend ID' ) +import_cmd.parser.add_option( + '--from-logfile', dest='from_logfiles', action='append', + metavar='PATH', + help='read skipped paths from an existing logfile' +) import_cmd.parser.add_option( '--set', dest='set_fields', action='callback', callback=_store_dict, diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index 5184f433f..d47ee3c4a 100644 --- a/docs/guides/tagger.rst +++ b/docs/guides/tagger.rst @@ -103,8 +103,8 @@ command-line options you should know: * ``beet import -l LOGFILE``: write a message to ``LOGFILE`` every time you skip an album or choose to take its tags "as-is" (see below) or the album is skipped as a duplicate; this lets you come back later and reexamine albums - that weren't tagged successfully. Run ``beet import LOGFILE`` rerun the - importer on such paths from the logfile. + that weren't tagged successfully. Run ``beet import --from-logfile=LOGFILE`` + rerun the importer on such paths from the logfile. * ``beet import -q``: quiet mode. Never prompt for input and, instead, conservatively skip any albums that need your opinion. The ``-ql`` combination diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index dad407489..3726fb373 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -86,7 +86,9 @@ Optional command flags: that weren't tagged successfully---either because they're not in the MusicBrainz database or because something's wrong with the files. Use the ``-l`` option to specify a filename to log every time you skip an album - or import it "as-is" or an album gets skipped as a duplicate. + or import it "as-is" or an album gets skipped as a duplicate. You can later + review the file manually or import skipped paths from the logfile + automatically by using the ``--from-logfile LOGFILE`` argument. * Relatedly, the ``-q`` (quiet) option can help with large imports by autotagging without ever bothering to ask for user input. Whenever the From 3e37d0163e2575000265be2349d7da5702cfce4a Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 27 Jun 2022 19:48:22 +0200 Subject: [PATCH 3/7] test(import): Add test for `_paths_from_logfile` method --- test/test_ui.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/test_ui.py b/test/test_ui.py index dd24fce1a..0c7478d45 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -729,6 +729,40 @@ class ImportTest(_common.TestCase): self.assertRaises(ui.UserError, commands.import_files, None, [], None) + def test_parse_paths_from_logfile(self): + if os.path.__name__ == 'ntpath': + logfile_content = ( + "import started Wed Jun 15 23:08:26 2022\n" + "asis C:\\music\\Beatles, The\\The Beatles; C:\\music\\Beatles, The\\The Beatles\\CD 01; C:\\music\\Beatles, The\\The Beatles\\CD 02\n" # noqa: E501 + "duplicate-replace C:\\music\\Bill Evans\\Trio '65\n" + "skip C:\\music\\Michael Jackson\\Bad\n" + "skip C:\\music\\Soulwax\\Any Minute Now\n" + ) + expected_paths = [ + "C:\\music\\Beatles, The\\The Beatles", + "C:\\music\\Michael Jackson\\Bad", + "C:\\music\\Soulwax\\Any Minute Now", + ] + else: + logfile_content = ( + "import started Wed Jun 15 23:08:26 2022\n" + "asis /music/Beatles, The/The Beatles; /music/Beatles, The/The Beatles/CD 01; /music/Beatles, The/The Beatles/CD 02\n" # noqa: E501 + "duplicate-replace /music/Bill Evans/Trio '65\n" + "skip /music/Michael Jackson/Bad\n" + "skip /music/Soulwax/Any Minute Now\n" + ) + expected_paths = [ + "/music/Beatles, The/The Beatles", + "/music/Michael Jackson/Bad", + "/music/Soulwax/Any Minute Now", + ] + + logfile = os.path.join(self.temp_dir, b"logfile.log") + with open(logfile, mode="w") as fp: + fp.write(logfile_content) + actual_paths = list(commands._paths_from_logfile(logfile)) + self.assertEqual(actual_paths, expected_paths) + @_common.slow_test() class ConfigTest(unittest.TestCase, TestHelper, _common.Assertions): From cfc34826a20203240e1bc8f2ba0cee328dfe18df Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 28 Jun 2022 22:10:11 +0200 Subject: [PATCH 4/7] feat(import): Improve error reporting when reading paths from logfile --- beets/ui/commands.py | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 1bcb1de1d..cbdd8e3bf 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -84,21 +84,38 @@ def _paths_from_logfile(path): command. """ with open(path, mode="r", encoding="utf-8") as fp: - for line in fp: + for i, line in enumerate(fp, start=1): verb, sep, paths = line.rstrip("\n").partition(" ") if not sep: - raise ValueError("malformed line in logfile") + raise ValueError(f"line {i} is invalid") # Ignore informational lines that don't need to be re-imported. if verb in {"import", "duplicate-keep", "duplicate-replace"}: continue if verb not in {"asis", "skip", "duplicate-skip"}: - raise ValueError(f"unknown verb {verb} found in logfile") + raise ValueError(f"line {i} contains unknown verb {verb}") yield os.path.commonpath(paths.split("; ")) +def _parse_logfiles(logfiles): + """Parse all `logfiles` and yield paths from it.""" + for logfile in logfiles: + try: + yield from _paths_from_logfile(syspath(normpath(logfile))) + except ValueError as err: + raise ui.UserError('malformed logfile {}: {}'.format( + util.displayable_path(logfile), + str(err) + )) from err + except IOError as err: + raise ui.UserError('unreadable logfile {}: {}'.format( + util.displayable_path(logfile), + str(err) + )) from err + + # fields: Shows a list of available fields for queries and format strings. def _print_keys(query): @@ -977,20 +994,7 @@ def import_func(lib, opts, args): # The paths from the logfiles go into a separate list to allow handling # errors differently from user-specified paths. - paths_from_logfiles = [] - for logfile in opts.from_logfiles or []: - try: - paths_from_logfiles.extend( - list(_paths_from_logfile(syspath(normpath(logfile)))) - ) - except ValueError as err: - raise ui.UserError('malformed logfile {}'.format( - util.displayable_path(logfile), - )) from err - except IOError as err: - raise ui.UserError('unreadable logfile {}'.format( - util.displayable_path(logfile), - )) from err + paths_from_logfiles = list(_parse_logfiles(opts.from_logfiles or [])) if not paths and not paths_from_logfiles: raise ui.UserError('no path specified') From 91c2cd2ee50cce85b9f4d42956a96786e79ca346 Mon Sep 17 00:00:00 2001 From: toaru_yousei Date: Wed, 21 Jul 2021 21:02:39 +0900 Subject: [PATCH 5/7] Fix importadded plugin with reflink --- beetsplug/importadded.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beetsplug/importadded.py b/beetsplug/importadded.py index e6665e0ff..409056c80 100644 --- a/beetsplug/importadded.py +++ b/beetsplug/importadded.py @@ -34,6 +34,7 @@ class ImportAddedPlugin(BeetsPlugin): register('item_copied', self.record_import_mtime) register('item_linked', self.record_import_mtime) register('item_hardlinked', self.record_import_mtime) + register('item_reflinked', self.record_import_mtime) register('album_imported', self.update_album_times) register('item_imported', self.update_item_times) register('after_write', self.update_after_write_time) @@ -49,7 +50,8 @@ class ImportAddedPlugin(BeetsPlugin): def record_if_inplace(self, task, session): if not (session.config['copy'] or session.config['move'] or - session.config['link'] or session.config['hardlink']): + session.config['link'] or session.config['hardlink'] or + session.config['reflink']): self._log.debug("In place import detected, recording mtimes from " "source paths") items = [task.item] \ From ea571a56f214c6fc5767946ec74f2611bb5183f5 Mon Sep 17 00:00:00 2001 From: toaru_yousei Date: Thu, 30 Jun 2022 00:50:55 +0900 Subject: [PATCH 6/7] ImportAdded reflink fix: Update changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index dbee12764..31ca6aa3f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -82,6 +82,9 @@ Bug fixes: :bug:`4272` * :doc:`plugins/lyrics`: Fixed issue with Genius header being included in lyrics, added test case of up-to-date Genius html +* :doc:`plugins/importadded`: Fix a bug with recently added reflink import option + that casues a crash when ImportAdded plugin enabled. + :bug:`4389` For packagers: From bfe008ed24255542a7cd09c3f2624fcc586e5955 Mon Sep 17 00:00:00 2001 From: Mark Trolley Date: Thu, 30 Jun 2022 14:19:58 -0400 Subject: [PATCH 7/7] Fix typo in Acousticbrainz warning log Add missing space in Acousticbrainz plugin warning log. --- beetsplug/acousticbrainz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index eabc5849f..040463375 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -321,7 +321,7 @@ class AcousticPlugin(plugins.BeetsPlugin): else: yield v, subdata[k] else: - self._log.warning('Acousticbrainz did not provide info' + self._log.warning('Acousticbrainz did not provide info ' 'about {}', k) self._log.debug('Data {} could not be mapped to scheme {} ' 'because key {} was not found', subdata, v, k)