From d95bb5683b6413b5d2e45dafdb86013a732e70b2 Mon Sep 17 00:00:00 2001 From: ybnd Date: Tue, 28 Jan 2020 21:19:39 +0100 Subject: [PATCH 01/89] Analyze replaygain in parallel with multiprocessing.pool.ThreadPool * Add `--jobs` or `-j` to `replaygain`-> set the pool size * Single-threaded execution by default, if `--jobs` is unset * If multithreaded, calls `Backend.compute_album_gain` or `Backend.compute_track_gain` asynchronously with metadata storing/writing in the callback --- beetsplug/replaygain.py | 123 ++++++++++++++++++++++++++++++---------- 1 file changed, 94 insertions(+), 29 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 1076ac714..e534c6fb8 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -104,6 +104,8 @@ class Backend(object): """An abstract class representing engine for calculating RG values. """ + do_parallel = False + def __init__(self, config, log): """Initialize the backend with the configuration view for the plugin. @@ -135,6 +137,8 @@ class Bs1770gainBackend(Backend): -18: "replaygain", } + do_parallel = True + def __init__(self, config, log): super(Bs1770gainBackend, self).__init__(config, log) config.add({ @@ -346,6 +350,9 @@ class Bs1770gainBackend(Backend): class FfmpegBackend(Backend): """A replaygain backend using ffmpeg's ebur128 filter. """ + + do_parallel = True + def __init__(self, config, log): super(FfmpegBackend, self).__init__(config, log) self._ffmpeg_path = "ffmpeg" @@ -588,6 +595,7 @@ class FfmpegBackend(Backend): # mpgain/aacgain CLI tool backend. class CommandBackend(Backend): + do_parallel = True def __init__(self, config, log): super(CommandBackend, self).__init__(config, log) @@ -716,7 +724,6 @@ class CommandBackend(Backend): # GStreamer-based backend. class GStreamerBackend(Backend): - def __init__(self, config, log): super(GStreamerBackend, self).__init__(config, log) self._import_gst() @@ -1173,6 +1180,7 @@ class ReplayGainPlugin(BeetsPlugin): self.overwrite = self.config['overwrite'].get(bool) self.per_disc = self.config['per_disc'].get(bool) backend_name = self.config['backend'].as_str() + if backend_name not in self.backends: raise ui.UserError( u"Selected ReplayGain backend {0} is not supported. " @@ -1290,8 +1298,6 @@ class ReplayGainPlugin(BeetsPlugin): self._log.info(u'Skipping album {0}', album) return - self._log.info(u'analyzing {0}', album) - if (any([self.should_use_r128(item) for item in album.items()]) and not all(([self.should_use_r128(item) for item in album.items()]))): raise ReplayGainError( @@ -1299,6 +1305,8 @@ class ReplayGainPlugin(BeetsPlugin): u" for some tracks in album {0}".format(album) ) + self._log.info(u'analyzing {0}', album) + tag_vals = self.tag_specific_values(album.items()) store_track_gain, store_album_gain, target_level, peak = tag_vals @@ -1311,27 +1319,56 @@ class ReplayGainPlugin(BeetsPlugin): else: discs[1] = album.items() - for discnumber, items in discs.items(): - try: - album_gain = self.backend_instance.compute_album_gain( - items, target_level, peak + if hasattr(self, 'pool'): + for discnumber, items in discs.items(): + def _store_album(album_gain): + if len(album_gain.track_gains) != len(items): + raise ReplayGainError( + u"ReplayGain backend failed " + u"for some tracks in album {0}".format(album) + ) + + for item, track_gain in zip(items, + album_gain.track_gains): + store_track_gain(item, track_gain) + store_album_gain(item, album_gain.album_gain) + if write: + item.try_write() + self._log.debug(u'done analyzing {0}', item) + + self.pool.apply_async( + self.backend_instance.compute_album_gain, args=(), + kwds={ + "items": [i for i in items], + "target_level": target_level, + "peak": peak + }, + callback=_store_album ) - if len(album_gain.track_gains) != len(items): - raise ReplayGainError( - u"ReplayGain backend failed " - u"for some tracks in album {0}".format(album) + else: + for discnumber, items in discs.items(): + try: + album_gain = self.backend_instance.compute_album_gain( + items, target_level, peak ) - for item, track_gain in zip(items, album_gain.track_gains): - store_track_gain(item, track_gain) - store_album_gain(item, album_gain.album_gain) - if write: - item.try_write() - except ReplayGainError as e: - self._log.info(u"ReplayGain error: {0}", e) - except FatalReplayGainError as e: - raise ui.UserError( - u"Fatal replay gain error: {0}".format(e)) + if len(album_gain.track_gains) != len(items): + raise ReplayGainError( + u"ReplayGain backend failed " + u"for some tracks in album {0}".format(album) + ) + + for item, track_gain in zip(items, + album_gain.track_gains): + store_track_gain(item, track_gain) + store_album_gain(item, album_gain.album_gain) + if write: + item.try_write() + except ReplayGainError as e: + self._log.info(u"ReplayGain error: {0}", e) + except FatalReplayGainError as e: + raise ui.UserError( + u"Fatal replay gain error: {0}".format(e)) def handle_track(self, item, write, force=False): """Compute track replay gain and store it in the item. @@ -1344,23 +1381,38 @@ class ReplayGainPlugin(BeetsPlugin): self._log.info(u'Skipping track {0}', item) return - self._log.info(u'analyzing {0}', item) - tag_vals = self.tag_specific_values([item]) store_track_gain, store_album_gain, target_level, peak = tag_vals - try: - track_gains = self.backend_instance.compute_track_gain( - [item], target_level, peak - ) + def _store_track(track_gains): if len(track_gains) != 1: raise ReplayGainError( - u"ReplayGain backend failed for track {0}".format(item) + u"ReplayGain backend failed for track {0}".format( + item) ) store_track_gain(item, track_gains[0]) if write: item.try_write() + self._log.debug(u'done analyzing {0}', item) + + try: + if hasattr(self, 'pool'): + self.pool.apply_async( + self.backend_instance.compute_track_gain, args=(), + kwds={ + "items": [item], + "target_level": target_level, + "peak": peak, + }, + callback=_store_track + ) + else: + _store_track( + self.backend_instance.compute_track_gain( + [item], target_level, peak + ) + ) except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: @@ -1381,17 +1433,30 @@ class ReplayGainPlugin(BeetsPlugin): def func(lib, opts, args): write = ui.should_write(opts.write) force = opts.force + jobs = opts.jobs + + if self.backend_instance.do_parallel and jobs > 0: + from multiprocessing.pool import ThreadPool + self.pool = ThreadPool(jobs) if opts.album: for album in lib.albums(ui.decargs(args)): self.handle_album(album, write, force) - else: for item in lib.items(ui.decargs(args)): self.handle_track(item, write, force) + if hasattr(self, 'pool'): + self.pool.close() + self.pool.join() + self.pool.terminate() + cmd = ui.Subcommand('replaygain', help=u'analyze for ReplayGain') cmd.parser.add_album_option() + cmd.parser.add_option( + "-j", "--jobs", dest="jobs", type=int, default=0, + help=u"worker pool size" + ) cmd.parser.add_option( "-f", "--force", dest="force", action="store_true", default=False, help=u"analyze all files, including those that " From 42e895c2396c48dafcd3e853963813b380ced02b Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 08:28:04 +0100 Subject: [PATCH 02/89] Match --jobs default & signature to that of convert plugin (--threads) And change local function `func` to `ReplayGainPlugin` method `replaygain_func` so that `self` is passed explicitly --- beetsplug/replaygain.py | 54 ++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e534c6fb8..e361016d8 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -28,7 +28,7 @@ from six.moves import zip from beets import ui from beets.plugins import BeetsPlugin from beets.util import (syspath, command_output, bytestring_path, - displayable_path, py3_path) + displayable_path, py3_path, cpu_count) # Utilities. @@ -1170,6 +1170,7 @@ class ReplayGainPlugin(BeetsPlugin): 'overwrite': False, 'auto': True, 'backend': u'command', + 'threads': cpu_count(), 'per_disc': False, 'peak': 'true', 'targetlevel': 89, @@ -1430,32 +1431,12 @@ class ReplayGainPlugin(BeetsPlugin): def commands(self): """Return the "replaygain" ui subcommand. """ - def func(lib, opts, args): - write = ui.should_write(opts.write) - force = opts.force - jobs = opts.jobs - - if self.backend_instance.do_parallel and jobs > 0: - from multiprocessing.pool import ThreadPool - self.pool = ThreadPool(jobs) - - if opts.album: - for album in lib.albums(ui.decargs(args)): - self.handle_album(album, write, force) - else: - for item in lib.items(ui.decargs(args)): - self.handle_track(item, write, force) - - if hasattr(self, 'pool'): - self.pool.close() - self.pool.join() - self.pool.terminate() - cmd = ui.Subcommand('replaygain', help=u'analyze for ReplayGain') cmd.parser.add_album_option() cmd.parser.add_option( - "-j", "--jobs", dest="jobs", type=int, default=0, - help=u"worker pool size" + "-t", "--threads", dest="threads", + help=u'change the number of threads, \ + defaults to maximum available processors' ) cmd.parser.add_option( "-f", "--force", dest="force", action="store_true", default=False, @@ -1467,5 +1448,28 @@ class ReplayGainPlugin(BeetsPlugin): cmd.parser.add_option( "-W", "--nowrite", dest="write", action="store_false", help=u"don't write metadata (opposite of -w)") - cmd.func = func + cmd.func = self.replaygain_func return [cmd] + + def replaygain_func(self, lib, opts, args): + """Handle `replaygain` ui subcommand + """ + write = ui.should_write(opts.write) + force = opts.force + threads = opts.threads or self.config['threads'].get(int) + + if self.backend_instance.do_parallel: + from multiprocessing.pool import ThreadPool + self.pool = ThreadPool(threads) + + if opts.album: + for album in lib.albums(ui.decargs(args)): + self.handle_album(album, write, force) + else: + for item in lib.items(ui.decargs(args)): + self.handle_track(item, write, force) + + if hasattr(self, 'pool'): + self.pool.close() + self.pool.join() + self.pool.terminate() From 388d2d2c0de0b7bed66d661645b6adead79c3c09 Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 09:32:03 +0100 Subject: [PATCH 03/89] Consolidate `ThreadPool` checking, opening and closing into methods --- beetsplug/replaygain.py | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e361016d8..e540d3d99 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -22,6 +22,7 @@ import math import sys import warnings import enum +from multiprocessing.pool import ThreadPool import xml.parsers.expat from six.moves import zip @@ -1320,7 +1321,7 @@ class ReplayGainPlugin(BeetsPlugin): else: discs[1] = album.items() - if hasattr(self, 'pool'): + if self.has_pool(): for discnumber, items in discs.items(): def _store_album(album_gain): if len(album_gain.track_gains) != len(items): @@ -1398,7 +1399,7 @@ class ReplayGainPlugin(BeetsPlugin): self._log.debug(u'done analyzing {0}', item) try: - if hasattr(self, 'pool'): + if self.has_pool(): self.pool.apply_async( self.backend_instance.compute_track_gain, args=(), kwds={ @@ -1420,6 +1421,27 @@ class ReplayGainPlugin(BeetsPlugin): raise ui.UserError( u"Fatal replay gain error: {0}".format(e)) + def has_pool(self): + """Check whether a `ThreadPool` is running instance in `self.pool` + """ + if hasattr(self, 'pool'): + if isinstance(self.pool, ThreadPool) and self.pool._state == 'RUN': + return True + return False + + def open_pool(self, threads): + """Open a `ThreadPool` instance in `self.pool` + """ + if not self.has_pool() and self.backend_instance.do_parallel: + self.pool = ThreadPool(threads) + + def close_pool(self): + """Close the `ThreadPool` instance in `self.pool` (if there is one) + """ + if self.has_pool(): + self.pool.close() + self.pool.join() + def imported(self, session, task): """Add replay gain info to items or albums of ``task``. """ @@ -1450,7 +1472,7 @@ class ReplayGainPlugin(BeetsPlugin): help=u"don't write metadata (opposite of -w)") cmd.func = self.replaygain_func return [cmd] - + def replaygain_func(self, lib, opts, args): """Handle `replaygain` ui subcommand """ @@ -1458,9 +1480,7 @@ class ReplayGainPlugin(BeetsPlugin): force = opts.force threads = opts.threads or self.config['threads'].get(int) - if self.backend_instance.do_parallel: - from multiprocessing.pool import ThreadPool - self.pool = ThreadPool(threads) + self.open_pool(threads) if opts.album: for album in lib.albums(ui.decargs(args)): @@ -1469,7 +1489,4 @@ class ReplayGainPlugin(BeetsPlugin): for item in lib.items(ui.decargs(args)): self.handle_track(item, write, force) - if hasattr(self, 'pool'): - self.pool.close() - self.pool.join() - self.pool.terminate() + self.close_pool() From 79c5535cf6ea80993ae1d2f00ea15911f2581c3c Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 09:34:37 +0100 Subject: [PATCH 04/89] Open/close pool at begin/end of import session --- beetsplug/replaygain.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e540d3d99..4d963816d 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1204,6 +1204,8 @@ class ReplayGainPlugin(BeetsPlugin): # On-import analysis. if self.config['auto']: + self.register_listener('import_begin', self.import_begin) + self.register_listener('import', self.import_end) self.import_stages = [self.imported] # Formats to use R128. @@ -1442,6 +1444,16 @@ class ReplayGainPlugin(BeetsPlugin): self.pool.close() self.pool.join() + def import_begin(self, session): + """Handle `import_begin` event -> open pool + """ + self.open_pool(self.config['threads'].get(int)) + + def import_end(self, paths): + """Handle `import` event -> close pool + """ + self.close_pool() + def imported(self, session, task): """Add replay gain info to items or albums of ``task``. """ From 0fede91bbd1a037dc7a9e277c0219f0adf7fd74b Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 11:43:19 +0100 Subject: [PATCH 05/89] Workaround to pass ReplayGainLdnsCliMalformedTest.test_malformed_output ~ Python 3.8 --- beetsplug/replaygain.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 4d963816d..12b292af5 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -323,10 +323,19 @@ class Bs1770gainBackend(Backend): try: parser.Parse(text, True) except xml.parsers.expat.ExpatError: - raise ReplayGainError( - u'The bs1770gain tool produced malformed XML. ' - 'Using version >=0.4.10 may solve this problem.' - ) + # ReplayGainLdnsCliMalformedTest.test_malformed_output + # fails in Python 3.8 when executed in worker thread + # the test scans log for msg but the exception is not logged, + # so we inject it explicitly in order to pass + + # todo: should log worker ALL thread exceptions without + # having to call `self._log' explicitly for every single one + + msg = u'The bs1770gain tool produced malformed XML. ' \ + u'Using version >=0.4.10 may solve this problem.' + if sys.version >= '3.8': + self._log.info(msg) + raise ReplayGainError(msg) if len(per_file_gain) != len(path_list): raise ReplayGainError( From b126ecafdd6c7f15b4493fe5fc3c43f3e9d06ddb Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 12:38:20 +0100 Subject: [PATCH 06/89] Clean up single/multithreaded execution selection logic As suggested in https://github.com/beetbox/beets/pull/3478#discussion_r372467445 --- beetsplug/replaygain.py | 83 ++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 51 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 12b292af5..031c8845a 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1332,9 +1332,9 @@ class ReplayGainPlugin(BeetsPlugin): else: discs[1] = album.items() - if self.has_pool(): - for discnumber, items in discs.items(): - def _store_album(album_gain): + for discnumber, items in discs.items(): + def _store_album(album_gain): + try: if len(album_gain.track_gains) != len(items): raise ReplayGainError( u"ReplayGain backend failed " @@ -1348,41 +1348,22 @@ class ReplayGainPlugin(BeetsPlugin): if write: item.try_write() self._log.debug(u'done analyzing {0}', item) - - self.pool.apply_async( - self.backend_instance.compute_album_gain, args=(), - kwds={ - "items": [i for i in items], - "target_level": target_level, - "peak": peak - }, - callback=_store_album - ) - else: - for discnumber, items in discs.items(): - try: - album_gain = self.backend_instance.compute_album_gain( - items, target_level, peak - ) - - if len(album_gain.track_gains) != len(items): - raise ReplayGainError( - u"ReplayGain backend failed " - u"for some tracks in album {0}".format(album) - ) - - for item, track_gain in zip(items, - album_gain.track_gains): - store_track_gain(item, track_gain) - store_album_gain(item, album_gain.album_gain) - if write: - item.try_write() except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: raise ui.UserError( u"Fatal replay gain error: {0}".format(e)) + self._apply( + self.backend_instance.compute_album_gain, args=(), + kwds={ + "items": [i for i in items], + "target_level": target_level, + "peak": peak + }, + callback=_store_album + ) + def handle_track(self, item, write, force=False): """Compute track replay gain and store it in the item. @@ -1410,29 +1391,23 @@ class ReplayGainPlugin(BeetsPlugin): self._log.debug(u'done analyzing {0}', item) try: - if self.has_pool(): - self.pool.apply_async( - self.backend_instance.compute_track_gain, args=(), - kwds={ - "items": [item], - "target_level": target_level, - "peak": peak, - }, - callback=_store_track - ) - else: - _store_track( - self.backend_instance.compute_track_gain( - [item], target_level, peak - ) - ) + self._apply( + self.backend_instance.compute_track_gain, args=(), + kwds={ + "items": [item], + "target_level": target_level, + "peak": peak, + }, + callback=_store_track + ) + except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: raise ui.UserError( u"Fatal replay gain error: {0}".format(e)) - def has_pool(self): + def _has_pool(self): """Check whether a `ThreadPool` is running instance in `self.pool` """ if hasattr(self, 'pool'): @@ -1443,13 +1418,19 @@ class ReplayGainPlugin(BeetsPlugin): def open_pool(self, threads): """Open a `ThreadPool` instance in `self.pool` """ - if not self.has_pool() and self.backend_instance.do_parallel: + if not self._has_pool() and self.backend_instance.do_parallel: self.pool = ThreadPool(threads) + def _apply(self, func, args, kwds, callback): + if self._has_pool(): + self.pool.apply_async(func, args, kwds, callback) + else: + callback(func(*args, **kwds)) + def close_pool(self): """Close the `ThreadPool` instance in `self.pool` (if there is one) """ - if self.has_pool(): + if self._has_pool(): self.pool.close() self.pool.join() From b90358416331f416f36d6857bc3bc10534feadcb Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 13:31:14 +0100 Subject: [PATCH 07/89] Fix `--threads` argument handling --- beetsplug/replaygain.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 031c8845a..b00edcabe 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1458,7 +1458,7 @@ class ReplayGainPlugin(BeetsPlugin): cmd = ui.Subcommand('replaygain', help=u'analyze for ReplayGain') cmd.parser.add_album_option() cmd.parser.add_option( - "-t", "--threads", dest="threads", + "-t", "--threads", dest="threads", type=int, help=u'change the number of threads, \ defaults to maximum available processors' ) @@ -1480,9 +1480,10 @@ class ReplayGainPlugin(BeetsPlugin): """ write = ui.should_write(opts.write) force = opts.force - threads = opts.threads or self.config['threads'].get(int) - self.open_pool(threads) + if opts.threads != 0: + threads = opts.threads or self.config['threads'].get(int) + self.open_pool(threads) if opts.album: for album in lib.albums(ui.decargs(args)): From 65ffca215a27b5dd37b9a6ff4abfee97f0e0632a Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 17:58:40 +0100 Subject: [PATCH 08/89] Exception handling in main & worker threads * With `bs1770gain` installed the `Bs1770gainBackend` tests fail, but this should be fixed by https://github.com/beetbox/beets/pull/3480. --- beetsplug/replaygain.py | 64 +++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index b00edcabe..bf913b397 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1334,13 +1334,12 @@ class ReplayGainPlugin(BeetsPlugin): for discnumber, items in discs.items(): def _store_album(album_gain): + if len(album_gain.track_gains) != len(items): + raise ReplayGainError( + u"ReplayGain backend failed " + u"for some tracks in album {0}".format(album) + ) try: - if len(album_gain.track_gains) != len(items): - raise ReplayGainError( - u"ReplayGain backend failed " - u"for some tracks in album {0}".format(album) - ) - for item, track_gain in zip(items, album_gain.track_gains): store_track_gain(item, track_gain) @@ -1351,18 +1350,23 @@ class ReplayGainPlugin(BeetsPlugin): except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: - raise ui.UserError( - u"Fatal replay gain error: {0}".format(e)) + raise ui.UserError(u"Fatal ReplayGain error: {0}".format(e)) - self._apply( - self.backend_instance.compute_album_gain, args=(), - kwds={ - "items": [i for i in items], - "target_level": target_level, - "peak": peak - }, - callback=_store_album - ) + try: + self._apply( + self.backend_instance.compute_album_gain, args=(), + kwds={ + "items": [i for i in items], + "target_level": target_level, + "peak": peak + }, + callback=_store_album + ) + except ReplayGainError as e: + self._log.info(u"ReplayGain error: {0}", e) + except FatalReplayGainError as e: + raise ui.UserError( + u"Fatal replay gain error: {0}".format(e)) def handle_track(self, item, write, force=False): """Compute track replay gain and store it in the item. @@ -1379,16 +1383,21 @@ class ReplayGainPlugin(BeetsPlugin): store_track_gain, store_album_gain, target_level, peak = tag_vals def _store_track(track_gains): - if len(track_gains) != 1: - raise ReplayGainError( - u"ReplayGain backend failed for track {0}".format( - item) - ) + try: + if len(track_gains) != 1: + raise ReplayGainError( + u"ReplayGain backend failed for track {0}".format( + item) + ) - store_track_gain(item, track_gains[0]) - if write: - item.try_write() - self._log.debug(u'done analyzing {0}', item) + store_track_gain(item, track_gains[0]) + if write: + item.try_write() + self._log.debug(u'done analyzing {0}', item) + except ReplayGainError as e: + self._log.info(u"ReplayGain error: {0}", e) + except FatalReplayGainError as e: + raise ui.UserError(u"Fatal ReplayGain error: {0}".format(e)) try: self._apply( @@ -1404,8 +1413,7 @@ class ReplayGainPlugin(BeetsPlugin): except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: - raise ui.UserError( - u"Fatal replay gain error: {0}".format(e)) + raise ui.UserError(u"Fatal replay gain error: {0}".format(e)) def _has_pool(self): """Check whether a `ThreadPool` is running instance in `self.pool` From f51a68c7e100aea55b092b976076a04d59f32986 Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 19:48:31 +0100 Subject: [PATCH 09/89] Implement comments --- beetsplug/replaygain.py | 82 ++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index bf913b397..2658d2dd3 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1339,18 +1339,13 @@ class ReplayGainPlugin(BeetsPlugin): u"ReplayGain backend failed " u"for some tracks in album {0}".format(album) ) - try: - for item, track_gain in zip(items, - album_gain.track_gains): - store_track_gain(item, track_gain) - store_album_gain(item, album_gain.album_gain) - if write: - item.try_write() - self._log.debug(u'done analyzing {0}', item) - except ReplayGainError as e: - self._log.info(u"ReplayGain error: {0}", e) - except FatalReplayGainError as e: - raise ui.UserError(u"Fatal ReplayGain error: {0}".format(e)) + for item, track_gain in zip(items, + album_gain.track_gains): + store_track_gain(item, track_gain) + store_album_gain(item, album_gain.album_gain) + if write: + item.try_write() + self._log.debug(u'done analyzing {0}', item) try: self._apply( @@ -1383,21 +1378,16 @@ class ReplayGainPlugin(BeetsPlugin): store_track_gain, store_album_gain, target_level, peak = tag_vals def _store_track(track_gains): - try: - if len(track_gains) != 1: - raise ReplayGainError( - u"ReplayGain backend failed for track {0}".format( - item) - ) + if len(track_gains) != 1: + raise ReplayGainError( + u"ReplayGain backend failed for track {0}".format( + item) + ) - store_track_gain(item, track_gains[0]) - if write: - item.try_write() - self._log.debug(u'done analyzing {0}', item) - except ReplayGainError as e: - self._log.info(u"ReplayGain error: {0}", e) - except FatalReplayGainError as e: - raise ui.UserError(u"Fatal ReplayGain error: {0}".format(e)) + store_track_gain(item, track_gains[0]) + if write: + item.try_write() + self._log.debug(u'done analyzing {0}', item) try: self._apply( @@ -1409,7 +1399,6 @@ class ReplayGainPlugin(BeetsPlugin): }, callback=_store_track ) - except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: @@ -1463,6 +1452,24 @@ class ReplayGainPlugin(BeetsPlugin): def commands(self): """Return the "replaygain" ui subcommand. """ + def func(lib, opts, args): + write = ui.should_write(opts.write) + force = opts.force + + # Bypass self.open_pool() if called with `--threads 0` + if opts.threads != 0: + threads = opts.threads or self.config['threads'].get(int) + self.open_pool(threads) + + if opts.album: + for album in lib.albums(ui.decargs(args)): + self.handle_album(album, write, force) + else: + for item in lib.items(ui.decargs(args)): + self.handle_track(item, write, force) + + self.close_pool() + cmd = ui.Subcommand('replaygain', help=u'analyze for ReplayGain') cmd.parser.add_album_option() cmd.parser.add_option( @@ -1480,24 +1487,5 @@ class ReplayGainPlugin(BeetsPlugin): cmd.parser.add_option( "-W", "--nowrite", dest="write", action="store_false", help=u"don't write metadata (opposite of -w)") - cmd.func = self.replaygain_func + cmd.func = func return [cmd] - - def replaygain_func(self, lib, opts, args): - """Handle `replaygain` ui subcommand - """ - write = ui.should_write(opts.write) - force = opts.force - - if opts.threads != 0: - threads = opts.threads or self.config['threads'].get(int) - self.open_pool(threads) - - if opts.album: - for album in lib.albums(ui.decargs(args)): - self.handle_album(album, write, force) - else: - for item in lib.items(ui.decargs(args)): - self.handle_track(item, write, force) - - self.close_pool() From b39df01d3553ee98bbb12e52ba641ac0e12c01e0 Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 20:02:51 +0100 Subject: [PATCH 10/89] Add to documentation --- docs/plugins/replaygain.rst | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index 9602618da..bda85bb05 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -13,12 +13,16 @@ Installation This plugin can use one of many backends to compute the ReplayGain values: GStreamer, mp3gain (and its cousin, aacgain), Python Audio Tools or ffmpeg. ffmpeg and mp3gain can be easier to install. mp3gain supports less audio formats -then the other backend. +than the other backend. Once installed, this plugin analyzes all files during the import process. This can be a slow process; to instead analyze after the fact, disable automatic analysis and use the ``beet replaygain`` command (see below). +To speed up analysis with some of the avalaible backends, this plugin processes +tracks or albums (when using the ``-a`` option) in parallel. By default, +a single thread is used per logical core of your CPU. + GStreamer ````````` @@ -35,6 +39,8 @@ the GStreamer backend by adding this to your configuration file:: replaygain: backend: gstreamer +The GStreamer backend does not support parallel analysis. + mp3gain and aacgain ``````````````````` @@ -73,6 +79,8 @@ On OS X, most of the dependencies can be installed with `Homebrew`_:: brew install mpg123 mp3gain vorbisgain faad2 libvorbis +The Python Audio Tools backend does not support parallel analysis. + .. _Python Audio Tools: http://audiotools.sourceforge.net ffmpeg @@ -92,6 +100,9 @@ configuration file. The available options are: - **auto**: Enable ReplayGain analysis during import. Default: ``yes``. +- **threads**: The number of parallel threads to run the analysis in. Overridden + by ``--threads`` at the command line. + Default: # of logical CPU cores - **backend**: The analysis backend; either ``gstreamer``, ``command``, ``audiotools`` or ``ffmpeg``. Default: ``command``. @@ -143,8 +154,15 @@ whether ReplayGain tags are written into the music files, or stored in the beets database only (the default is to use :ref:`the importer's configuration `). +To execute with a different number of threads, call ``beet replaygain --threads N``:: + + $ beet replaygain --threads N [-Waf] [QUERY] + +with N any integer. To disable parallelism, use ``--threads 0``. + ReplayGain analysis is not fast, so you may want to disable it during import. Use the ``auto`` config option to control this:: replaygain: auto: no + From 4a427182cddb6b4c84340c9b1ba30b1e6017f99c Mon Sep 17 00:00:00 2001 From: ybnd Date: Fri, 31 Jan 2020 13:50:38 +0100 Subject: [PATCH 11/89] Handle exceptions in pooled threads * ExceptionWatcher instance running in parallel to the pool, monitoring a queue for exceptions * Pooled threads push exceptions to this queue, log non-fatal exceptions * Application exits on fatal exception in pooled thread * More front end info logs in the CLI --- beetsplug/replaygain.py | 112 ++++++++++++++++++++++++++++++++++------ 1 file changed, 97 insertions(+), 15 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 2658d2dd3..01878a3d4 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -22,9 +22,11 @@ import math import sys import warnings import enum -from multiprocessing.pool import ThreadPool +from multiprocessing.pool import ThreadPool, RUN +from threading import Thread, Event import xml.parsers.expat -from six.moves import zip +from six.moves import zip, queue +import six from beets import ui from beets.plugins import BeetsPlugin @@ -1153,6 +1155,33 @@ class AudioToolsBackend(Backend): ) +class ExceptionWatcher(Thread): + """Monitors a queue for exceptions asynchronously. + Once an exception occurs, raise it and execute a callback. + """ + + def __init__(self, queue, callback): + self._queue = queue + self._callback = callback + self._stopevent = Event() + Thread.__init__(self) + + def run(self): + while not self._stopevent.is_set(): + try: + exc = self._queue.get_nowait() + self._callback() + six.reraise(exc[0], exc[1], exc[2]) + except queue.Empty: + # No exceptions yet, loop back to check + # whether `_stopevent` is set + pass + + def join(self, timeout=None): + self._stopevent.set() + Thread.join(self, timeout) + + # Main plugin logic. class ReplayGainPlugin(BeetsPlugin): @@ -1190,13 +1219,15 @@ class ReplayGainPlugin(BeetsPlugin): self.overwrite = self.config['overwrite'].get(bool) self.per_disc = self.config['per_disc'].get(bool) - backend_name = self.config['backend'].as_str() - if backend_name not in self.backends: + # Remember which backend is used for CLI feedback + self.backend_name = self.config['backend'].as_str() + + if self.backend_name not in self.backends: raise ui.UserError( u"Selected ReplayGain backend {0} is not supported. " u"Please select one of: {1}".format( - backend_name, + self.backend_name, u', '.join(self.backends.keys()) ) ) @@ -1221,7 +1252,7 @@ class ReplayGainPlugin(BeetsPlugin): self.r128_whitelist = self.config['r128'].as_str_seq() try: - self.backend_instance = self.backends[backend_name]( + self.backend_instance = self.backends[self.backend_name]( self.config, self._log ) except (ReplayGainError, FatalReplayGainError) as e: @@ -1334,10 +1365,15 @@ class ReplayGainPlugin(BeetsPlugin): for discnumber, items in discs.items(): def _store_album(album_gain): - if len(album_gain.track_gains) != len(items): + if not album_gain or not album_gain.album_gain \ + or len(album_gain.track_gains) != len(items): + # In some cases, backends fail to produce a valid + # `album_gain` without throwing FatalReplayGainError + # => raise non-fatal exception & continue raise ReplayGainError( - u"ReplayGain backend failed " - u"for some tracks in album {0}".format(album) + u"ReplayGain backend `{}` failed " + u"for some tracks in album {}" + .format(self.backend_name, album) ) for item, track_gain in zip(items, album_gain.track_gains): @@ -1378,10 +1414,13 @@ class ReplayGainPlugin(BeetsPlugin): store_track_gain, store_album_gain, target_level, peak = tag_vals def _store_track(track_gains): - if len(track_gains) != 1: + if not track_gains or len(track_gains) != 1: + # In some cases, backends fail to produce a valid + # `track_gains` without throwing FatalReplayGainError + # => raise non-fatal exception & continue raise ReplayGainError( - u"ReplayGain backend failed for track {0}".format( - item) + u"ReplayGain backend `{}` failed for track {}" + .format(self.backend_name, item) ) store_track_gain(item, track_gains[0]) @@ -1408,7 +1447,7 @@ class ReplayGainPlugin(BeetsPlugin): """Check whether a `ThreadPool` is running instance in `self.pool` """ if hasattr(self, 'pool'): - if isinstance(self.pool, ThreadPool) and self.pool._state == 'RUN': + if isinstance(self.pool, ThreadPool) and self.pool._state == RUN: return True return False @@ -1417,19 +1456,52 @@ class ReplayGainPlugin(BeetsPlugin): """ if not self._has_pool() and self.backend_instance.do_parallel: self.pool = ThreadPool(threads) + self.exc_queue = queue.Queue() + + self.exc_watcher = ExceptionWatcher( + self.exc_queue, # threads push exceptions here + self.terminate_pool # abort once an exception occurs + ) + self.exc_watcher.start() def _apply(self, func, args, kwds, callback): if self._has_pool(): + def catch_exc(func, exc_queue, log): + """Wrapper to catch raised exceptions in threads + """ + def wfunc(*args, **kwargs): + try: + return func(*args, **kwargs) + except ReplayGainError as e: + log.info(e.args[0]) # log non-fatal exceptions + except Exception: + exc_queue.put(sys.exc_info()) + return wfunc + + # Wrap function and callback to catch exceptions + func = catch_exc(func, self.exc_queue, self._log) + callback = catch_exc(callback, self.exc_queue, self._log) + self.pool.apply_async(func, args, kwds, callback) else: callback(func(*args, **kwds)) + def terminate_pool(self): + """Terminate the `ThreadPool` instance in `self.pool` + (e.g. stop execution in case of exception) + """ + if self._has_pool(): + self.pool.terminate() + self.pool.join() + self.exc_watcher.join() + def close_pool(self): """Close the `ThreadPool` instance in `self.pool` (if there is one) """ if self._has_pool(): self.pool.close() self.pool.join() + self.exc_watcher.join() def import_begin(self, session): """Handle `import_begin` event -> open pool @@ -1462,10 +1534,20 @@ class ReplayGainPlugin(BeetsPlugin): self.open_pool(threads) if opts.album: - for album in lib.albums(ui.decargs(args)): + albums = lib.albums(ui.decargs(args)) + self._log.info( + "Analyzing {} albums ~ {} backend..." + .format(len(albums), self.backend_name) + ) + for album in albums: self.handle_album(album, write, force) else: - for item in lib.items(ui.decargs(args)): + items = lib.items(ui.decargs(args)) + self._log.info( + "Analyzing {} tracks ~ {} backend..." + .format(len(items), self.backend_name) + ) + for item in items: self.handle_track(item, write, force) self.close_pool() From 4970585b0a22a6f131f6b377bfc13733d2a427b7 Mon Sep 17 00:00:00 2001 From: ybnd Date: Fri, 31 Jan 2020 14:08:28 +0100 Subject: [PATCH 12/89] Remove temporary workaround for silent exceptions --- beetsplug/replaygain.py | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 01878a3d4..1eb15724f 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -325,19 +325,9 @@ class Bs1770gainBackend(Backend): try: parser.Parse(text, True) except xml.parsers.expat.ExpatError: - # ReplayGainLdnsCliMalformedTest.test_malformed_output - # fails in Python 3.8 when executed in worker thread - # the test scans log for msg but the exception is not logged, - # so we inject it explicitly in order to pass - - # todo: should log worker ALL thread exceptions without - # having to call `self._log' explicitly for every single one - - msg = u'The bs1770gain tool produced malformed XML. ' \ - u'Using version >=0.4.10 may solve this problem.' - if sys.version >= '3.8': - self._log.info(msg) - raise ReplayGainError(msg) + raise ReplayGainError( + u'The bs1770gain tool produced malformed XML. ' + u'Using version >=0.4.10 may solve this problem.') if len(per_file_gain) != len(path_list): raise ReplayGainError( From 9bd78424c1a5d5e548370d277063437cb070415e Mon Sep 17 00:00:00 2001 From: ybnd Date: Tue, 4 Feb 2020 19:06:39 +0100 Subject: [PATCH 13/89] Handle keyboard interrupts more cleanly --- beetsplug/replaygain.py | 66 ++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 1eb15724f..cc7ce547e 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -24,6 +24,7 @@ import warnings import enum from multiprocessing.pool import ThreadPool, RUN from threading import Thread, Event +import signal import xml.parsers.expat from six.moves import zip, queue import six @@ -1448,6 +1449,8 @@ class ReplayGainPlugin(BeetsPlugin): self.pool = ThreadPool(threads) self.exc_queue = queue.Queue() + signal.signal(signal.SIGINT, self._interrupt) + self.exc_watcher = ExceptionWatcher( self.exc_queue, # threads push exceptions here self.terminate_pool # abort once an exception occurs @@ -1480,11 +1483,22 @@ class ReplayGainPlugin(BeetsPlugin): """Terminate the `ThreadPool` instance in `self.pool` (e.g. stop execution in case of exception) """ - if self._has_pool(): + # Don't call self._as_pool() here, + # self.pool._state may not be == RUN + if hasattr(self, 'pool') and isinstance(self.pool, ThreadPool): self.pool.terminate() self.pool.join() self.exc_watcher.join() + def _interrupt(self, signal, frame): + try: + self._log.info('interrupted') + self.terminate_pool() + exit(0) + except SystemExit: + # Silence raised SystemExit ~ exit(0) + pass + def close_pool(self): """Close the `ThreadPool` instance in `self.pool` (if there is one) """ @@ -1515,32 +1529,36 @@ class ReplayGainPlugin(BeetsPlugin): """Return the "replaygain" ui subcommand. """ def func(lib, opts, args): - write = ui.should_write(opts.write) - force = opts.force + try: + write = ui.should_write(opts.write) + force = opts.force - # Bypass self.open_pool() if called with `--threads 0` - if opts.threads != 0: - threads = opts.threads or self.config['threads'].get(int) - self.open_pool(threads) + # Bypass self.open_pool() if called with `--threads 0` + if opts.threads != 0: + threads = opts.threads or self.config['threads'].get(int) + self.open_pool(threads) - if opts.album: - albums = lib.albums(ui.decargs(args)) - self._log.info( - "Analyzing {} albums ~ {} backend..." - .format(len(albums), self.backend_name) - ) - for album in albums: - self.handle_album(album, write, force) - else: - items = lib.items(ui.decargs(args)) - self._log.info( - "Analyzing {} tracks ~ {} backend..." - .format(len(items), self.backend_name) - ) - for item in items: - self.handle_track(item, write, force) + if opts.album: + albums = lib.albums(ui.decargs(args)) + self._log.info( + "Analyzing {} albums ~ {} backend..." + .format(len(albums), self.backend_name) + ) + for album in albums: + self.handle_album(album, write, force) + else: + items = lib.items(ui.decargs(args)) + self._log.info( + "Analyzing {} tracks ~ {} backend..." + .format(len(items), self.backend_name) + ) + for item in items: + self.handle_track(item, write, force) - self.close_pool() + self.close_pool() + except (SystemExit, KeyboardInterrupt): + # Silence interrupt exceptions + pass cmd = ui.Subcommand('replaygain', help=u'analyze for ReplayGain') cmd.parser.add_album_option() From 9b283be7bf886ff64bd7331bfe7ac28e1210ed94 Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 5 Feb 2020 08:29:17 +0100 Subject: [PATCH 14/89] Add to changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 95fde6888..85bda3cab 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -107,6 +107,9 @@ New features: titles. Thanks to :user:`cole-miller`. :bug:`3459` +* :doc:`/plugins/replaygain` now does its analysis in parallel when using + the ``command``, ``ffmpeg`` or ``bs1770gain`` backends. + :bug:`3478` Fixes: From e943486da99df0ebbc6f707bbbfc8ea5c99732a8 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:11:30 +0200 Subject: [PATCH 15/89] first try at mocking get_work_by_id --- test/test_parentwork.py | 60 ++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index df6a98d79..fe4d663a4 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -23,6 +23,42 @@ from test.helper import TestHelper from beets.library import Item from beetsplug import parentwork +import musicbrainzngs + +work = {'work': {'id': '1', + 'work-relation-list': [{'type': 'parts', + 'direction': 'backwards', + 'id': '2'}], + 'artist-relation-list': [{'type': 'composer', + 'artist': {'name': + 'random composer', + 'sort-name': + 'composer, random'}}]}} +dp_work = {'work': {'id': '2', + 'work-relation-list': [{'type': 'parts', + 'direction': 'backwards', + 'id': '3'}], + 'artist-relation-list': [{'type': 'composer', + 'artist': {'name': + 'random composer', + 'sort-name': + 'composer, random' + }}]}} +p_work = {'work': {'id': '3', + 'artist-relation-list': [{'type': 'composer', + 'artist': {'name': + 'random composer', + 'sort-name': + 'composer, random'}}]}} + + +def mock_workid_response(mbid): + if mbid == '1': + return work + elif mbid == '2': + return dp_work + elif mbid == '3': + return p_work class ParentWorkTest(unittest.TestCase, TestHelper): @@ -35,19 +71,21 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() + @unittest.mock.patch('musicbrainzngs.get_work_by_id', + side_effect=mock_workid_response) @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') def test_normal_case(self): item = Item(path='/file', - mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53') + mb_workid='1') item.add(self.lib) self.run_command('parentwork') item.load() self.assertEqual(item['mb_parentworkid'], - u'32c8943f-1b27-3a23-8660-4567f4847c94') + '3') @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', @@ -55,7 +93,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): def test_force(self): self.config['parentwork']['force'] = True item = Item(path='/file', - mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', + mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) @@ -63,21 +101,20 @@ class ParentWorkTest(unittest.TestCase, TestHelper): item.load() self.assertEqual(item['mb_parentworkid'], - u'32c8943f-1b27-3a23-8660-4567f4847c94') + '3') @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') def test_no_force(self): self.config['parentwork']['force'] = True - item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-\ - b8ebc18e8c53', mb_parentworkid=u'XXX') + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) self.run_command('parentwork') item.load() - self.assertEqual(item['mb_parentworkid'], u'XXX') + self.assertEqual(item['mb_parentworkid'], '3') # test different cases, still with Matthew Passion Ouverture or Mozart # requiem @@ -86,11 +123,10 @@ class ParentWorkTest(unittest.TestCase, TestHelper): os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') def test_direct_parent_work(self): - mb_workid = u'2e4a3668-458d-3b2a-8be2-0b08e0d8243a' - self.assertEqual(u'f04b42df-7251-4d86-a5ee-67cfa49580d1', - parentwork.direct_parent_id(mb_workid)[0]) - self.assertEqual(u'45afb3b2-18ac-4187-bc72-beb1b1c194ba', - parentwork.work_parent_id(mb_workid)[0]) + self.assertEqual('2', + parentwork.direct_parent_id('1')[0]) + self.assertEqual('3', + parentwork.work_parent_id('1')[0]) def suite(): From 2d756df5b7d202c79e0798f26a4a2f5d14aab550 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:16:51 +0200 Subject: [PATCH 16/89] second try --- test/test_parentwork.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index fe4d663a4..809bffcbb 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -20,6 +20,7 @@ from __future__ import division, absolute_import, print_function import os import unittest from test.helper import TestHelper +import mock from beets.library import Item from beetsplug import parentwork @@ -71,8 +72,8 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() - @unittest.mock.patch('musicbrainzngs.get_work_by_id', - side_effect=mock_workid_response) + @mock.patch('musicbrainzngs.get_work_by_id', + side_effect=mock_workid_response) @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') From 94dd7cee7035c770c030b30bb90d6e77dac1045a Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:20:18 +0200 Subject: [PATCH 17/89] unused library --- test/test_parentwork.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 809bffcbb..6b5f93515 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -24,7 +24,6 @@ import mock from beets.library import Item from beetsplug import parentwork -import musicbrainzngs work = {'work': {'id': '1', 'work-relation-list': [{'type': 'parts', From 58b11d9d3c48c2fb17639736f734721fc3211fbf Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:34:24 +0200 Subject: [PATCH 18/89] make tests both with MB and without --- test/test_parentwork.py | 70 ++++++++++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 6b5f93515..4da7401a7 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -73,9 +73,6 @@ class ParentWorkTest(unittest.TestCase, TestHelper): @mock.patch('musicbrainzngs.get_work_by_id', side_effect=mock_workid_response) - @unittest.skipUnless( - os.environ.get('INTEGRATION_TEST', '0') == '1', - 'integration testing not enabled') def test_normal_case(self): item = Item(path='/file', mb_workid='1') @@ -87,9 +84,6 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.assertEqual(item['mb_parentworkid'], '3') - @unittest.skipUnless( - os.environ.get('INTEGRATION_TEST', '0') == '1', - 'integration testing not enabled') def test_force(self): self.config['parentwork']['force'] = True item = Item(path='/file', @@ -103,9 +97,6 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.assertEqual(item['mb_parentworkid'], '3') - @unittest.skipUnless( - os.environ.get('INTEGRATION_TEST', '0') == '1', - 'integration testing not enabled') def test_no_force(self): self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') @@ -119,15 +110,70 @@ class ParentWorkTest(unittest.TestCase, TestHelper): # test different cases, still with Matthew Passion Ouverture or Mozart # requiem - @unittest.skipUnless( - os.environ.get('INTEGRATION_TEST', '0') == '1', - 'integration testing not enabled') def test_direct_parent_work(self): self.assertEqual('2', parentwork.direct_parent_id('1')[0]) self.assertEqual('3', parentwork.work_parent_id('1')[0]) + # test how it works with real musicbrainz data + @unittest.skipUnless( + os.environ.get('INTEGRATION_TEST', '0') == '1', + 'integration testing not enabled') + def test_normal_case_real(self): + item = Item(path='/file', + mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], + u'32c8943f-1b27-3a23-8660-4567f4847c94') + + @unittest.skipUnless( + os.environ.get('INTEGRATION_TEST', '0') == '1', + 'integration testing not enabled') + def test_force_real(self): + self.config['parentwork']['force'] = True + item = Item(path='/file', + mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', + mb_parentworkid=u'XXX') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], + u'32c8943f-1b27-3a23-8660-4567f4847c94') + + @unittest.skipUnless( + os.environ.get('INTEGRATION_TEST', '0') == '1', + 'integration testing not enabled') + def test_no_force_real(self): + self.config['parentwork']['force'] = True + item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53', mb_parentworkid=u'XXX') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], u'XXX') + + # test different cases, still with Matthew Passion Ouverture or Mozart + # requiem + + @unittest.skipUnless( + os.environ.get('INTEGRATION_TEST', '0') == '1', + 'integration testing not enabled') + def test_direct_parent_work_real(self): + mb_workid = u'2e4a3668-458d-3b2a-8be2-0b08e0d8243a' + self.assertEqual(u'f04b42df-7251-4d86-a5ee-67cfa49580d1', + parentwork.direct_parent_id(mb_workid)[0]) + self.assertEqual(u'45afb3b2-18ac-4187-bc72-beb1b1c194ba', + parentwork.work_parent_id(mb_workid)[0]) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From d71ee6bc31545fad415464ac701a1a1363c23c9c Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:38:02 +0200 Subject: [PATCH 19/89] exchanged tests with and without MB --- test/test_parentwork.py | 90 ++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 4da7401a7..830d0cea8 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -71,51 +71,6 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() - @mock.patch('musicbrainzngs.get_work_by_id', - side_effect=mock_workid_response) - def test_normal_case(self): - item = Item(path='/file', - mb_workid='1') - item.add(self.lib) - - self.run_command('parentwork') - - item.load() - self.assertEqual(item['mb_parentworkid'], - '3') - - def test_force(self): - self.config['parentwork']['force'] = True - item = Item(path='/file', - mb_workid='1', - mb_parentworkid=u'XXX') - item.add(self.lib) - - self.run_command('parentwork') - - item.load() - self.assertEqual(item['mb_parentworkid'], - '3') - - def test_no_force(self): - self.config['parentwork']['force'] = True - item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') - item.add(self.lib) - - self.run_command('parentwork') - - item.load() - self.assertEqual(item['mb_parentworkid'], '3') - - # test different cases, still with Matthew Passion Ouverture or Mozart - # requiem - - def test_direct_parent_work(self): - self.assertEqual('2', - parentwork.direct_parent_id('1')[0]) - self.assertEqual('3', - parentwork.work_parent_id('1')[0]) - # test how it works with real musicbrainz data @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', @@ -174,6 +129,51 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.assertEqual(u'45afb3b2-18ac-4187-bc72-beb1b1c194ba', parentwork.work_parent_id(mb_workid)[0]) + @mock.patch('musicbrainzngs.get_work_by_id', + side_effect=mock_workid_response) + def test_normal_case(self): + item = Item(path='/file', + mb_workid='1') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], + '3') + + def test_force(self): + self.config['parentwork']['force'] = True + item = Item(path='/file', + mb_workid='1', + mb_parentworkid=u'XXX') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], + '3') + + def test_no_force(self): + self.config['parentwork']['force'] = True + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], '3') + + # test different cases, still with Matthew Passion Ouverture or Mozart + # requiem + + def test_direct_parent_work(self): + self.assertEqual('2', + parentwork.direct_parent_id('1')[0]) + self.assertEqual('3', + parentwork.work_parent_id('1')[0]) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From dba2e2320da48a653aaebcc7cb61b982fe867f99 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:41:10 +0200 Subject: [PATCH 20/89] hybrid mocking --- test/test_parentwork.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 830d0cea8..7db3acdcb 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -21,6 +21,7 @@ import os import unittest from test.helper import TestHelper import mock +import musicbrainzngs from beets.library import Item from beetsplug import parentwork @@ -59,6 +60,8 @@ def mock_workid_response(mbid): return dp_work elif mbid == '3': return p_work + else: + return musicbrainzngs.get_work_by_id(mbid) class ParentWorkTest(unittest.TestCase, TestHelper): From 1cb23e5c5c1953f5f0ad91784b5a19d530c08ba6 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:48:06 +0200 Subject: [PATCH 21/89] no hybrid mocking, two classes --- test/test_parentwork.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 7db3acdcb..395e6c107 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -60,8 +60,6 @@ def mock_workid_response(mbid): return dp_work elif mbid == '3': return p_work - else: - return musicbrainzngs.get_work_by_id(mbid) class ParentWorkTest(unittest.TestCase, TestHelper): @@ -132,6 +130,17 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.assertEqual(u'45afb3b2-18ac-4187-bc72-beb1b1c194ba', parentwork.work_parent_id(mb_workid)[0]) + +class ParentWorkTest_mock(unittest.TestCase, TestHelper): + def setUp(self): + """Set up configuration""" + self.setup_beets() + self.load_plugins('parentwork') + + def tearDown(self): + self.unload_plugins() + self.teardown_beets() + @mock.patch('musicbrainzngs.get_work_by_id', side_effect=mock_workid_response) def test_normal_case(self): From 82411c67cf32d1526844ba56ad519dc1d3c54773 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 21:51:45 +0200 Subject: [PATCH 22/89] minor fixes --- test/test_parentwork.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 395e6c107..da61fb007 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -21,7 +21,6 @@ import os import unittest from test.helper import TestHelper import mock -import musicbrainzngs from beets.library import Item from beetsplug import parentwork @@ -175,10 +174,7 @@ class ParentWorkTest_mock(unittest.TestCase, TestHelper): self.run_command('parentwork') item.load() - self.assertEqual(item['mb_parentworkid'], '3') - - # test different cases, still with Matthew Passion Ouverture or Mozart - # requiem + self.assertEqual(item['mb_parentworkid'], u'XXX') def test_direct_parent_work(self): self.assertEqual('2', From a4a9f0daa976e2e5c1afc82be5aa6ffcca6842c3 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:12:13 +0200 Subject: [PATCH 23/89] different way of mocking --- test/test_parentwork.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index da61fb007..566748a03 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -20,7 +20,8 @@ from __future__ import division, absolute_import, print_function import os import unittest from test.helper import TestHelper -import mock +from mock import Mock +import musicbrainzngs from beets.library import Item from beetsplug import parentwork @@ -130,7 +131,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): parentwork.work_parent_id(mb_workid)[0]) -class ParentWorkTest_mock(unittest.TestCase, TestHelper): +class ParentWorkMockTest(unittest.TestCase, TestHelper): def setUp(self): """Set up configuration""" self.setup_beets() @@ -140,8 +141,7 @@ class ParentWorkTest_mock(unittest.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() - @mock.patch('musicbrainzngs.get_work_by_id', - side_effect=mock_workid_response) + musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def test_normal_case(self): item = Item(path='/file', mb_workid='1') From df918136092d743d44d4950d74afaeabcde806a4 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:13:49 +0200 Subject: [PATCH 24/89] pep-8 --- test/test_parentwork.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 566748a03..163d59089 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -142,6 +142,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): self.teardown_beets() musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) + def test_normal_case(self): item = Item(path='/file', mb_workid='1') From 55995a32cb157178e8d712ee12f944362d39f308 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:17:24 +0200 Subject: [PATCH 25/89] correct force check, correct arguments for get_work_by_id --- test/test_parentwork.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 163d59089..fb28121e1 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -53,7 +53,7 @@ p_work = {'work': {'id': '3', 'composer, random'}}]}} -def mock_workid_response(mbid): +def mock_workid_response(mbid, includes): if mbid == '1': return work elif mbid == '2': @@ -107,7 +107,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') def test_no_force_real(self): - self.config['parentwork']['force'] = True + self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-\ b8ebc18e8c53', mb_parentworkid=u'XXX') item.add(self.lib) @@ -168,7 +168,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): '3') def test_no_force(self): - self.config['parentwork']['force'] = True + self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) From f87f271484845a6d013dd2e83b0c8be179c75a1f Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:22:16 +0200 Subject: [PATCH 26/89] correct mock results of get_work_by_id --- test/test_parentwork.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index fb28121e1..2a01473ea 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -29,7 +29,7 @@ from beetsplug import parentwork work = {'work': {'id': '1', 'work-relation-list': [{'type': 'parts', 'direction': 'backwards', - 'id': '2'}], + 'work': {'id': '2'}}], 'artist-relation-list': [{'type': 'composer', 'artist': {'name': 'random composer', @@ -38,7 +38,7 @@ work = {'work': {'id': '1', dp_work = {'work': {'id': '2', 'work-relation-list': [{'type': 'parts', 'direction': 'backwards', - 'id': '3'}], + 'work': {'id': '3'}}], 'artist-relation-list': [{'type': 'composer', 'artist': {'name': 'random composer', From ab2d653d82e3b084e6f954a629aa05204417d6ec Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:26:57 +0200 Subject: [PATCH 27/89] typo --- test/test_parentwork.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 2a01473ea..c5a5d250b 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -28,7 +28,7 @@ from beetsplug import parentwork work = {'work': {'id': '1', 'work-relation-list': [{'type': 'parts', - 'direction': 'backwards', + 'direction': 'backward', 'work': {'id': '2'}}], 'artist-relation-list': [{'type': 'composer', 'artist': {'name': @@ -37,7 +37,7 @@ work = {'work': {'id': '1', 'composer, random'}}]}} dp_work = {'work': {'id': '2', 'work-relation-list': [{'type': 'parts', - 'direction': 'backwards', + 'direction': 'backward', 'work': {'id': '3'}}], 'artist-relation-list': [{'type': 'composer', 'artist': {'name': From ccd44bc57c5f19b3d294a22d824da6b94592b4ac Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:31:51 +0200 Subject: [PATCH 28/89] complete mocked parentworks --- test/test_parentwork.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index c5a5d250b..63ff4f376 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -27,6 +27,7 @@ from beets.library import Item from beetsplug import parentwork work = {'work': {'id': '1', + 'title': 'work', 'work-relation-list': [{'type': 'parts', 'direction': 'backward', 'work': {'id': '2'}}], @@ -36,6 +37,7 @@ work = {'work': {'id': '1', 'sort-name': 'composer, random'}}]}} dp_work = {'work': {'id': '2', + 'title': 'directparentwork', 'work-relation-list': [{'type': 'parts', 'direction': 'backward', 'work': {'id': '3'}}], @@ -46,6 +48,7 @@ dp_work = {'work': {'id': '2', 'composer, random' }}]}} p_work = {'work': {'id': '3', + 'title': 'parentwork', 'artist-relation-list': [{'type': 'composer', 'artist': {'name': 'random composer', From 66bb2adf19e367170a1796d7cf4e657a124d80a1 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:40:34 +0200 Subject: [PATCH 29/89] try something --- test/test_parentwork.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 63ff4f376..e7c9d43ba 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -147,31 +147,26 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def test_normal_case(self): - item = Item(path='/file', - mb_workid='1') + item = Item(path='/file', mb_workid='1') item.add(self.lib) self.run_command('parentwork') item.load() - self.assertEqual(item['mb_parentworkid'], - '3') + self.assertEqual(item['mb_parentworkid'], '3') def test_force(self): self.config['parentwork']['force'] = True - item = Item(path='/file', - mb_workid='1', - mb_parentworkid=u'XXX') + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) self.run_command('parentwork') item.load() - self.assertEqual(item['mb_parentworkid'], - '3') + self.assertEqual(item['mb_parentworkid'], '3') def test_no_force(self): - self.config['parentwork']['force'] = False + self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) @@ -181,10 +176,8 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): self.assertEqual(item['mb_parentworkid'], u'XXX') def test_direct_parent_work(self): - self.assertEqual('2', - parentwork.direct_parent_id('1')[0]) - self.assertEqual('3', - parentwork.work_parent_id('1')[0]) + self.assertEqual('2', parentwork.direct_parent_id('1')[0]) + self.assertEqual('3', parentwork.work_parent_id('1')[0]) def suite(): From e3fb243782f97adacfde60340a75f7f4a4bde3f7 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:44:49 +0200 Subject: [PATCH 30/89] parentwork force test doesn't work --- test/test_parentwork.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index e7c9d43ba..da18219d5 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -156,17 +156,16 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): self.assertEqual(item['mb_parentworkid'], '3') def test_force(self): - self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) - self.run_command('parentwork') + self.run_command('parentwork -f') item.load() self.assertEqual(item['mb_parentworkid'], '3') def test_no_force(self): - self.config['parentwork']['force'] = True + self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') item.add(self.lib) From 8167870a7f25245037113a7ae0e301e0c41ca045 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:52:31 +0200 Subject: [PATCH 31/89] correct test for parentwork_id_current --- test/test_parentwork.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index da18219d5..1309cd363 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -81,7 +81,9 @@ class ParentWorkTest(unittest.TestCase, TestHelper): 'integration testing not enabled') def test_normal_case_real(self): item = Item(path='/file', - mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53') + mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', + parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53') item.add(self.lib) self.run_command('parentwork') @@ -97,7 +99,9 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', - mb_parentworkid=u'XXX') + mb_parentworkid=u'XXX', + parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53') item.add(self.lib) self.run_command('parentwork') @@ -112,7 +116,9 @@ class ParentWorkTest(unittest.TestCase, TestHelper): def test_no_force_real(self): self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-\ - b8ebc18e8c53', mb_parentworkid=u'XXX') + b8ebc18e8c53', mb_parentworkid=u'XXX', + parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53') item.add(self.lib) self.run_command('parentwork') @@ -147,7 +153,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def test_normal_case(self): - item = Item(path='/file', mb_workid='1') + item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') item.add(self.lib) self.run_command('parentwork') @@ -156,17 +162,20 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): self.assertEqual(item['mb_parentworkid'], '3') def test_force(self): - item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') + self.config['parentwork']['force'] = True + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', + parentwork_workid_current='1') item.add(self.lib) - self.run_command('parentwork -f') + self.run_command('parentwork') item.load() self.assertEqual(item['mb_parentworkid'], '3') def test_no_force(self): self.config['parentwork']['force'] = False - item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX') + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', + parentwork_workid_current='1') item.add(self.lib) self.run_command('parentwork') From 08c1344639d0b1bee5e9864d49d5c2983cc00847 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 22:57:05 +0200 Subject: [PATCH 32/89] correct test for parentwork --- test/test_parentwork.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 1309cd363..f595e7770 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -164,7 +164,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): def test_force(self): self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', - parentwork_workid_current='1') + parentwork_workid_current='1', mb_parentwork='parentwork') item.add(self.lib) self.run_command('parentwork') @@ -175,7 +175,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): def test_no_force(self): self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', - parentwork_workid_current='1') + parentwork_workid_current='1', mb_parentwork='parentwork') item.add(self.lib) self.run_command('parentwork') From ef08fbee0cbb087e27aa03b3a22b2a5b6ca4943c Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 23:01:18 +0200 Subject: [PATCH 33/89] correct test for parentwork --- test/test_parentwork.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index f595e7770..98b07322d 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -101,7 +101,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', mb_parentworkid=u'XXX', parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ - b8ebc18e8c53') + b8ebc18e8c53', parentwork='whatever') item.add(self.lib) self.run_command('parentwork') @@ -118,7 +118,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-\ b8ebc18e8c53', mb_parentworkid=u'XXX', parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ - b8ebc18e8c53') + b8ebc18e8c53', parentwork='whatever') item.add(self.lib) self.run_command('parentwork') @@ -164,7 +164,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): def test_force(self): self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', - parentwork_workid_current='1', mb_parentwork='parentwork') + parentwork_workid_current='1', parentwork='parentwork') item.add(self.lib) self.run_command('parentwork') @@ -175,7 +175,7 @@ class ParentWorkMockTest(unittest.TestCase, TestHelper): def test_no_force(self): self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', - parentwork_workid_current='1', mb_parentwork='parentwork') + parentwork_workid_current='1', parentwork='parentwork') item.add(self.lib) self.run_command('parentwork') From 770664f27d097733a25280ac31013ed61faae9de Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 13 Jul 2020 23:05:53 +0200 Subject: [PATCH 34/89] typo --- beetsplug/parentwork.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/parentwork.py b/beetsplug/parentwork.py index b3e464e60..85f8b0839 100644 --- a/beetsplug/parentwork.py +++ b/beetsplug/parentwork.py @@ -96,7 +96,7 @@ class ParentWorkPlugin(BeetsPlugin): item.try_write() command = ui.Subcommand( 'parentwork', - help=u'fetche parent works, composers and dates') + help=u'fetch parent works, composers and dates') command.parser.add_option( u'-f', u'--force', dest='force', From 72e91d84f28630310b7077d83fde2178f2361f72 Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 14 Jul 2020 00:01:22 +0200 Subject: [PATCH 35/89] names --- test/test_parentwork.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 98b07322d..0570afd35 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -65,7 +65,7 @@ def mock_workid_response(mbid, includes): return p_work -class ParentWorkTest(unittest.TestCase, TestHelper): +class ParentWorkIntegrationTest(unittest.TestCase, TestHelper): def setUp(self): """Set up configuration""" self.setup_beets() @@ -140,7 +140,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): parentwork.work_parent_id(mb_workid)[0]) -class ParentWorkMockTest(unittest.TestCase, TestHelper): +class ParentWorkTest(unittest.TestCase, TestHelper): def setUp(self): """Set up configuration""" self.setup_beets() From 7fb3c24c102dea06d62893f381874222e1652030 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Fri, 28 Jul 2017 16:55:53 +0200 Subject: [PATCH 36/89] Add reflink to setup requirements and config. --- beets/config_default.yaml | 1 + beets/importer.py | 17 +++++++++++++++-- beets/util/__init__.py | 2 ++ setup.py | 2 ++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 0fd6eb592..892a5a336 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -7,6 +7,7 @@ import: move: no link: no hardlink: no + reflink: no delete: no resume: ask incremental: no diff --git a/beets/importer.py b/beets/importer.py index 68d5f3d5d..9c86b21c6 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -222,19 +222,30 @@ class ImportSession(object): iconfig['resume'] = False iconfig['incremental'] = False - # Copy, move, link, and hardlink are mutually exclusive. + if iconfig['reflink']: + iconfig['reflink'] = iconfig['reflink'].as_choice(['auto', True, False]) + + # Copy, move, reflink, link, and hardlink are mutually exclusive. if iconfig['move']: iconfig['copy'] = False iconfig['link'] = False iconfig['hardlink'] = False + iconfig['reflink'] = False elif iconfig['link']: iconfig['copy'] = False iconfig['move'] = False iconfig['hardlink'] = False + iconfig['reflink'] = False elif iconfig['hardlink']: iconfig['copy'] = False iconfig['move'] = False iconfig['link'] = False + iconfig['reflink'] = False + elif iconfig['reflink']: + iconfig['copy'] = False + iconfig['move'] = False + iconfig['link'] = False + iconfig['hardlink'] = False # Only delete when copying. if not iconfig['copy']: @@ -707,7 +718,7 @@ class ImportTask(BaseImportTask): item.update(changes) def manipulate_files(self, operation=None, write=False, session=None): - """ Copy, move, link or hardlink (depending on `operation`) the files + """ Copy, move, link, hardlink or reflink (depending on `operation`) the files as well as write metadata. `operation` should be an instance of `util.MoveOperation`. @@ -1536,6 +1547,8 @@ def manipulate_files(session, task): operation = MoveOperation.LINK elif session.config['hardlink']: operation = MoveOperation.HARDLINK + elif session.config['reflink']: + operation = MoveOperation.REFLINK else: operation = None diff --git a/beets/util/__init__.py b/beets/util/__init__.py index bb84aedc7..90f12e639 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -134,6 +134,8 @@ class MoveOperation(Enum): COPY = 1 LINK = 2 HARDLINK = 3 + REFLINK = 4 + REFLINK_AUTO = 5 def normpath(path): diff --git a/setup.py b/setup.py index 0e2cb332a..55714654e 100755 --- a/setup.py +++ b/setup.py @@ -93,6 +93,7 @@ setup( 'pyyaml', 'mediafile>=0.2.0', 'confuse>=1.0.0', + 'reflink', ] + [ # Avoid a version of munkres incompatible with Python 3. 'munkres~=1.0.0' if sys.version_info < (3, 5, 0) else @@ -123,6 +124,7 @@ setup( 'rarfile', 'responses>=0.3.0', 'requests_oauthlib', + 'reflink', ] + ( # Tests for the thumbnails plugin need pathlib on Python 2 too. ['pathlib'] if (sys.version_info < (3, 4, 0)) else [] From 2926b4962835e651cf2a0dde8115c59653e95350 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Fri, 28 Jul 2017 17:37:09 +0200 Subject: [PATCH 37/89] Add HAVE_REFLINK flag for tests --- test/_common.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/_common.py b/test/_common.py index 8e3b1dd18..e44fac48b 100644 --- a/test/_common.py +++ b/test/_common.py @@ -25,6 +25,8 @@ import six import unittest from contextlib import contextmanager +import reflink + # Mangle the search path to include the beets sources. sys.path.insert(0, '..') @@ -55,6 +57,7 @@ _item_ident = 0 # OS feature test. HAVE_SYMLINK = sys.platform != 'win32' HAVE_HARDLINK = sys.platform != 'win32' +HAVE_REFLINK = reflink.supported_at(tempfile.gettempdir()) def item(lib=None): From 5e2856ef87dd31fc73c7f852b230134beced1920 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Fri, 28 Jul 2017 16:51:19 +0200 Subject: [PATCH 38/89] Add reflink routine --- beets/importer.py | 3 ++- beets/library.py | 20 ++++++++++++++++++++ beets/util/__init__.py | 23 +++++++++++++++++++++++ test/test_files.py | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/beets/importer.py b/beets/importer.py index 9c86b21c6..3220b260f 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -223,7 +223,8 @@ class ImportSession(object): iconfig['incremental'] = False if iconfig['reflink']: - iconfig['reflink'] = iconfig['reflink'].as_choice(['auto', True, False]) + iconfig['reflink'] = iconfig['reflink'] \ + .as_choice(['auto', True, False]) # Copy, move, reflink, link, and hardlink are mutually exclusive. if iconfig['move']: diff --git a/beets/library.py b/beets/library.py index e22d4edc0..dea2a937e 100644 --- a/beets/library.py +++ b/beets/library.py @@ -747,6 +747,20 @@ class Item(LibModel): util.hardlink(self.path, dest) plugins.send("item_hardlinked", item=self, source=self.path, destination=dest) + elif operation == MoveOperation.REFLINK: + util.reflink(self.path, dest, fallback=False) + plugins.send("item_reflinked", item=self, source=self.path, + destination=dest) + elif operation == MoveOperation.REFLINK_AUTO: + util.reflink(self.path, dest, fallback=True) + plugins.send("item_reflinked", item=self, source=self.path, + destination=dest) + else: + plugins.send("before_item_moved", item=self, source=self.path, + destination=dest) + util.move(self.path, dest) + plugins.send("item_moved", item=self, source=self.path, + destination=dest) # Either copying or moving succeeded, so update the stored path. self.path = dest @@ -1087,6 +1101,12 @@ class Album(LibModel): util.link(old_art, new_art) elif operation == MoveOperation.HARDLINK: util.hardlink(old_art, new_art) + elif operation == MoveOperation.REFLINK: + util.reflink(old_art, new_art, fallback=False) + elif operation == MoveOperation.REFLINK_AUTO: + util.reflink(old_art, new_art, fallback=True) + else: + util.move(old_art, new_art) self.artpath = new_art def move(self, operation=MoveOperation.MOVE, basedir=None, store=True): diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 90f12e639..3bd2a7649 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -34,6 +34,7 @@ from beets.util import hidden import six from unidecode import unidecode from enum import Enum +import reflink as pyreflink MAX_FILENAME_LENGTH = 200 @@ -547,6 +548,28 @@ def hardlink(path, dest, replace=False): traceback.format_exc()) +def reflink(path, dest, replace=False, fallback=False): + """Create a reflink from `dest` to `path`. Raises an `OSError` if + `dest` already exists, unless `replace` is True. Does nothing if + `path` == `dest`. When `fallback` is True, `reflink` falls back on + `copy` when the filesystem does not support reflinks. + """ + if samefile(path, dest): + return + + if os.path.exists(syspath(dest)) and not replace: + raise FilesystemError(u'file exists', 'rename', (path, dest)) + + try: + pyreflink.reflink(path, dest) + except (NotImplementedError, pyreflink.ReflinkImpossibleError) as exc: + if fallback: + copy(path, dest, replace) + else: + raise FilesystemError(u'OS/filesystem does not support reflinks.', + 'link', (path, dest), traceback.format_exc()) + + def unique_path(path): """Returns a version of ``path`` that does not exist on the filesystem. Specifically, if ``path` itself already exists, then diff --git a/test/test_files.py b/test/test_files.py index f31779672..e9aee3f5b 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -86,6 +86,24 @@ class MoveTest(_common.TestCase): self.i.move(operation=MoveOperation.COPY) self.assertExists(self.path) + def test_reflink_arrives(self): + self.i.move(operation=MoveOperation.REFLINK_AUTO) + self.assertExists(self.dest) + + def test_reflink_does_not_depart(self): + self.i.move(operation=MoveOperation.REFLINK_AUTO) + self.assertExists(self.path) + + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_force_reflink_arrives(self): + self.i.move(operation=MoveOperation.REFLINK) + self.assertExists(self.dest) + + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_force_reflink_does_not_depart(self): + self.i.move(operation=MoveOperation.REFLINK) + self.assertExists(self.path) + def test_move_changes_path(self): self.i.move() self.assertEqual(self.i.path, util.normpath(self.dest)) @@ -249,6 +267,17 @@ class AlbumFileTest(_common.TestCase): self.assertTrue(os.path.exists(oldpath)) self.assertTrue(os.path.exists(self.i.path)) + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_albuminfo_move_reflinks_file(self): + oldpath = self.i.path + self.ai.album = u'newAlbumName' + self.ai.move(operation=MoveOperation.REFLINK) + self.ai.store() + self.i.load() + + self.assertTrue(os.path.exists(oldpath)) + self.assertTrue(os.path.exists(self.i.path)) + def test_albuminfo_move_to_custom_dir(self): self.ai.move(basedir=self.otherdir) self.i.load() @@ -530,6 +559,12 @@ class SafeMoveCopyTest(_common.TestCase): self.assertExists(self.dest) self.assertExists(self.path) + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_successful_reflink(self): + util.reflink(self.path, self.dest) + self.assertExists(self.dest) + self.assertExists(self.path) + def test_unsuccessful_move(self): with self.assertRaises(util.FilesystemError): util.move(self.path, self.otherpath) @@ -538,6 +573,11 @@ class SafeMoveCopyTest(_common.TestCase): with self.assertRaises(util.FilesystemError): util.copy(self.path, self.otherpath) + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_unsuccessful_reflink(self): + with self.assertRaises(util.FilesystemError): + util.reflink(self.path, self.otherpath) + def test_self_move(self): util.move(self.path, self.path) self.assertExists(self.path) From e1def7559ed598b280b8c8f0b19a0b62195142b0 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Sun, 30 Jul 2017 17:12:04 +0200 Subject: [PATCH 39/89] Add reflink docs --- docs/changelog.rst | 3 +++ docs/dev/plugins.rst | 4 ++++ docs/reference/config.rst | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0f41c38ec..81ca994a8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,6 +8,9 @@ New features: * :doc:`/plugins/lastgenre`: Added more heavy metal genres: https://en.wikipedia.org/wiki/Heavy_metal_genres to genres.txt and genres-tree.yaml * :doc:`/plugins/subsonicplaylist`: import playlist from a subsonic server. +* A new :ref:`reflink` config option instructs the importer to create reflinks + on filesystems that support them. Thanks to :user:`rubdos`. + :bug:`2642` * A new :ref:`extra_tags` configuration option allows more tagged metadata to be included in MusicBrainz queries. * A new :doc:`/plugins/fish` adds `Fish shell`_ tab autocompletion to beets diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 3328654e0..3ead4f860 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -164,6 +164,10 @@ The events currently available are: created for a file. Parameters: ``item``, ``source`` path, ``destination`` path +* `item_reflinked`: called with an ``Item`` object whenever a reflink is + created for a file. + Parameters: ``item``, ``source`` path, ``destination`` path + * `item_removed`: called with an ``Item`` object every time an item (singleton or album's part) is removed from the library (even when its file is not deleted from disk). diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 46f14f2c5..c6e319a43 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -483,6 +483,27 @@ As with symbolic links (see :ref:`link`, above), this will not work on Windows and you will want to set ``write`` to ``no``. Otherwise, metadata on the original file will be modified. +.. _reflink: + +reflink +~~~~~~~ + +Either ``yes``, ``no`` or ``auto``, indicating whether to `reflink clone`_ files +into the library directory when using ``beet import``. Defaults to ``no``. +When ``auto`` is specified, ``reflink`` will fall back on ``copy``, +in case that ``reflink``'s are not supported on the used filesystem. + + +The option is ignored if ``move`` is enabled (i.e., beets can move or +copy files but it doesn't make sense to do both). + + +The option is filesystem dependent. For filesystem support, refer to the +`pyreflink`_ documentation. + +.. _reflink clone: https://blogs.oracle.com/otn/save-disk-space-on-linux-by-cloning-files-on-btrfs-and-ocfs2 +.. _pyreflink: https://reflink.readthedocs.io/en/latest/ + resume ~~~~~~ From 43f27506bfb409564ec2c5d10d008163e2db5a89 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Mon, 31 Jul 2017 17:00:11 +0200 Subject: [PATCH 40/89] Make reflink optional --- beets/util/__init__.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 3bd2a7649..425e7ac87 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -34,7 +34,6 @@ from beets.util import hidden import six from unidecode import unidecode from enum import Enum -import reflink as pyreflink MAX_FILENAME_LENGTH = 200 @@ -554,6 +553,7 @@ def reflink(path, dest, replace=False, fallback=False): `path` == `dest`. When `fallback` is True, `reflink` falls back on `copy` when the filesystem does not support reflinks. """ + import reflink as pyreflink if samefile(path, dest): return diff --git a/setup.py b/setup.py index 55714654e..ac7ebc2a3 100755 --- a/setup.py +++ b/setup.py @@ -93,7 +93,6 @@ setup( 'pyyaml', 'mediafile>=0.2.0', 'confuse>=1.0.0', - 'reflink', ] + [ # Avoid a version of munkres incompatible with Python 3. 'munkres~=1.0.0' if sys.version_info < (3, 5, 0) else @@ -161,6 +160,7 @@ setup( 'scrub': ['mutagen>=1.33'], 'bpd': ['PyGObject'], 'replaygain': ['PyGObject'], + 'reflink': ['reflink'], }, # Non-Python/non-PyPI plugin dependencies: # chroma: chromaprint or fpcalc From b78c510ff29434383c2dca4125a1b11f8da489c5 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 29 Oct 2017 15:57:01 -0400 Subject: [PATCH 41/89] Expand docstring for reflink utility --- beets/util/__init__.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 425e7ac87..090151df2 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -548,12 +548,18 @@ def hardlink(path, dest, replace=False): def reflink(path, dest, replace=False, fallback=False): - """Create a reflink from `dest` to `path`. Raises an `OSError` if - `dest` already exists, unless `replace` is True. Does nothing if - `path` == `dest`. When `fallback` is True, `reflink` falls back on - `copy` when the filesystem does not support reflinks. + """Create a reflink from `dest` to `path`. + + Raise an `OSError` if `dest` already exists, unless `replace` is + True. If `path` == `dest`, then do nothing. + + If reflinking fails and `fallback` is enabled, try copying the file + instead. Otherwise, raise an error without trying a plain copy. + + May raise an `ImportError` if the `reflink` module is not available. """ import reflink as pyreflink + if samefile(path, dest): return From 39827394ae600735da328eda580b3947091edf3c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 29 Oct 2017 15:57:27 -0400 Subject: [PATCH 42/89] Expand the reflink changelog entry --- docs/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 81ca994a8..5a631f582 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,9 +8,9 @@ New features: * :doc:`/plugins/lastgenre`: Added more heavy metal genres: https://en.wikipedia.org/wiki/Heavy_metal_genres to genres.txt and genres-tree.yaml * :doc:`/plugins/subsonicplaylist`: import playlist from a subsonic server. -* A new :ref:`reflink` config option instructs the importer to create reflinks - on filesystems that support them. Thanks to :user:`rubdos`. - :bug:`2642` +* A new :ref:`reflink` config option instructs the importer to create fast, + copy-on-write file clones on filesystems that support them. Thanks to + :user:`rubdos`. * A new :ref:`extra_tags` configuration option allows more tagged metadata to be included in MusicBrainz queries. * A new :doc:`/plugins/fish` adds `Fish shell`_ tab autocompletion to beets From e7597916a2e9c24da923ad1a921327ad4b896a04 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 29 Oct 2017 16:02:25 -0400 Subject: [PATCH 43/89] Revise reflink docs --- docs/reference/config.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index c6e319a43..ffc3a5c32 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -476,7 +476,7 @@ hardlink ~~~~~~~~ Either ``yes`` or ``no``, indicating whether to use hard links instead of -moving or copying or symlinking files. (It conflicts with the ``move``, +moving, copying, or symlinking files. (It conflicts with the ``move``, ``copy``, and ``link`` options.) Defaults to ``no``. As with symbolic links (see :ref:`link`, above), this will not work on Windows @@ -488,19 +488,19 @@ original file will be modified. reflink ~~~~~~~ -Either ``yes``, ``no`` or ``auto``, indicating whether to `reflink clone`_ files -into the library directory when using ``beet import``. Defaults to ``no``. -When ``auto`` is specified, ``reflink`` will fall back on ``copy``, -in case that ``reflink``'s are not supported on the used filesystem. +Either ``yes``, ``no``, or ``auto``, indicating whether to use copy-on-write +file clones (a.k.a. "reflinks") instead of copying or moving files. +The ``auto`` option uses reflinks when possible and falls back to plain +copying when necessary. +Defaults to ``no``. +This kind of clone is only available on certain filesystems: for example, +btrfs and APFS. For more details on filesystem support, see the `pyreflink`_ +documentation. The option is ignored if ``move`` is enabled (i.e., beets can move or copy files but it doesn't make sense to do both). - -The option is filesystem dependent. For filesystem support, refer to the -`pyreflink`_ documentation. - .. _reflink clone: https://blogs.oracle.com/otn/save-disk-space-on-linux-by-cloning-files-on-btrfs-and-ocfs2 .. _pyreflink: https://reflink.readthedocs.io/en/latest/ From 99cd7e2de457af1b05a2974fee975b48f15518dc Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Wed, 22 Jul 2020 18:09:33 +0200 Subject: [PATCH 44/89] Fixup flake8 --- beets/util/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 090151df2..d2eca4963 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -568,7 +568,7 @@ def reflink(path, dest, replace=False, fallback=False): try: pyreflink.reflink(path, dest) - except (NotImplementedError, pyreflink.ReflinkImpossibleError) as exc: + except (NotImplementedError, pyreflink.ReflinkImpossibleError): if fallback: copy(path, dest, replace) else: From 14cfad7bd2ffacf1bdd532869e65123e1f3d3531 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Thu, 23 Jul 2020 09:55:32 +0200 Subject: [PATCH 45/89] Use Oracle documentation link --- docs/reference/config.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index ffc3a5c32..d88f0e14e 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -489,7 +489,7 @@ reflink ~~~~~~~ Either ``yes``, ``no``, or ``auto``, indicating whether to use copy-on-write -file clones (a.k.a. "reflinks") instead of copying or moving files. +`file clones`_ (a.k.a. "reflinks") instead of copying or moving files. The ``auto`` option uses reflinks when possible and falls back to plain copying when necessary. Defaults to ``no``. @@ -501,7 +501,7 @@ documentation. The option is ignored if ``move`` is enabled (i.e., beets can move or copy files but it doesn't make sense to do both). -.. _reflink clone: https://blogs.oracle.com/otn/save-disk-space-on-linux-by-cloning-files-on-btrfs-and-ocfs2 +.. _file clones: https://blogs.oracle.com/otn/save-disk-space-on-linux-by-cloning-files-on-btrfs-and-ocfs2 .. _pyreflink: https://reflink.readthedocs.io/en/latest/ resume From 76ca0549a9ef32ce70f68ccf1a204cf75935dfa3 Mon Sep 17 00:00:00 2001 From: soergeld Date: Sun, 9 Aug 2020 15:45:58 +0200 Subject: [PATCH 46/89] move mocking to setUp --- test/test_parentwork.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 0570afd35..d1093ad9d 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -145,13 +145,12 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') + musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def tearDown(self): self.unload_plugins() self.teardown_beets() - musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) - def test_normal_case(self): item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') item.add(self.lib) From d748cc7744e89496f8187f539ac78895a19dba9f Mon Sep 17 00:00:00 2001 From: soergeld Date: Sun, 9 Aug 2020 18:56:52 +0200 Subject: [PATCH 47/89] small problem with parent composer --- beetsplug/parentwork.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/parentwork.py b/beetsplug/parentwork.py index 85f8b0839..a5e042344 100644 --- a/beetsplug/parentwork.py +++ b/beetsplug/parentwork.py @@ -129,6 +129,7 @@ class ParentWorkPlugin(BeetsPlugin): if 'artist-relation-list' in work_info['work']: for artist in work_info['work']['artist-relation-list']: if artist['type'] == 'composer': + composer_exists = True parent_composer.append(artist['artist']['name']) parent_composer_sort.append(artist['artist']['sort-name']) if 'end' in artist.keys(): From d727149dd4ffb07755bd3c2b556d241f6e7cecec Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 10 Aug 2020 10:55:30 +0200 Subject: [PATCH 48/89] first try at using patch --- test/test_parentwork.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index d1093ad9d..f4306cf6f 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -19,13 +19,15 @@ from __future__ import division, absolute_import, print_function import os import unittest +from unittest.mock import patch from test.helper import TestHelper -from mock import Mock -import musicbrainzngs +#from mock import Mock +#import musicbrainzngs from beets.library import Item from beetsplug import parentwork + work = {'work': {'id': '1', 'title': 'work', 'work-relation-list': [{'type': 'parts', @@ -56,15 +58,6 @@ p_work = {'work': {'id': '3', 'composer, random'}}]}} -def mock_workid_response(mbid, includes): - if mbid == '1': - return work - elif mbid == '2': - return dp_work - elif mbid == '3': - return p_work - - class ParentWorkIntegrationTest(unittest.TestCase, TestHelper): def setUp(self): """Set up configuration""" @@ -145,11 +138,22 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') - musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) + self.patcher1 = patch('musicbrainzngs.get_work_by_id') + mock_workid_response = self.patcher1.start() + #musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def tearDown(self): self.unload_plugins() self.teardown_beets() + self.patcher1.stop() + + def mock_workid_response(self, mbid, includes): + if mbid == '1': + return work + elif mbid == '2': + return dp_work + elif mbid == '3': + return p_work def test_normal_case(self): item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') From 4f0a48394849ce7e0a673ed9e218dd11cb192857 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 10 Aug 2020 10:59:30 +0200 Subject: [PATCH 49/89] change function to method --- test/test_parentwork.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index f4306cf6f..a42ff610e 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -139,7 +139,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.setup_beets() self.load_plugins('parentwork') self.patcher1 = patch('musicbrainzngs.get_work_by_id') - mock_workid_response = self.patcher1.start() + self.mock_workid_response = self.patcher1.start() #musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def tearDown(self): From 29a8eef2815e24a44dfc3954c8596cc537549724 Mon Sep 17 00:00:00 2001 From: soergeld Date: Mon, 10 Aug 2020 11:02:14 +0200 Subject: [PATCH 50/89] problems importing patch --- test/test_parentwork.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index a42ff610e..b3322bcac 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -19,7 +19,6 @@ from __future__ import division, absolute_import, print_function import os import unittest -from unittest.mock import patch from test.helper import TestHelper #from mock import Mock #import musicbrainzngs @@ -138,7 +137,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') - self.patcher1 = patch('musicbrainzngs.get_work_by_id') + self.patcher1 = unittest.mock.patch('musicbrainzngs.get_work_by_id') self.mock_workid_response = self.patcher1.start() #musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) From 50757d34ad045eb0154a499cffb76fe6d4df0883 Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 12 Aug 2020 11:21:35 +0200 Subject: [PATCH 51/89] Fix conflicts --- beetsplug/replaygain.py | 9 ++-- docs/changelog.rst | 103 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index cc7ce547e..2be09dac5 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -22,17 +22,18 @@ import math import sys import warnings import enum +import re +import xml.parsers.expat +from six.moves import zip + from multiprocessing.pool import ThreadPool, RUN from threading import Thread, Event import signal -import xml.parsers.expat -from six.moves import zip, queue -import six from beets import ui from beets.plugins import BeetsPlugin from beets.util import (syspath, command_output, bytestring_path, - displayable_path, py3_path, cpu_count) + displayable_path, py3_path) # Utilities. diff --git a/docs/changelog.rst b/docs/changelog.rst index 85bda3cab..47159ddc2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,14 @@ Changelog New features: +* :doc:`/plugins/lastgenre`: Added more heavy metal genres: https://en.wikipedia.org/wiki/Heavy_metal_genres to genres.txt and genres-tree.yaml +* :doc:`/plugins/subsonicplaylist`: import playlist from a subsonic server. +* A new :ref:`extra_tags` configuration option allows more tagged metadata + to be included in MusicBrainz queries. +* A new :doc:`/plugins/fish` adds `Fish shell`_ tab autocompletion to beets +* :doc:`plugins/fetchart` and :doc:`plugins/embedart`: Added a new ``quality`` + option that controls the quality of the image output when the image is + resized. * :doc:`plugins/keyfinder`: Added support for `keyfinder-cli`_ Thanks to :user:`BrainDamage`. * :doc:`plugins/fetchart`: Added a new ``high_resolution`` config option to @@ -107,12 +115,48 @@ New features: titles. Thanks to :user:`cole-miller`. :bug:`3459` -* :doc:`/plugins/replaygain` now does its analysis in parallel when using - the ``command``, ``ffmpeg`` or ``bs1770gain`` backends. - :bug:`3478` +* :doc:`/plugins/fetchart`: Album art can now be fetched from `last.fm`_. + :bug:`3530` +* The classes ``AlbumInfo`` and ``TrackInfo`` now have flexible attributes, + allowing to solve :bug:`1547`. + Thanks to :user:`dosoe`. +* :doc:`/plugins/web`: The query API now interprets backslashes as path + separators to support path queries. + Thanks to :user:`nmeum`. + :bug:`3567` +* ``beet import`` now handles tar archives with bzip2 or gzip compression. + :bug:`3606` +* :doc:`/plugins/plexupdate`: Add option to use secure connection to Plex + server, and to ignore certificate validation errors if necessary. + :bug:`2871` +* :doc:`/plugins/lyrics`: Improved searching Genius backend when artist + contained special characters. + :bug:`3634` +* :doc:`/plugins/parentwork`: Also get the composition date of the parent work, + instead of just the child work. + Thanks to :user:`aereaux`. + :bug:`3650` +* :doc:`/plugins/lyrics`: Fix a bug in the heuristic for detecting valid + lyrics in the Google source of the lyrics plugin + :bug:`2969` +* :doc:`/plugins/thumbnails`: Fix a bug where pathlib expected a string instead + of bytes for a path. + :bug:`3360` +* :doc:`/plugins/convert`: If ``delete_originals`` is enabled, then the source files will + be deleted after importing. + Thanks to :user:`logan-arens`. + :bug:`2947` Fixes: +* :doc:`/plugins/the`: Fixed incorrect regex for 'the' that matched any + 3-letter combination of the letters t, h, e. + :bug:`3701` +* :doc:`/plugins/fetchart`: Fixed a bug that caused fetchart to not take + environment variables such as proxy servers into account when making requests + :bug:`3450` +* :doc:`/plugins/fetchart`: Temporary files for fetched album art that fail + validation are now removed * :doc:`/plugins/inline`: In function-style field definitions that refer to flexible attributes, values could stick around from one function invocation to the next. This meant that, when displaying a list of objects, later @@ -154,6 +198,57 @@ Fixes: * :doc:`/plugins/bpd`: Fix the transition to next track when in consume mode. Thanks to :user:`aereaux`. :bug:`3437` +* :doc:`/plugins/lyrics`: Fix a corner-case with Genius lowercase artist names + :bug:`3446` +* :doc:`/plugins/replaygain`: Support ``bs1770gain`` v0.6.0 and up + :bug:`3480` +* :doc:`/plugins/parentwork`: Don't save tracks when nothing has changed. + :bug:`3492` +* Added a warning when configuration files defined in the `include` directive + of the configuration file fail to be imported. + :bug:`3498` +* Added the normalize method to the dbcore.types.INTEGER class which now + properly returns integer values, which should avoid problems where fields + like ``bpm`` would sometimes store non-integer values. + :bug:`762` :bug:`3507` :bug:`3508` +* Removed ``@classmethod`` decorator from dbcore.query.NoneQuery.match method + failing with AttributeError when called. It is now an instance method. + :bug:`3516` :bug:`3517` +* :doc:`/plugins/lyrics`: Tolerate missing lyrics div in Genius scraper. + Thanks to :user:`thejli21`. + :bug:`3535` :bug:`3554` +* :doc:`/plugins/lyrics`: Use the artist sort name to search for lyrics, which + can help find matches when the artist name has special characters. + Thanks to :user:`hashhar`. + :bug:`3340` :bug:`3558` +* :doc:`/plugins/replaygain`: Trying to calculate volume gain for an album + consisting of some formats using ``ReplayGain`` and some using ``R128`` + will no longer crash; instead it is skipped and and a message is logged. + The log message has also been rewritten for to improve clarity. + Thanks to :user:`autrimpo`. + :bug:`3533` +* :doc:`/plugins/lyrics`: Adapt the Genius backend to changes in markup to + reduce the scraping failure rate. + :bug:`3535` :bug:`3594` +* :doc:`/plugins/lyrics`: Fix crash when writing ReST files for a query without + results or fetched lyrics + :bug:`2805` +* Adapt to breaking changes in Python's ``ast`` module in 3.8 +* :doc:`/plugins/fetchart`: Attempt to fetch pre-resized thumbnails from Cover + Art Archive if the ``maxwidth`` option matches one of the sizes supported by + the Cover Art Archive API. + Thanks to :user:`trolley`. + :bug:`3637` +* :doc:`/plugins/ipfs`: Fix Python 3 compatibility. + Thanks to :user:`musoke`. + :bug:`2554` +* Fix a bug that caused metadata starting with something resembling a drive + letter to be incorrectly split into an extra directory after the colon. + :bug:`3685` +* :doc:`/plugins/mpdstats`: Don't record a skip when stopping MPD, as MPD keeps + the current track in the queue. + Thanks to :user:`aereaux`. + :bug:`3722` For plugin developers: @@ -205,11 +300,13 @@ For packagers: the test may no longer be necessary. * This version drops support for Python 3.4. +.. _Fish shell: https://fishshell.com/ .. _MediaFile: https://github.com/beetbox/mediafile .. _Confuse: https://github.com/beetbox/confuse .. _works: https://musicbrainz.org/doc/Work .. _Deezer: https://www.deezer.com .. _keyfinder-cli: https://github.com/EvanPurkhiser/keyfinder-cli +.. _last.fm: https://last.fm 1.4.9 (May 30, 2019) From f4db4ae1bfb3ee8e15862a5adb4bb4e5a28cf7a4 Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 12 Aug 2020 11:59:59 +0200 Subject: [PATCH 52/89] Re-add to changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 47159ddc2..3278c73a8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -146,6 +146,9 @@ New features: be deleted after importing. Thanks to :user:`logan-arens`. :bug:`2947` +* :doc:`/plugins/replaygain` now does its analysis in parallel when using + the ``command``, ``ffmpeg`` or ``bs1770gain`` backends. + :bug:`3478` Fixes: From e1876590ba26074d85b38d40b4e36d51b4a14e5a Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 12 Aug 2020 12:49:16 +0200 Subject: [PATCH 53/89] Fix replaygain.py to pass test_replaygain.py --- beetsplug/replaygain.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 076ca0f5c..09607ffe9 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -24,16 +24,18 @@ import warnings import enum import re import xml.parsers.expat -from six.moves import zip +from six.moves import zip, queue +import six from multiprocessing.pool import ThreadPool, RUN from threading import Thread, Event import signal +from sqlite3 import OperationalError from beets import ui from beets.plugins import BeetsPlugin from beets.util import (syspath, command_output, bytestring_path, - displayable_path, py3_path) + displayable_path, py3_path, cpu_count) # Utilities. @@ -1307,30 +1309,39 @@ class ReplayGainPlugin(BeetsPlugin): (not item.rg_album_gain or not item.rg_album_peak) for item in album.items()]) + def _store(self, item): + try: + item.store() + except OperationalError: + # test_replaygain.py :memory: library can fail with + # `sqlite3.OperationalError: no such table: items` + # but the second attempt succeeds + item.store() + def store_track_gain(self, item, track_gain): item.rg_track_gain = track_gain.gain item.rg_track_peak = track_gain.peak - item.store() + self._store(item) self._log.debug(u'applied track gain {0} LU, peak {1} of FS', item.rg_track_gain, item.rg_track_peak) def store_album_gain(self, item, album_gain): item.rg_album_gain = album_gain.gain item.rg_album_peak = album_gain.peak - item.store() + self._store(item) self._log.debug(u'applied album gain {0} LU, peak {1} of FS', item.rg_album_gain, item.rg_album_peak) def store_track_r128_gain(self, item, track_gain): item.r128_track_gain = track_gain.gain - item.store() + self._store(item) self._log.debug(u'applied r128 track gain {0} LU', item.r128_track_gain) def store_album_r128_gain(self, item, album_gain): item.r128_album_gain = album_gain.gain - item.store() + self._store(item) self._log.debug(u'applied r128 album gain {0} LU', item.r128_album_gain) @@ -1520,7 +1531,7 @@ class ReplayGainPlugin(BeetsPlugin): if hasattr(self, 'pool') and isinstance(self.pool, ThreadPool): self.pool.terminate() self.pool.join() - self.exc_watcher.join() + # self.exc_watcher.join() def _interrupt(self, signal, frame): try: From 176fa55bf6f88c9c45f9c45ce7e951a2c5444c37 Mon Sep 17 00:00:00 2001 From: AnonTester <40003252+AnonTester@users.noreply.github.com> Date: Sat, 7 Nov 2020 21:41:16 +0000 Subject: [PATCH 54/89] lyrics: Strip \u2005 (four-per-em space) in lyrics (Issue 3789) https://github.com/beetbox/beets/issues/3789 --- beetsplug/lyrics.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 5591598ae..58773ae2b 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -424,6 +424,7 @@ def _scrape_strip_cruft(html, plain_text_out=False): html = re.sub(r' +', ' ', html) # Whitespaces collapse. html = BREAK_RE.sub('\n', html) #
eats up surrounding '\n'. html = re.sub(r'(?s)<(script).*?', '', html) # Strip script tags. + html = re.sub(u'\u2005', " ", html) # replace Unicode Four-per-em space with regular space if plain_text_out: # Strip remaining HTML tags html = COMMENT_RE.sub('', html) From ecfafb5041d06217230a458668efd1930a024dc6 Mon Sep 17 00:00:00 2001 From: AnonTester <40003252+AnonTester@users.noreply.github.com> Date: Sat, 7 Nov 2020 21:48:21 +0000 Subject: [PATCH 55/89] Adjust comment to pass lint test --- beetsplug/lyrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 58773ae2b..00d65b578 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -424,7 +424,7 @@ def _scrape_strip_cruft(html, plain_text_out=False): html = re.sub(r' +', ' ', html) # Whitespaces collapse. html = BREAK_RE.sub('\n', html) #
eats up surrounding '\n'. html = re.sub(r'(?s)<(script).*?', '', html) # Strip script tags. - html = re.sub(u'\u2005', " ", html) # replace Unicode Four-per-em space with regular space + html = re.sub(u'\u2005', " ", html) # replace unicode with regular space if plain_text_out: # Strip remaining HTML tags html = COMMENT_RE.sub('', html) From 8f8fd4231a1f950f53f3183f01d21d71575e30d9 Mon Sep 17 00:00:00 2001 From: Benedikt Tissot Date: Mon, 9 Nov 2020 15:55:21 +0100 Subject: [PATCH 56/89] escape ? in fish completion --- beetsplug/fish.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/beetsplug/fish.py b/beetsplug/fish.py index b842ac70f..abe3c3d71 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -201,6 +201,10 @@ def get_subcommands(cmd_name_and_help, nobasicfields, extravalues): # Formatting for Fish to complete our fields/values word = "" for cmdname, cmdhelp in cmd_name_and_help: + # Escape ? in fish + if cmdname == "?": + cmdname = "\\" + cmdname + word += "\n" + "# ------ {} -------".format( "fieldsetups for " + cmdname) + "\n" word += ( @@ -232,6 +236,10 @@ def get_all_commands(beetcmds): names = [alias for alias in cmd.aliases] names.append(cmd.name) for name in names: + # Escape ? in fish + if name == "?": + name = "\\" + name + word += "\n" word += ("\n" * 2) + "# ====== {} =====".format( "completions for " + name) + "\n" From 57a1b3aca86253d130848fdfcd2accbed861b5ee Mon Sep 17 00:00:00 2001 From: Benedikt Tissot Date: Mon, 9 Nov 2020 17:07:00 +0100 Subject: [PATCH 57/89] escape using helper function --- beetsplug/fish.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/beetsplug/fish.py b/beetsplug/fish.py index abe3c3d71..9883ed693 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -132,6 +132,11 @@ class FishPlugin(BeetsPlugin): with open(completion_file_path, 'w') as fish_file: fish_file.write(totstring) +def _escape(name): + # Escape ? in fish + if name == "?": + name = "\\" + name + def get_cmds_list(cmds_names): # Make a list of all Beets core & plugin commands @@ -201,9 +206,7 @@ def get_subcommands(cmd_name_and_help, nobasicfields, extravalues): # Formatting for Fish to complete our fields/values word = "" for cmdname, cmdhelp in cmd_name_and_help: - # Escape ? in fish - if cmdname == "?": - cmdname = "\\" + cmdname + cmdname = _escape(cmdname) word += "\n" + "# ------ {} -------".format( "fieldsetups for " + cmdname) + "\n" @@ -236,9 +239,7 @@ def get_all_commands(beetcmds): names = [alias for alias in cmd.aliases] names.append(cmd.name) for name in names: - # Escape ? in fish - if name == "?": - name = "\\" + name + name = _escape(name) word += "\n" word += ("\n" * 2) + "# ====== {} =====".format( From 020c082f3f21005896ebe116475ce5fa6bf8b08a Mon Sep 17 00:00:00 2001 From: Benedikt Tissot Date: Mon, 9 Nov 2020 17:12:44 +0100 Subject: [PATCH 58/89] make CI happy --- beetsplug/fish.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/fish.py b/beetsplug/fish.py index 9883ed693..0f7fe1e2c 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -132,6 +132,7 @@ class FishPlugin(BeetsPlugin): with open(completion_file_path, 'w') as fish_file: fish_file.write(totstring) + def _escape(name): # Escape ? in fish if name == "?": From bb80fe5c5ba6ccf75b8e102085fdaa3b53b7b29e Mon Sep 17 00:00:00 2001 From: rachmadaniHaryono Date: Wed, 11 Nov 2020 12:04:11 +0800 Subject: [PATCH 59/89] fix: doc: copyartifacts fork --- docs/plugins/index.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 1d5d1b815..2b16d96d8 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -275,7 +275,7 @@ Here are a few of the plugins written by the beets community: * `beet-amazon`_ adds Amazon.com as a tagger data source. -* `copyartifacts`_ helps bring non-music files along during import. +* `beets-copyartifacts`_ helps bring non-music files along during import. * `beets-check`_ automatically checksums your files to detect corruption. @@ -326,7 +326,7 @@ Here are a few of the plugins written by the beets community: .. _beets-barcode: https://github.com/8h2a/beets-barcode .. _beets-check: https://github.com/geigerzaehler/beets-check -.. _copyartifacts: https://github.com/sbarakat/beets-copyartifacts +.. _beets-copyartifacts: https://github.com/adammillerio/beets-copyartifacts .. _dsedivec: https://github.com/dsedivec/beets-plugins .. _beets-artistcountry: https://github.com/agrausem/beets-artistcountry .. _beetFs: https://github.com/jbaiter/beetfs From 4acbd2a8b37fe931a1d899140f93c7411a475ebc Mon Sep 17 00:00:00 2001 From: rachmadaniHaryono Date: Wed, 11 Nov 2020 12:28:16 +0800 Subject: [PATCH 60/89] new: doc: changelog --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 458e4b8d2..af4cc9493 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -169,6 +169,7 @@ New features: Fixes: +* :doc:`/plugins/index`: Change beets-copyartifacts to maintained repo. * :doc:`/plugins/subsonicupdate`: REST was using `POST` method rather `GET` method. Also includes better exception handling, response parsing, and tests. * :doc:`/plugins/the`: Fixed incorrect regex for 'the' that matched any From 76a5b055f4a36d67e670cea3ffc820645030be76 Mon Sep 17 00:00:00 2001 From: rachmadaniHaryono Date: Thu, 12 Nov 2020 06:26:47 +0800 Subject: [PATCH 61/89] Revert "new: doc: changelog" This reverts commit 4acbd2a8b37fe931a1d899140f93c7411a475ebc. --- docs/changelog.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index af4cc9493..458e4b8d2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -169,7 +169,6 @@ New features: Fixes: -* :doc:`/plugins/index`: Change beets-copyartifacts to maintained repo. * :doc:`/plugins/subsonicupdate`: REST was using `POST` method rather `GET` method. Also includes better exception handling, response parsing, and tests. * :doc:`/plugins/the`: Fixed incorrect regex for 'the' that matched any From 30a2dd99988e20b411e5079c2cee34c69f6b43e4 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Thu, 26 Nov 2020 13:05:10 +0100 Subject: [PATCH 62/89] assert False on unknown move operation --- beets/library.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/beets/library.py b/beets/library.py index dea2a937e..a060e93d6 100644 --- a/beets/library.py +++ b/beets/library.py @@ -756,11 +756,7 @@ class Item(LibModel): plugins.send("item_reflinked", item=self, source=self.path, destination=dest) else: - plugins.send("before_item_moved", item=self, source=self.path, - destination=dest) - util.move(self.path, dest) - plugins.send("item_moved", item=self, source=self.path, - destination=dest) + assert False, 'unknown MoveOperation' # Either copying or moving succeeded, so update the stored path. self.path = dest @@ -1106,7 +1102,7 @@ class Album(LibModel): elif operation == MoveOperation.REFLINK_AUTO: util.reflink(old_art, new_art, fallback=True) else: - util.move(old_art, new_art) + assert False, 'unknown MoveOperation' self.artpath = new_art def move(self, operation=MoveOperation.MOVE, basedir=None, store=True): From 5efaa09482d99f3ea60b23edd07e53564a3e4835 Mon Sep 17 00:00:00 2001 From: Ruben De Smet Date: Thu, 26 Nov 2020 13:45:33 +0100 Subject: [PATCH 63/89] Note installing pyreflink in docs --- docs/reference/config.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index d88f0e14e..d103b26d6 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -496,7 +496,8 @@ Defaults to ``no``. This kind of clone is only available on certain filesystems: for example, btrfs and APFS. For more details on filesystem support, see the `pyreflink`_ -documentation. +documentation. Note that you need to install ``pyreflink``, either through +``python -m pip install beets[reflink]`` or ``python -m pip install reflink``. The option is ignored if ``move`` is enabled (i.e., beets can move or copy files but it doesn't make sense to do both). From e11687f80a377ce2de38fe38a22c45be28837a5d Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Mon, 7 Dec 2020 22:04:05 -0500 Subject: [PATCH 64/89] keyfinder: Catch output from keyfinder-cli with no key --- beetsplug/keyfinder.py | 9 ++++++++- docs/changelog.rst | 2 ++ test/test_keyfinder.py | 10 ++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index a75b8d972..7836b4c54 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -76,7 +76,14 @@ class KeyFinderPlugin(BeetsPlugin): item.path) continue - key_raw = output.rsplit(None, 1)[-1] + try: + key_raw = output.rsplit(None, 1)[-1] + except IndexError: + # Sometimes keyfinder-cli returns 0 but with no key, usually when + # the file is silent or corrupt, so we log and skip. + self._log.error(u'no key returned for path: {0}', item.path) + continue + try: key = util.text_string(key_raw) except UnicodeDecodeError: diff --git a/docs/changelog.rst b/docs/changelog.rst index 0de3b15a2..41221b1f8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -282,6 +282,8 @@ Fixes: :bug:`3773` :bug:`3774` * Fix a bug causing PIL to generate poor quality JPEGs when resizing artwork. :bug:`3743` +* :doc:`plugins/keyfinder`: Catch output from ``keyfinder-cli`` that is missing key. + :bug:`2242` For plugin developers: diff --git a/test/test_keyfinder.py b/test/test_keyfinder.py index a9ac43a27..c8735e47f 100644 --- a/test/test_keyfinder.py +++ b/test/test_keyfinder.py @@ -76,6 +76,16 @@ class KeyFinderTest(unittest.TestCase, TestHelper): item.load() self.assertEqual(item['initial_key'], 'F') + def test_no_key(self, command_output): + item = Item(path='/file') + item.add(self.lib) + + command_output.return_value = util.CommandOutput(b"", b"") + self.run_command('keyfinder') + + item.load() + self.assertEqual(item['initial_key'], None) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From c1d93165f05ba87387c61ae2eda3c93a77efa956 Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Mon, 7 Dec 2020 22:11:08 -0500 Subject: [PATCH 65/89] Fix line length --- beetsplug/keyfinder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index 7836b4c54..702003f0f 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -79,8 +79,8 @@ class KeyFinderPlugin(BeetsPlugin): try: key_raw = output.rsplit(None, 1)[-1] except IndexError: - # Sometimes keyfinder-cli returns 0 but with no key, usually when - # the file is silent or corrupt, so we log and skip. + # Sometimes keyfinder-cli returns 0 but with no key, usually + # when the file is silent or corrupt, so we log and skip. self._log.error(u'no key returned for path: {0}', item.path) continue From 363f71af2e3544605ac560122a965ab4bf5516d5 Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 14 Dec 2020 22:10:02 +0100 Subject: [PATCH 66/89] Move OperationalError handler to test_replaygain.py --- beetsplug/replaygain.py | 16 ++++++++-------- test/test_replaygain.py | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 09607ffe9..31742256a 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -30,7 +30,6 @@ import six from multiprocessing.pool import ThreadPool, RUN from threading import Thread, Event import signal -from sqlite3 import OperationalError from beets import ui from beets.plugins import BeetsPlugin @@ -1310,13 +1309,14 @@ class ReplayGainPlugin(BeetsPlugin): for item in album.items()]) def _store(self, item): - try: - item.store() - except OperationalError: - # test_replaygain.py :memory: library can fail with - # `sqlite3.OperationalError: no such table: items` - # but the second attempt succeeds - item.store() + """Store an item to the database. + When testing, item.store() sometimes fails non-destructively with + sqlite.OperationalError. + This method is here to be patched to a retry-once helper function + in test_replaygain.py, so that it can still fail appropriately + outside of these tests. + """ + item.store() def store_track_gain(self, item, track_gain): item.rg_track_gain = track_gain.gain diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 969f5c230..21a5fa5a7 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -22,11 +22,15 @@ import six from mock import patch from test.helper import TestHelper, capture_log, has_program +from sqlite3 import OperationalError + from beets import config from beets.util import CommandOutput from mediafile import MediaFile from beetsplug.replaygain import (FatalGstreamerPluginReplayGainError, - GStreamerBackend) + GStreamerBackend, + ReplayGainPlugin) + try: import gi @@ -55,10 +59,28 @@ def reset_replaygain(item): item['rg_album_gain'] = None item.write() item.store() + item.store() + item.store() +def _store_retry_once(self, item): + """Helper method to retry item.store() once in case + of a sqlite3.OperationalError exception. + + :param self: `ReplayGainPlugin` instance + :param item: a library item to store + """ + try: + item.store() + except OperationalError: + # test_replaygain.py :memory: library can fail with + # `sqlite3.OperationalError: no such table: items` + # but the second attempt succeeds + item.store() + + +@patch.object(ReplayGainPlugin, '_store', _store_retry_once) class ReplayGainCliTestBase(TestHelper): - def setUp(self): self.setup_beets() self.config['replaygain']['backend'] = self.backend From 3fec6d347a432e34adc9d74e87ff465107da74b7 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Tue, 15 Dec 2020 12:45:13 +0100 Subject: [PATCH 67/89] Use caching in plugins.find_plugins() This function is called from library.Item._getters(), making it a hot path when exporting data. We cache plugin instances but previously we reran `import` for each module on each call. We now return the cached values providing a speed boost for `beet export`. This has a small effect on `beet ls` speed: for me it went 11.00s -> 10.40. It had a significant effect on `beet export` speed when combined with https://github.com/beetbox/beets/pull/3762/. Before: $ time /home/sam/.local/bin/beet 'export' '--library' '--format' 'jsonlines' '--include-keys' 'artist,title,path,mb_artistid,mb_trackid' 'artist+ title+' > /dev/null Executed in 25.13 secs After: $ time /home/sam/.local/bin/beet 'export' '--library' '--format' 'jsonlines' '--include-keys' 'artist,title,path,mb_artistid,mb_trackid' 'artist+ title+' > /dev/null Executed in 10.49 secs --- beets/plugins.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/beets/plugins.py b/beets/plugins.py index 695725cb8..3abd911c9 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -301,6 +301,11 @@ def find_plugins(): currently loaded beets plugins. Loads the default plugin set first. """ + if _instances: + # After the first call, use cached instances for performance reasons. + # See https://github.com/beetbox/beets/pull/3810 + return list(_instances.values()) + load_plugins() plugins = [] for cls in _classes: From 95677c8626135864cebfef969d15e391f2ca1063 Mon Sep 17 00:00:00 2001 From: sentriz Date: Tue, 15 Dec 2020 13:12:47 +0000 Subject: [PATCH 68/89] lastgenre: Make TitleCasing optional --- beetsplug/lastgenre/__init__.py | 8 +++++++- docs/changelog.rst | 1 + docs/plugins/lastgenre.rst | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index cf90facbd..59beddfa9 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -111,6 +111,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): 'auto': True, 'separator': u', ', 'prefer_specific': False, + 'title_case': True, }) self.setup() @@ -224,12 +225,17 @@ class LastGenrePlugin(plugins.BeetsPlugin): # c14n only adds allowed genres but we may have had forbidden genres in # the original tags list - tags = [x.title() for x in tags if self._is_allowed(x)] + tags = [self._format_tag(x) for x in tags if self._is_allowed(x)] return self.config['separator'].as_str().join( tags[:self.config['count'].get(int)] ) + def _format_tag(self, tag): + if self.config["title_case"]: + return tag.title() + return tag + def fetch_genre(self, lastfm_obj): """Return the genre for a pylast entity or None if no suitable genre can be found. Ex. 'Electronic, House, Dance' diff --git a/docs/changelog.rst b/docs/changelog.rst index c1a4f9fa3..e2ef817b6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,7 @@ Changelog New features: +* Add ``title_case`` config option to lastgenre to make TitleCasing optional. * When config is printed with no available configuration a new message is printed. :bug:`3779` * When importing a duplicate album it ask if it should "Keep all" instead of "Keep both". diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index dee4260de..cbc54f1cc 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -146,6 +146,8 @@ configuration file. The available options are: - **whitelist**: The filename of a custom genre list, ``yes`` to use the internal whitelist, or ``no`` to consider all genres valid. Default: ``yes``. +- **title_case**: Convert the new tags to TitleCase before saving. + Default: ``yes``. Running Manually ---------------- From c94809f6da0821fc392cd3b877d62681911aa0f3 Mon Sep 17 00:00:00 2001 From: Billy Janitsch Date: Wed, 16 Dec 2020 02:15:26 -0500 Subject: [PATCH 69/89] Fix escape helper in fish completion plugin --- beetsplug/fish.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/fish.py b/beetsplug/fish.py index 0f7fe1e2c..12f474e88 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -137,6 +137,7 @@ def _escape(name): # Escape ? in fish if name == "?": name = "\\" + name + return name def get_cmds_list(cmds_names): From 1dfd79e6584325a476f41859d85744dbf86a914f Mon Sep 17 00:00:00 2001 From: ybnd Date: Sun, 20 Dec 2020 19:38:21 +0100 Subject: [PATCH 70/89] Fix logic for replaygain auto-compute on import --- beetsplug/replaygain.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 31742256a..d3fff4a62 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1553,7 +1553,8 @@ class ReplayGainPlugin(BeetsPlugin): def import_begin(self, session): """Handle `import_begin` event -> open pool """ - self.open_pool(self.config['threads'].get(int)) + if self.config['auto'].get(bool): + self.open_pool(self.config['threads'].get(int)) def import_end(self, paths): """Handle `import` event -> close pool @@ -1563,10 +1564,19 @@ class ReplayGainPlugin(BeetsPlugin): def imported(self, session, task): """Add replay gain info to items or albums of ``task``. """ - if task.is_album: - self.handle_album(task.album, False) - else: - self.handle_track(task.item, False) + if self.config['auto'].get(bool): + if task.is_album: + self.handle_album( + task.album, + self.config['auto'].get(bool), + self.config['overwrite'].get(bool) + ) + else: + self.handle_track( + task.item, + self.config['auto'].get(bool), + self.config['overwrite'].get(bool) + ) def commands(self): """Return the "replaygain" ui subcommand. From 12db40fa67e7b905592495df4ac12bb0c6a36356 Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 21 Dec 2020 08:50:39 +0100 Subject: [PATCH 71/89] Remove .get() when checking config values --- beetsplug/replaygain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index d3fff4a62..75e6ce23b 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1553,7 +1553,7 @@ class ReplayGainPlugin(BeetsPlugin): def import_begin(self, session): """Handle `import_begin` event -> open pool """ - if self.config['auto'].get(bool): + if self.config['auto']: self.open_pool(self.config['threads'].get(int)) def import_end(self, paths): @@ -1564,7 +1564,7 @@ class ReplayGainPlugin(BeetsPlugin): def imported(self, session, task): """Add replay gain info to items or albums of ``task``. """ - if self.config['auto'].get(bool): + if self.config['auto']: if task.is_album: self.handle_album( task.album, From 30baf22e77d8a9ba2cc7de408af593fa393a5e02 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Mon, 21 Dec 2020 11:04:29 +0100 Subject: [PATCH 72/89] replaygain: Use on-disk database there can only be a single connection to an in-memory database, but the parallel replaygain code accesses the db from different threads --- test/test_replaygain.py | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 21a5fa5a7..0100b520e 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -22,14 +22,11 @@ import six from mock import patch from test.helper import TestHelper, capture_log, has_program -from sqlite3 import OperationalError - from beets import config from beets.util import CommandOutput from mediafile import MediaFile from beetsplug.replaygain import (FatalGstreamerPluginReplayGainError, - GStreamerBackend, - ReplayGainPlugin) + GStreamerBackend) try: @@ -63,26 +60,9 @@ def reset_replaygain(item): item.store() -def _store_retry_once(self, item): - """Helper method to retry item.store() once in case - of a sqlite3.OperationalError exception. - - :param self: `ReplayGainPlugin` instance - :param item: a library item to store - """ - try: - item.store() - except OperationalError: - # test_replaygain.py :memory: library can fail with - # `sqlite3.OperationalError: no such table: items` - # but the second attempt succeeds - item.store() - - -@patch.object(ReplayGainPlugin, '_store', _store_retry_once) class ReplayGainCliTestBase(TestHelper): def setUp(self): - self.setup_beets() + self.setup_beets(disk=True) self.config['replaygain']['backend'] = self.backend try: From 10e81a55dd8459e9f223048306422d00689da99b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 21 Dec 2020 08:49:14 -0500 Subject: [PATCH 73/89] Disable Python 3.5 CI on Windows It was broken and is not worth fixing. --- appveyor.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 5a0f32135..233fa3178 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -9,8 +9,6 @@ environment: matrix: - PYTHON: C:\Python27 TOX_ENV: py27-test - - PYTHON: C:\Python35 - TOX_ENV: py35-test - PYTHON: C:\Python36 TOX_ENV: py36-test - PYTHON: C:\Python37 From 93101e7ea64e5f46063e28552e9a2bbd660a63f4 Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 21 Dec 2020 19:19:20 +0100 Subject: [PATCH 74/89] Disable replaygain parallelism during import --- beetsplug/replaygain.py | 7 +++++-- docs/changelog.rst | 2 ++ docs/plugins/replaygain.rst | 6 ++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 75e6ce23b..5dba3ad60 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1234,6 +1234,7 @@ class ReplayGainPlugin(BeetsPlugin): 'auto': True, 'backend': u'command', 'threads': cpu_count(), + 'parallel_on_import': False, 'per_disc': False, 'peak': 'true', 'targetlevel': 89, @@ -1553,8 +1554,10 @@ class ReplayGainPlugin(BeetsPlugin): def import_begin(self, session): """Handle `import_begin` event -> open pool """ - if self.config['auto']: - self.open_pool(self.config['threads'].get(int)) + threads = self.config['threads'].get(int) + + if self.config['parallel_on_import'] and threads: + self.open_pool(threads) def import_end(self, paths): """Handle `import` event -> close pool diff --git a/docs/changelog.rst b/docs/changelog.rst index e2ef817b6..268080d4c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -288,6 +288,8 @@ Fixes: :bug:`3743` * :doc:`plugins/keyfinder`: Catch output from ``keyfinder-cli`` that is missing key. :bug:`2242` +* :doc:`plugins/replaygain`: Disable parallel analysis on import by default. + For plugin developers: diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index bda85bb05..fa0e10b75 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -103,6 +103,12 @@ configuration file. The available options are: - **threads**: The number of parallel threads to run the analysis in. Overridden by ``--threads`` at the command line. Default: # of logical CPU cores +- **parallel_on_import**: Whether to enable parallel analysis during import. + As of now this ReplayGain data is not written to files properly, so this option + is disabled by default. + If you wish to enable it, remember to run ``beet write`` after importing to + actually write to the imported files. + Default: ``no`` - **backend**: The analysis backend; either ``gstreamer``, ``command``, ``audiotools`` or ``ffmpeg``. Default: ``command``. From cc18beb12add9ab63e750b4099ed572f65730410 Mon Sep 17 00:00:00 2001 From: ybnd Date: Mon, 21 Dec 2020 19:33:23 +0100 Subject: [PATCH 75/89] Reference issue --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 268080d4c..b73320756 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -289,7 +289,7 @@ Fixes: * :doc:`plugins/keyfinder`: Catch output from ``keyfinder-cli`` that is missing key. :bug:`2242` * :doc:`plugins/replaygain`: Disable parallel analysis on import by default. - + :bug:`3819` For plugin developers: From 18bfefece78f015669f8cbc83809b1cdfb60ada9 Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 22 Dec 2020 11:51:24 +0100 Subject: [PATCH 76/89] revert changes to working version with external mock --- test/test_parentwork.py | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index b3322bcac..e46ce2d74 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -20,8 +20,8 @@ from __future__ import division, absolute_import, print_function import os import unittest from test.helper import TestHelper -#from mock import Mock -#import musicbrainzngs +from mock import Mock +import musicbrainzngs from beets.library import Item from beetsplug import parentwork @@ -56,6 +56,16 @@ p_work = {'work': {'id': '3', 'sort-name': 'composer, random'}}]}} +def mock_workid_response(mbid, includes): + if mbid == '1': + return work + elif mbid == '2': + return dp_work + elif mbid == '3': + return p_work + + + class ParentWorkIntegrationTest(unittest.TestCase, TestHelper): def setUp(self): @@ -137,22 +147,22 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') - self.patcher1 = unittest.mock.patch('musicbrainzngs.get_work_by_id') - self.mock_workid_response = self.patcher1.start() - #musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) + #self.patcher1 = unittest.mock.patch('musicbrainzngs.get_work_by_id') + #self.mock_workid_response = self.patcher1.start() + musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def tearDown(self): self.unload_plugins() self.teardown_beets() - self.patcher1.stop() + #self.patcher1.stop() - def mock_workid_response(self, mbid, includes): - if mbid == '1': - return work - elif mbid == '2': - return dp_work - elif mbid == '3': - return p_work + #def mock_workid_response(self, mbid, includes): + # if mbid == '1': + # return work + # elif mbid == '2': + # return dp_work + # elif mbid == '3': + # return p_work def test_normal_case(self): item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') From a8b81ca193a1c86ec2b277a1da8a5ea7fe2e921b Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 22 Dec 2020 12:19:26 +0100 Subject: [PATCH 77/89] style --- test/test_parentwork.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index e46ce2d74..be8ccb596 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -56,6 +56,7 @@ p_work = {'work': {'id': '3', 'sort-name': 'composer, random'}}]}} + def mock_workid_response(mbid, includes): if mbid == '1': return work @@ -65,8 +66,6 @@ def mock_workid_response(mbid, includes): return p_work - - class ParentWorkIntegrationTest(unittest.TestCase, TestHelper): def setUp(self): """Set up configuration""" @@ -147,22 +146,11 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') - #self.patcher1 = unittest.mock.patch('musicbrainzngs.get_work_by_id') - #self.mock_workid_response = self.patcher1.start() musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) def tearDown(self): self.unload_plugins() self.teardown_beets() - #self.patcher1.stop() - - #def mock_workid_response(self, mbid, includes): - # if mbid == '1': - # return work - # elif mbid == '2': - # return dp_work - # elif mbid == '3': - # return p_work def test_normal_case(self): item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') From eb8ba838e5f4e3f1c8fe7f2b14a7331d83e51c0c Mon Sep 17 00:00:00 2001 From: ybnd Date: Tue, 22 Dec 2020 12:45:28 +0100 Subject: [PATCH 78/89] Check 'auto' on import_begin --- beetsplug/replaygain.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 5dba3ad60..9d6fa23c4 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1556,7 +1556,9 @@ class ReplayGainPlugin(BeetsPlugin): """ threads = self.config['threads'].get(int) - if self.config['parallel_on_import'] and threads: + if self.config['parallel_on_import'] \ + and self.config['auto'] \ + and threads: self.open_pool(threads) def import_end(self, paths): From a8d1cc9ce4dec4e5ac97496e4b53cf3021a995d5 Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 22 Dec 2020 15:40:50 +0100 Subject: [PATCH 79/89] mock confined to test --- test/test_parentwork.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index be8ccb596..4ba1caac6 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -22,6 +22,7 @@ import unittest from test.helper import TestHelper from mock import Mock import musicbrainzngs +from mock import patch from beets.library import Item from beetsplug import parentwork @@ -147,10 +148,13 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.setup_beets() self.load_plugins('parentwork') musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) + self.patcher = patch('musicbrainzngs.get_work_by_id', + side_effect=mock_workid_response) def tearDown(self): self.unload_plugins() self.teardown_beets() + self.patcher.stop() def test_normal_case(self): item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') From 53474678e9c8ea70c76d214d2769221759e1a922 Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 22 Dec 2020 15:41:49 +0100 Subject: [PATCH 80/89] mock confined to test --- test/test_parentwork.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index 4ba1caac6..d288e6900 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -147,7 +147,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') - musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) + #musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) self.patcher = patch('musicbrainzngs.get_work_by_id', side_effect=mock_workid_response) From 7803ce271ed839faf3ef6c68d854d6d1f5956c37 Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 22 Dec 2020 15:59:45 +0100 Subject: [PATCH 81/89] mock confined to test --- test/test_parentwork.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index d288e6900..e423e6048 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -147,9 +147,10 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') - #musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) +# musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) self.patcher = patch('musicbrainzngs.get_work_by_id', side_effect=mock_workid_response) + self.patcher.start() def tearDown(self): self.unload_plugins() From 03cc76375d4b5cb54b404f1dba44060590851bf7 Mon Sep 17 00:00:00 2001 From: soergeld Date: Tue, 22 Dec 2020 16:02:48 +0100 Subject: [PATCH 82/89] style --- test/test_parentwork.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index e423e6048..b124785bb 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -20,8 +20,6 @@ from __future__ import division, absolute_import, print_function import os import unittest from test.helper import TestHelper -from mock import Mock -import musicbrainzngs from mock import patch from beets.library import Item @@ -147,7 +145,6 @@ class ParentWorkTest(unittest.TestCase, TestHelper): """Set up configuration""" self.setup_beets() self.load_plugins('parentwork') -# musicbrainzngs.get_work_by_id = Mock(side_effect=mock_workid_response) self.patcher = patch('musicbrainzngs.get_work_by_id', side_effect=mock_workid_response) self.patcher.start() From 8bda9f991ab824d66940d02e9c6ada3184404022 Mon Sep 17 00:00:00 2001 From: Auguste Olivry Date: Tue, 5 Jan 2021 14:07:00 +0100 Subject: [PATCH 83/89] Keep index tracks in coalesced tracklist --- beetsplug/discogs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 3b2595585..29a7e8585 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -499,7 +499,7 @@ class DiscogsPlugin(BeetsPlugin): else: # Promote the subtracks to real tracks, discarding the # index track, assuming the subtracks are physical tracks. - index_track = tracklist.pop() + index_track = tracklist[-1] # Fix artists when they are specified on the index track. if index_track.get('artists'): for subtrack in subtracks: From c61d18bed3d98ac46c81590099c8094f8d976c0d Mon Sep 17 00:00:00 2001 From: Auguste Olivry Date: Tue, 5 Jan 2021 14:14:30 +0100 Subject: [PATCH 84/89] Remove semicolon when prefix is empty --- beetsplug/discogs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 29a7e8585..f9a343cc9 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -557,7 +557,8 @@ class DiscogsPlugin(BeetsPlugin): title = track['title'] if self.config['index_tracks']: prefix = ', '.join(divisions) - title = ': '.join([prefix, title]) + if prefix != '': + title = ': '.join([prefix, title]) track_id = None medium, medium_index, _ = self.get_track_index(track['position']) artist, artist_id = MetadataSourcePlugin.get_artist( From a3917406147e76449f8c9fa3e3acfa9baf13429f Mon Sep 17 00:00:00 2001 From: Auguste Olivry Date: Tue, 5 Jan 2021 14:25:57 +0100 Subject: [PATCH 85/89] Update changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index b73320756..ce445496e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -176,6 +176,9 @@ New features: Fixes: +* :bug:`/plugins/discogs`: Fixed a bug with ``index_tracks`` options that + sometimes caused the index to be discarded. Also remove the extra semicolon + that was added when there is no index track. * :doc:`/plugins/subsonicupdate`: REST was using `POST` method rather `GET` method. Also includes better exception handling, response parsing, and tests. * :doc:`/plugins/the`: Fixed incorrect regex for 'the' that matched any From 345cf6e39b1954d0f80d388d660c8d61804e1320 Mon Sep 17 00:00:00 2001 From: Auguste Olivry Date: Tue, 5 Jan 2021 16:14:36 +0100 Subject: [PATCH 86/89] Move index handling inside coalesce_tracks --- beetsplug/discogs.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index f9a343cc9..915333ff7 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -499,12 +499,18 @@ class DiscogsPlugin(BeetsPlugin): else: # Promote the subtracks to real tracks, discarding the # index track, assuming the subtracks are physical tracks. - index_track = tracklist[-1] + index_track = tracklist.pop() # Fix artists when they are specified on the index track. if index_track.get('artists'): for subtrack in subtracks: if not subtrack.get('artists'): subtrack['artists'] = index_track['artists'] + # Concatenate index with track title when index_tracks + # option is set + if self.config['index_tracks']: + for subtrack in subtracks: + subtrack['title'] = '{}: {}'.format( + index_track['title'], subtrack['title']) tracklist.extend(subtracks) else: # Merge the subtracks, pick a title, and append the new track. From 34c38f41bd8c4a19eba78f22dae948c808fc5186 Mon Sep 17 00:00:00 2001 From: Auguste Olivry Date: Tue, 5 Jan 2021 16:15:36 +0100 Subject: [PATCH 87/89] Cosmetic fix --- beetsplug/discogs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 915333ff7..b1b1593cd 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -563,8 +563,8 @@ class DiscogsPlugin(BeetsPlugin): title = track['title'] if self.config['index_tracks']: prefix = ', '.join(divisions) - if prefix != '': - title = ': '.join([prefix, title]) + if prefix: + title = '{}: {}'.format(prefix, title) track_id = None medium, medium_index, _ = self.get_track_index(track['position']) artist, artist_id = MetadataSourcePlugin.get_artist( From 73a7723a156d7221d5ac8fee55493051891df837 Mon Sep 17 00:00:00 2001 From: Andrew Simmons Date: Wed, 6 Jan 2021 10:33:50 -0500 Subject: [PATCH 88/89] Add python version check --- beetsplug/mpdstats.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py index 39b045f9b..599aa7631 100644 --- a/beetsplug/mpdstats.py +++ b/beetsplug/mpdstats.py @@ -18,6 +18,7 @@ from __future__ import division, absolute_import, print_function import mpd import socket import select +import sys import time import os @@ -52,7 +53,13 @@ class MPDClientWrapper(object): self.music_directory = ( mpd_config['music_directory'].as_str()) - self.client = mpd.MPDClient(use_unicode=True) + if sys.version_info < (3, 0): + # On Python 2, use_unicode will enable the utf-8 mode for + # python-mpd2 + self.client = mpd.MPDClient(use_unicode=True) + else: + # On Python 3, python-mpd2 always uses Unicode + self.client = mpd.MPDClient() def connect(self): """Connect to the MPD. From 3117cc0d318428b079c965d62319ed663236e9eb Mon Sep 17 00:00:00 2001 From: Andrew Simmons Date: Wed, 6 Jan 2021 10:52:18 -0500 Subject: [PATCH 89/89] Update changelog --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ce445496e..dd8e08ca2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -293,6 +293,8 @@ Fixes: :bug:`2242` * :doc:`plugins/replaygain`: Disable parallel analysis on import by default. :bug:`3819` +* :doc:`/plugins/mpdstats`: Fix Python 2/3 compatibility + :bug:`3798` For plugin developers: