diff --git a/NEWS b/NEWS index 363fb8f1e..0745302d6 100644 --- a/NEWS +++ b/NEWS @@ -17,9 +17,18 @@ * Relatedly, the -L flag to the "import" command makes it take a query as its argument instead of a list of directories. The matched albums (or items, depending on the -s flag) are then re-imported. +* A new flag -i to the import command runs incremental imports, keeping + track of and skipping previously-imported directories. This has the + effect of making repeated import commands pick up only newly-added + directories. The "import_incremental" config option makes this the + default. * When pruning directories, "clutter" files such as .DS_Store and Thumbs.db are ignored (and removed with otherwise-empty directories). +* A new plugin, called "web", encapsulates a simple Web-based GUI for + beets. The current iteration can browse the library and play music + in browsers that support HTML5 Audio. +* Files are no longer silently overwritten when moving and copying files. * Handle exceptions thrown when running Mutagen. * Fix a missing __future__ import in embedart on Python 2.5. * Fix ID3 and MPEG-4 tag names for the album-artist field. diff --git a/beets/importer.py b/beets/importer.py index bc2101d5a..a6edfc949 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -187,6 +187,18 @@ def _infer_album_fields(task): for k, v in changes.iteritems(): setattr(item, k, v) +def _open_state(): + """Reads the state file, returning a dictionary.""" + try: + with open(STATE_FILE) as f: + return pickle.load(f) + except IOError: + return {} +def _save_state(state): + """Writes the state dictionary out to disk.""" + with open(STATE_FILE, 'w') as f: + pickle.dump(state, f) + # Utilities for reading and writing the beets progress file, which # allows long tagging tasks to be resumed when they pause (or crash). @@ -196,11 +208,9 @@ def progress_set(toppath, path): `path`. If path is None, then clear the progress value (indicating that the tagging completed). """ - try: - with open(STATE_FILE) as f: - state = pickle.load(f) - except IOError: - state = {PROGRESS_KEY: {}} + state = _open_state() + if PROGRESS_KEY not in state: + state[PROGRESS_KEY] = {} if path is None: # Remove progress from file. @@ -209,20 +219,41 @@ def progress_set(toppath, path): else: state[PROGRESS_KEY][toppath] = path - with open(STATE_FILE, 'w') as f: - pickle.dump(state, f) + _save_state(state) def progress_get(toppath): """Get the last successfully tagged subpath of toppath. If toppath has no progress information, returns None. """ - try: - with open(STATE_FILE) as f: - state = pickle.load(f) - except IOError: + state = _open_state() + if PROGRESS_KEY not in state: return None return state[PROGRESS_KEY].get(toppath) +# Similarly, utilities for manipulating the "incremental" import log. +# This keeps track of all directories that were ever imported, which +# allows the importer to only import new stuff. +HISTORY_KEY = 'taghistory' +def history_add(path): + """Indicate that the import of `path` is completed and should not + be repeated in incremental imports. + """ + state = _open_state() + if HISTORY_KEY not in state: + state[HISTORY_KEY] = set() + + state[HISTORY_KEY].add(path) + + _save_state(state) +def history_get(): + """Get the set of completed paths in incremental imports. + """ + state = _open_state() + if HISTORY_KEY not in state: + return set() + return state[HISTORY_KEY] + + # The configuration structure. class ImportConfig(object): @@ -234,7 +265,7 @@ class ImportConfig(object): 'quiet_fallback', 'copy', 'write', 'art', 'delete', 'choose_match_func', 'should_resume_func', 'threaded', 'autot', 'singletons', 'timid', 'choose_item_func', - 'query'] + 'query', 'incremental'] def __init__(self, **kwargs): for slot in self._fields: setattr(self, slot, kwargs[slot]) @@ -243,11 +274,16 @@ class ImportConfig(object): if self.paths: self.paths = map(normpath, self.paths) + # Incremental and progress are mutually exclusive. + if self.incremental: + self.resume = False + # When based on a query instead of directories, never # save progress or try to resume. if self.query is not None: self.paths = None self.resume = False + self.incremental = False # The importer task class. @@ -352,6 +388,12 @@ class ImportTask(object): # album task, which implies the same. progress_set(self.toppath, self.path) + def save_history(self): + """Save the directory in the history for incremental imports. + """ + if self.sentinel or self.is_album: + history_add(self.path) + # Logical decisions. def should_write_tags(self): """Should new info be written to the files' metadata?""" @@ -398,6 +440,10 @@ def read_tasks(config): else: # Clear progress; we're starting from the top. progress_set(path, None) + + # Look for saved incremental directories. + if config.incremental: + history_dirs = history_get() for toppath in config.paths: # Check whether the path is to a file. @@ -410,6 +456,7 @@ def read_tasks(config): if progress: resume_dir = resume_dirs.get(toppath) for path, items in autotag.albums_in_dir(toppath): + # Skip according to progress. if progress and resume_dir: # We're fast-forwarding to resume a previous tagging. if path == resume_dir: @@ -418,6 +465,10 @@ def read_tasks(config): resume_dir = None continue + # When incremental, skip paths in the history. + if config.incremental and path in history_dirs: + continue + # Yield all the necessary tasks. if config.singletons: for item in items: @@ -634,6 +685,8 @@ def finalize(config): if task.should_skip(): if config.resume is not False: task.save_progress() + if config.incremental: + task.save_history() continue items = task.items if task.is_album else [task.item] @@ -657,6 +710,8 @@ def finalize(config): # Update progress. if config.resume is not False: task.save_progress() + if config.incremental: + task.save_history() # Singleton pipeline stages. diff --git a/beets/library.py b/beets/library.py index 51d376b36..90f7952a3 100644 --- a/beets/library.py +++ b/beets/library.py @@ -15,7 +15,6 @@ import sqlite3 import os import re -import shutil import sys from string import Template import logging @@ -242,12 +241,9 @@ class Item(object): if not samefile(self.path, dest): if copy: - # copyfile rather than copy will not copy permissions - # bits, thus possibly making the copy writable even when - # the original is read-only. - shutil.copyfile(syspath(self.path), syspath(dest)) + util.copy(self.path, dest) else: - shutil.move(syspath(self.path), syspath(dest)) + util.move(self.path, dest) # Either copying or moving succeeded, so update the stored path. old_path = self.path @@ -1179,9 +1175,9 @@ class Album(BaseAlbum): new_art = self.art_destination(old_art, newdir) if new_art != old_art: if copy: - shutil.copy(syspath(old_art), syspath(new_art)) + util.copy(old_art, new_art) else: - shutil.move(syspath(old_art), syspath(new_art)) + util.move(old_art, new_art) self.artpath = new_art if not copy: # Prune old path. util.prune_dirs(os.path.dirname(old_art), @@ -1236,5 +1232,5 @@ class Album(BaseAlbum): # Normal operation. if oldart == artdest: util.soft_remove(oldart) - shutil.copyfile(syspath(path), syspath(artdest)) + util.copy(path, artdest) self.artpath = artdest diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 456a26ec6..6bd015576 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -84,6 +84,7 @@ DEFAULT_IMPORT_ART = True DEFAULT_IMPORT_QUIET = False DEFAULT_IMPORT_QUIET_FALLBACK = 'skip' DEFAULT_IMPORT_RESUME = None # "ask" +DEFAULT_IMPORT_INCREMENTAL = False DEFAULT_THREADED = True DEFAULT_COLOR = True @@ -483,7 +484,7 @@ def choose_item(task, config): def import_files(lib, paths, copy, write, autot, logpath, art, threaded, color, delete, quiet, resume, quiet_fallback, singletons, - timid, query): + timid, query, incremental): """Import the files in the given list of paths, tagging each leaf directory as an album. If copy, then the files are copied into the library folder. If write, then new metadata is written to the @@ -544,6 +545,7 @@ def import_files(lib, paths, copy, write, autot, logpath, art, threaded, timid = timid, choose_item_func = choose_item, query = query, + incremental = incremental, ) # If we were logging, close the file. @@ -587,6 +589,8 @@ import_cmd.parser.add_option('-t', '--timid', dest='timid', action='store_true', help='always confirm all actions') import_cmd.parser.add_option('-L', '--library', dest='library', action='store_true', help='retag items matching a query') +import_cmd.parser.add_option('-i', '--incremental', dest='incremental', + action='store_true', help='skip already-imported directories') def import_func(lib, config, opts, args): copy = opts.copy if opts.copy is not None else \ ui.config_val(config, 'beets', 'import_copy', @@ -612,6 +616,9 @@ def import_func(lib, config, opts, args): DEFAULT_IMPORT_TIMID, bool) logpath = opts.logpath if opts.logpath is not None else \ ui.config_val(config, 'beets', 'import_log', None) + incremental = opts.incremental if opts.incremental is not None else \ + ui.config_val(config, 'beets', 'import_incremental', + DEFAULT_IMPORT_INCREMENTAL, bool) # Resume has three options: yes, no, and "ask" (None). resume = opts.resume if opts.resume is not None else \ @@ -638,7 +645,7 @@ def import_func(lib, config, opts, args): import_files(lib, paths, copy, write, autot, logpath, art, threaded, color, delete, quiet, resume, quiet_fallback, singletons, - timid, query) + timid, query, incremental) import_cmd.func = import_func default_commands.append(import_cmd) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index f73811ac9..67a878342 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -185,6 +185,32 @@ def soft_remove(path): if os.path.exists(path): os.remove(path) +def _assert_not_exists(path, pathmod=None): + """Raises an OSError if the path exists.""" + pathmod = pathmod or os.path + if pathmod.exists(path): + raise OSError('file exists: %s' % path) + +def copy(path, dest, replace=False, pathmod=None): + """Copy a plain file. Permissions are not copied. If dest already + exists, raises an OSError unless replace is True. Paths are + translated to system paths before the syscall. + """ + path = syspath(path) + dest = syspath(dest) + _assert_not_exists(dest, pathmod) + return shutil.copyfile(path, dest) + +def move(path, dest, replace=False, pathmod=None): + """Rename a file. dest may not be a directory. If dest already + exists, raises an OSError unless replace is True. Paths are + translated to system paths. + """ + path = syspath(path) + dest = syspath(dest) + _assert_not_exists(dest, pathmod) + return shutil.move(path, dest) + # Note: POSIX actually supports \ and : -- I just think they're # a pain. And ? has caused problems for some. CHAR_REPLACE = [ diff --git a/test/_common.py b/test/_common.py index 59f46d26e..f3cfff073 100644 --- a/test/_common.py +++ b/test/_common.py @@ -87,6 +87,7 @@ def iconfig(lib, **kwargs): choose_item_func = lambda x, y: importer.action.SKIP, timid = False, query = None, + incremental = False, ) for k, v in kwargs.items(): setattr(config, k, v) diff --git a/test/test_files.py b/test/test_files.py index da175fcb8..d99292959 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -248,6 +248,8 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): self.assertTrue('testotherdir' in newart) def test_setart_copies_image(self): + os.remove(self.art) + newart = os.path.join(self.libdir, 'newart.jpg') touch(newart) i2 = item() @@ -261,6 +263,8 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): self.assertTrue(os.path.exists(ai.artpath)) def test_setart_to_existing_art_works(self): + os.remove(self.art) + # Original art. newart = os.path.join(self.libdir, 'newart.jpg') touch(newart) @@ -293,6 +297,8 @@ class ArtFileTest(unittest.TestCase, _common.ExtraAsserts): self.assertTrue(os.path.exists(ai.artpath)) def test_setart_sets_permissions(self): + os.remove(self.art) + newart = os.path.join(self.libdir, 'newart.jpg') touch(newart) os.chmod(newart, 0400) # read-only @@ -393,6 +399,39 @@ class SoftRemoveTest(unittest.TestCase, _common.ExtraAsserts): except OSError: self.fail('OSError when removing path') +class SafeMoveCopyTest(unittest.TestCase): + def setUp(self): + self.path = os.path.join(_common.RSRC, 'testfile') + touch(self.path) + self.otherpath = os.path.join(_common.RSRC, 'testfile2') + touch(self.otherpath) + self.dest = self.path + '.dest' + def tearDown(self): + if os.path.exists(self.path): + os.remove(self.path) + if os.path.exists(self.otherpath): + os.remove(self.otherpath) + if os.path.exists(self.dest): + os.remove(self.dest) + + def test_existence_check(self): + with self.assertRaises(OSError): + util._assert_not_exists(self.path) + + def test_successful_move(self): + util.move(self.path, self.dest) + + def test_successful_copy(self): + util.copy(self.path, self.dest) + + def test_unsuccessful_move(self): + with self.assertRaises(OSError): + util.move(self.path, self.otherpath) + + def test_unsuccessful_copy(self): + with self.assertRaises(OSError): + util.copy(self.path, self.otherpath) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_importer.py b/test/test_importer.py index 514d5a6b1..4607f1087 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -101,6 +101,7 @@ class NonAutotaggedImportTest(unittest.TestCase): choose_item_func = None, timid = False, query = None, + incremental = False, ) return paths diff --git a/test/test_ui.py b/test/test_ui.py index 33984013b..6b56046bd 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -428,7 +428,8 @@ class ImportTest(unittest.TestCase): def test_quiet_timid_disallowed(self): self.assertRaises(ui.UserError, commands.import_files, None, [], False, False, False, None, False, False, - False, False, True, False, None, False, True, None) + False, False, True, False, None, False, True, None, + False) class InputTest(unittest.TestCase): def setUp(self):