diff --git a/dvc/dependency/base.py b/dvc/dependency/base.py index 5fd70a5ad..62637cb14 100644 --- a/dvc/dependency/base.py +++ b/dvc/dependency/base.py @@ -31,6 +31,12 @@ class DependencyBase(object): self.project = stage.project self.path = path + def __repr__(self): + return "{class_name}: '{path}'".format( + class_name=type(self).__name__, + path=(self.path or 'No path') + ) + def __str__(self): return self.path diff --git a/dvc/exceptions.py b/dvc/exceptions.py index d98019142..462970807 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -50,6 +50,13 @@ class CircularDependencyError(DvcException): super(CircularDependencyError, self).__init__(msg.format(dependency)) +class ArgumentDuplicationError(DvcException): + def __init__(self, path): + assert isinstance(path, str) + msg = "File '{}' is specified more than once." + super(ArgumentDuplicationError, self).__init__(msg.format(path)) + + class MoveNotDataSourceError(DvcException): def __init__(self, path): msg = "Move is not permitted for stages that are not data sources. " \ diff --git a/dvc/project.py b/dvc/project.py index be08b2ae8..cbb5f39b4 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -58,6 +58,9 @@ class Project(object): self.updater.check() + def __repr__(self): + return "Project: '{root_dir}'".format(root_dir=self.root_dir) + @staticmethod def _find_root(root=None): if root is None: @@ -209,17 +212,6 @@ class Project(object): stages = [o.stage.relpath, out.stage.relpath] raise OutputDuplicationError(o.path, stages) - def _check_circular_dependency(self, deps, outs): - from dvc.exceptions import CircularDependencyError - - circular_dependencies = ( - set(file.path for file in deps) & - set(file.path for file in outs) - ) - - if circular_dependencies: - raise CircularDependencyError(circular_dependencies.pop()) - def add(self, fname, recursive=False): from dvc.stage import Stage @@ -244,9 +236,9 @@ class Project(object): self._files_to_git_add = [] with self.state: for f in fnames: - stage = Stage.loads(project=self, - outs=[f], - add=True) + stage = Stage.create(project=self, + outs=[f], + add=True) if stage is None: stages.append(stage) @@ -378,17 +370,17 @@ class Project(object): from dvc.stage import Stage with self.state: - stage = Stage.loads(project=self, - fname=fname, - cmd=cmd, - cwd=cwd, - outs=outs, - outs_no_cache=outs_no_cache, - metrics_no_cache=metrics_no_cache, - deps=deps, - overwrite=overwrite, - ignore_build_cache=ignore_build_cache, - remove_outs=remove_outs) + stage = Stage.create(project=self, + fname=fname, + cmd=cmd, + cwd=cwd, + outs=outs, + outs_no_cache=outs_no_cache, + metrics_no_cache=metrics_no_cache, + deps=deps, + overwrite=overwrite, + ignore_build_cache=ignore_build_cache, + remove_outs=remove_outs) if stage is None: return None @@ -397,7 +389,6 @@ class Project(object): self._check_cwd_specified_as_output(cwd, all_stages) self._check_output_duplication(stage.outs, all_stages) - self._check_circular_dependency(stage.deps, stage.outs) self._files_to_git_add = [] with self.state: @@ -413,16 +404,15 @@ class Project(object): def imp(self, url, out): from dvc.stage import Stage - stage = Stage.loads(project=self, - cmd=None, - deps=[url], - outs=[out]) + stage = Stage.create(project=self, + cmd=None, + deps=[url], + outs=[out]) if stage is None: return None self._check_output_duplication(stage.outs, self.stages()) - self._check_circular_dependency(stage.deps, stage.outs) self._files_to_git_add = [] with self.state: diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 8868e4156..5f491d922 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -42,6 +42,12 @@ class RemoteBase(object): def __init__(self, project, config): pass + def __repr__(self): + return "{class_name}: '{url}'".format( + class_name=type(self).__name__, + url=(self.url or 'No url') + ) + def compat_config(config): return config.copy() diff --git a/dvc/remote/http.py b/dvc/remote/http.py index 10390715b..bdc49e12d 100644 --- a/dvc/remote/http.py +++ b/dvc/remote/http.py @@ -33,6 +33,7 @@ class RemoteHTTP(RemoteBase): def __init__(self, project, config): self.project = project self.cache_dir = config.get(Config.SECTION_REMOTE_URL) + self.url = self.cache_dir @property def prefix(self): diff --git a/dvc/scm.py b/dvc/scm.py index e32ed2913..49f5e6bbe 100644 --- a/dvc/scm.py +++ b/dvc/scm.py @@ -18,6 +18,12 @@ class Base(object): self.project = project self.root_dir = root_dir + def __repr__(self): + return "{class_name}: '{directory}'".format( + class_name=type(self).__name__, + directory=self.dir + ) + @property def dir(self): return None diff --git a/dvc/stage.py b/dvc/stage.py index 952197872..2dc1abc74 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -113,12 +113,20 @@ class Stage(object): self.md5 = md5 self.locked = locked + def __repr__(self): + return "Stage: '{path}'".format( + path=self.relpath if self.path else 'No path' + ) + @property def relpath(self): return os.path.relpath(self.path) @property def is_data_source(self): + """ + Whether the stage file was created with `dvc add`. + """ return self.cmd is None @staticmethod @@ -147,13 +155,20 @@ class Stage(object): @property def is_callback(self): + """ + A callback stage is always considered as changed, + so it runs on every `dvc repro` call. + """ return not self.is_data_source and len(self.deps) == 0 @property def is_import(self): - return not self.cmd and \ - len(self.deps) == 1 and \ - len(self.outs) == 1 + """ + Whether the stage file was created with `dvc import`. + """ + return (not self.cmd + and len(self.deps) == 1 + and len(self.outs) == 1) def _changed_deps(self, log): if self.locked: @@ -210,6 +225,9 @@ class Stage(object): return ret def remove_outs(self, ignore_remove=False): + """ + Used mainly for `dvc remove --outs` + """ for out in self.outs: out.remove(ignore_remove=ignore_remove) @@ -254,7 +272,7 @@ class Stage(object): raise StageFileFormatError(fname, exc) @staticmethod - def loadd(project, d, path): + def _loadd(project, d, path): Stage.validate(d, fname=os.path.relpath(path)) path = os.path.abspath(path) cwd = os.path.dirname(path) @@ -303,10 +321,10 @@ class Stage(object): if not os.path.realpath(cwd).startswith(proj_dir): raise StageBadCwdError(cwd) + @property def is_cached(self): """ - Checks if this stage has been already ran and saved to the same - dvc file. + Checks if this stage has been already ran and stored """ from dvc.remote.local import RemoteLOCAL from dvc.remote.s3 import RemoteS3 @@ -335,19 +353,19 @@ class Stage(object): return old_d == new_d @staticmethod - def loads(project=None, - cmd=None, - deps=[], - outs=[], - outs_no_cache=[], - metrics_no_cache=[], - fname=None, - cwd=os.curdir, - locked=False, - add=False, - overwrite=True, - ignore_build_cache=False, - remove_outs=False): + def create(project=None, + cmd=None, + deps=[], + outs=[], + outs_no_cache=[], + metrics_no_cache=[], + fname=None, + cwd=os.curdir, + locked=False, + add=False, + overwrite=True, + ignore_build_cache=False, + remove_outs=False): stage = Stage(project=project, cwd=cwd, @@ -360,6 +378,9 @@ class Stage(object): use_cache=False, metric=True) stage.deps = dependency.loads_from(stage, deps) + stage._check_circular_dependency() + stage._check_duplicated_arguments() + if fname is not None and os.path.basename(fname) != fname: msg = "Stage file name '{}' should not contain subdirectories. " \ "Use '-c|--cwd' to change location of the stage file." @@ -385,7 +406,7 @@ class Stage(object): stage.unprotect_outs() if os.path.exists(path): - if not ignore_build_cache and stage.is_cached(): + if not ignore_build_cache and stage.is_cached: Logger.info('Stage is cached, skipping.') return None @@ -423,7 +444,7 @@ class Stage(object): raise StageFileIsNotDvcFileError(fname) with open(fname, 'r') as fd: - return Stage.loadd(project, yaml.safe_load(fd) or dict(), fname) + return Stage._loadd(project, yaml.safe_load(fd) or dict(), fname) def dumpd(self): ret = {} @@ -505,6 +526,27 @@ class Stage(object): "https://github.com/iterative/dvc/issues/1307. " self.project.logger.warn(msg) + def _check_circular_dependency(self): + from dvc.exceptions import CircularDependencyError + + circular_dependencies = ( + set(file.path for file in self.deps) & + set(file.path for file in self.outs) + ) + + if circular_dependencies: + raise CircularDependencyError(circular_dependencies.pop()) + + def _check_duplicated_arguments(self): + from dvc.exceptions import ArgumentDuplicationError + from collections import Counter + + count = Counter(file.path for file in self.deps + self.outs) + + for file, occurrence in count.items(): + if occurrence > 1: + raise ArgumentDuplicationError(file) + def _run(self): self._check_missing_deps() executable = os.getenv('SHELL') if os.name != 'nt' else None diff --git a/dvc/state.py b/dvc/state.py index ce1dd4a90..7b23e5fbb 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -74,10 +74,6 @@ class State(object): def __exit__(self, type, value, tb): self.dump() - @staticmethod - def init(project): - return State(project) - def _collect(self, path): if os.path.isdir(path): return self.project.cache.local.collect_dir_cache(path) @@ -124,7 +120,7 @@ class State(object): assert n >= 0 return n - def _load(self, empty=False): + def _prepare_db(self, empty=False): from dvc import VERSION if not empty: @@ -183,7 +179,7 @@ class State(object): # Try loading once to check that the file is indeed a database # and reformat it if it is not. try: - self._load(empty=empty) + self._prepare_db(empty=empty) return except sqlite3.DatabaseError: self.c.close() @@ -268,8 +264,7 @@ class State(object): def _do_update(self, path): """ - Search for the current inode on the table or update it with - gathered information. + Make sure the stored info for the given path is up to date. """ if not os.path.exists(path): return (None, None) diff --git a/tests/test_run.py b/tests/test_run.py index 4cf05012a..40bbc27f3 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -11,6 +11,7 @@ from dvc.stage import StageFileBadNameError, MissingDep from dvc.stage import StageBadCwdError, StageFileAlreadyExistsError from dvc.exceptions import (OutputDuplicationError, CircularDependencyError, + ArgumentDuplicationError, WorkingDirectoryAsOutputError) from tests.basic_env import TestDvc @@ -121,6 +122,29 @@ class TestRunCircularDependency(TestDvc): fname='circular-dependency.dvc') +class TestRunDuplicatedArguments(TestDvc): + def test(self): + with self.assertRaises(ArgumentDuplicationError): + self.dvc.run(cmd='', + deps=[], + outs=[self.FOO, self.FOO], + fname='circular-dependency.dvc') + + def test_outs_no_cache(self): + with self.assertRaises(ArgumentDuplicationError): + self.dvc.run(cmd='', + outs=[self.FOO], + outs_no_cache=[self.FOO], + fname='circular-dependency.dvc') + + def test_non_normalized_paths(self): + with self.assertRaises(ArgumentDuplicationError): + self.dvc.run(cmd='', + deps=[], + outs=['foo', './foo'], + fname='circular-dependency.dvc') + + class TestRunWorkingDirectoryAsOutput(TestDvc): def test(self): self.dvc.run(cmd='',