diff --git a/dvc/command/imp.py b/dvc/command/imp.py index 452a481d5f..ba39cccdb9 100644 --- a/dvc/command/imp.py +++ b/dvc/command/imp.py @@ -52,6 +52,6 @@ def add_parser(subparsers, parent_parser): "-o", "--out", nargs="?", help="Destination path to put data to." ) import_parser.add_argument( - "--rev", nargs="?", help="DVC repository git revision." + "--rev", nargs="?", help="Git revision in repository to update from." ) import_parser.set_defaults(func=CmdImport) diff --git a/dvc/command/update.py b/dvc/command/update.py index 2b800c9fe0..70e9ce8b98 100644 --- a/dvc/command/update.py +++ b/dvc/command/update.py @@ -16,7 +16,7 @@ def run(self): ret = 0 for target in self.args.targets: try: - self.repo.update(target) + self.repo.update(target, rev=self.args.rev) except DvcException: logger.exception("failed to update '{}'.".format(target)) ret = 1 @@ -35,4 +35,7 @@ def add_parser(subparsers, parent_parser): update_parser.add_argument( "targets", nargs="+", help="DVC-files to update." ) + update_parser.add_argument( + "--rev", nargs="?", help="Git revision in repository to update from." + ) update_parser.set_defaults(func=CmdUpdate) diff --git a/dvc/dependency/base.py b/dvc/dependency/base.py index 4f52bf5d05..9341137af6 100644 --- a/dvc/dependency/base.py +++ b/dvc/dependency/base.py @@ -29,5 +29,5 @@ class DependencyBase(object): IsNotFileOrDirError = DependencyIsNotFileOrDirError IsStageFileError = DependencyIsStageFileError - def update(self): + def update(self, rev=None): pass diff --git a/dvc/dependency/repo.py b/dvc/dependency/repo.py index df35e45235..c5010e2fa6 100644 --- a/dvc/dependency/repo.py +++ b/dvc/dependency/repo.py @@ -114,6 +114,9 @@ def download(self, to): self.def_path, self.def_repo[self.PARAM_URL] ) - def update(self): - with self._make_repo(rev_lock=None) as repo: + def update(self, rev=None): + if not rev: + rev = self.def_repo.get("rev") + + with self._make_repo(rev=rev, rev_lock=None) as repo: self.def_repo[self.PARAM_REV_LOCK] = repo.scm.get_rev() diff --git a/dvc/repo/update.py b/dvc/repo/update.py index 9533bb6d36..c980e868b7 100644 --- a/dvc/repo/update.py +++ b/dvc/repo/update.py @@ -2,10 +2,11 @@ @locked -def update(self, target): +def update(self, target, rev=None): from dvc.stage import Stage stage = Stage.load(self, target) - stage.update() + + stage.update(rev=rev) stage.dump() diff --git a/dvc/stage.py b/dvc/stage.py index 236a5f2d39..294d4bf10f 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -132,6 +132,13 @@ def __init__(self, missing_files): super(MissingDataSource, self).__init__(msg) +class UpdateWithRevNotPossibleError(DvcException): + def __init__(self): + super(UpdateWithRevNotPossibleError, self).__init__( + "Revision option (`--rev`) is not a feature of non-Git sources." + ) + + @decorator def rwlocked(call, read=None, write=None): import sys @@ -395,11 +402,14 @@ def reproduce(self, interactive=False, **kwargs): return self - def update(self): + def update(self, rev=None): if not self.is_repo_import and not self.is_import: raise StageUpdateError(self.relpath) - self.deps[0].update() + if not self.is_repo_import and rev: + raise UpdateWithRevNotPossibleError() + + self.deps[0].update(rev) locked = self.locked self.locked = False try: diff --git a/tests/func/test_update.py b/tests/func/test_update.py index 888bea9290..ea428497a3 100644 --- a/tests/func/test_update.py +++ b/tests/func/test_update.py @@ -1,9 +1,12 @@ import filecmp import os import shutil +import pytest +from dvc.stage import UpdateWithRevNotPossibleError from dvc.external_repo import clean_repos from dvc.repo import Repo +from dvc.utils.compat import fspath def test_update_import(dvc_repo, erepo): @@ -72,3 +75,30 @@ def test_update_import_url(repo_dir, dvc_repo): assert os.path.exists(dst) assert os.path.isfile(dst) assert filecmp.cmp(src, dst, shallow=False) + + +def test_update_rev(tmp_dir, scm, dvc, erepo_dir, monkeypatch): + with monkeypatch.context() as m: + m.chdir(fspath(erepo_dir)) + erepo_dir.scm.checkout("feature", create_new=True) + erepo_dir.dvc_gen( + "foo", "foo content", commit="create foo on feature branch" + ) + erepo_dir.scm.checkout("master") + + stage = dvc.imp(fspath(erepo_dir), "foo", "foo_imported") + dvc.update(stage.path, rev="feature") + + assert (tmp_dir / "foo_imported").read_text() == "foo content" + + +def test_update_rev_non_git_failure(repo_dir, dvc_repo): + src = "file" + dst = src + "_imported" + + shutil.copyfile(repo_dir.FOO, src) + + stage = dvc_repo.imp_url(src, dst) + + with pytest.raises(UpdateWithRevNotPossibleError): + dvc_repo.update(stage.path, rev="dev") diff --git a/tests/unit/command/test_update.py b/tests/unit/command/test_update.py index c44def6fa6..ce72525f6e 100644 --- a/tests/unit/command/test_update.py +++ b/tests/unit/command/test_update.py @@ -1,15 +1,33 @@ +import logging + from dvc.cli import parse_args from dvc.command.update import CmdUpdate +from dvc.stage import UpdateWithRevNotPossibleError def test_update(dvc_repo, mocker): targets = ["target1", "target2", "target3"] - cli_args = parse_args(["update"] + targets) + cli_args = parse_args(["update", "--rev", "develop"] + targets) assert cli_args.func == CmdUpdate cmd = cli_args.func(cli_args) m = mocker.patch("dvc.repo.Repo.update") assert cmd.run() == 0 - calls = [mocker.call(target) for target in targets] + calls = [mocker.call(target, rev="develop") for target in targets] m.assert_has_calls(calls) + + +def test_update_rev_failed(mocker, caplog): + targets = ["target1", "target2", "target3"] + cli_args = parse_args(["update", "--rev", "develop"] + targets) + assert cli_args.func == CmdUpdate + + cmd = cli_args.func(cli_args) + with mocker.patch.object( + cmd.repo, "update", side_effect=UpdateWithRevNotPossibleError() + ): + with caplog.at_level(logging.ERROR, logger="dvc"): + assert cmd.run() == 1 + expected_error = "Revision option (`--rev`) is not a feature of non-Git sources." + assert expected_error in caplog.text