From 4e379976a2291a4df5e8979f5973d71d329d9d87 Mon Sep 17 00:00:00 2001 From: Peter Rowlands Date: Mon, 3 Aug 2020 14:27:02 +0900 Subject: [PATCH 1/4] restore Experiments.chdir * used in experiments.show --- dvc/repo/experiments/__init__.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py index fe899762d6..d3e3f18f8a 100644 --- a/dvc/repo/experiments/__init__.py +++ b/dvc/repo/experiments/__init__.py @@ -3,6 +3,7 @@ import re import tempfile from concurrent.futures import ProcessPoolExecutor, as_completed +from contextlib import contextmanager from typing import Iterable, Optional from funcy import cached_property @@ -79,6 +80,13 @@ def exp_dvc(self): return Repo(self.exp_dvc_dir) + @contextmanager + def chdir(self): + cwd = os.getcwd() + os.chdir(self.exp_dvc.root_dir) + yield self.exp_dvc.root_dir + os.chdir(cwd) + @cached_property def args_file(self): return os.path.join(self.exp_dvc.tmp_dir, self.PACKED_ARGS_FILE) From 11a2e9914b6143b27f9cf00631d63d519ec24dcb Mon Sep 17 00:00:00 2001 From: Peter Rowlands Date: Mon, 3 Aug 2020 15:35:22 +0900 Subject: [PATCH 2/4] experiments: make checkout work without `git reset --hard` --- dvc/repo/experiments/__init__.py | 34 ++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py index d3e3f18f8a..d0b1f3cdd7 100644 --- a/dvc/repo/experiments/__init__.py +++ b/dvc/repo/experiments/__init__.py @@ -184,7 +184,7 @@ def reproduce_one(self, queue=False, **kwargs): return [stash_rev] results = self.reproduce([stash_rev], keep_stash=False) for exp_rev in results: - self.checkout_exp(exp_rev, force=True) + self.checkout_exp(exp_rev) return results def reproduce_queued(self, **kwargs): @@ -346,30 +346,44 @@ def _collect_output(self, rev: str, executor: ExperimentExecutor): src = executor.path_info / relpath(fname, tree.tree_root) copyfile(src, fname) - def checkout_exp(self, rev, force=False): + def checkout_exp(self, rev): """Checkout an experiment to the user's workspace.""" from git.exc import GitCommandError from dvc.repo.checkout import _checkout as dvc_checkout - if force: - self.repo.scm.repo.git.reset(hard=True) self._scm_checkout(rev) tmp = tempfile.NamedTemporaryFile(delete=False).name self.scm.repo.head.commit.diff("HEAD~1", patch=True, output=tmp) + + logger.debug("Stashing workspace changes.") + self.repo.scm.repo.git.stash("push") + try: if os.path.getsize(tmp): logger.debug("Patching local workspace") self.repo.scm.repo.git.apply(tmp, reverse=True) - dvc_checkout(self.repo) + need_checkout = True + else: + need_checkout = False except GitCommandError: - raise DvcException( - "Checkout failed, experiment contains changes which " - "conflict with your current workspace. To overwrite " - "your workspace, use `dvc experiments checkout --force`." - ) + raise DvcException("failed to apply experiment changes.") finally: remove(tmp) + self._unstash_workspace() + + if need_checkout: + dvc_checkout(self.repo) + + def _unstash_workspace(self): + # Essentially we want `git stash pop` with `-X ours` merge strategy + # to prefer the applied experiment changes over stashed workspace + # changes. git stash doesn't support merge strategy parameters, but we + # can do it ourselves with checkout/reset. + logger.debug("Unstashing workspace changes.") + self.repo.scm.repo.git.checkout("--ours", "stash@{0}", "--", ".") + self.repo.scm.repo.git.reset("HEAD") + self.repo.scm.repo.git.stash("drop", "stash@{0}") def checkout(self, *args, **kwargs): from dvc.repo.experiments.checkout import checkout From d561c0712ef012e5059ec3186acc43649ffcca04 Mon Sep 17 00:00:00 2001 From: Peter Rowlands Date: Mon, 3 Aug 2020 15:37:01 +0900 Subject: [PATCH 3/4] experiments: remove --force parameter --- dvc/command/experiments.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/dvc/command/experiments.py b/dvc/command/experiments.py index a8d591f4ad..ffa14a4317 100644 --- a/dvc/command/experiments.py +++ b/dvc/command/experiments.py @@ -146,9 +146,7 @@ def run(self): if not self.repo.experiments: return 0 - self.repo.experiments.checkout( - self.args.experiment, force=self.args.force - ) + self.repo.experiments.checkout(self.args.experiment) return 0 @@ -287,14 +285,6 @@ def add_parser(subparsers, parent_parser): help=EXPERIMENTS_CHECKOUT_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - experiments_checkout_parser.add_argument( - "-f", - "--force", - action="store_true", - default=False, - help="Overwrite your current workspace with changes from the " - "experiment.", - ) experiments_checkout_parser.add_argument( "experiment", help="Checkout this experiment.", ) From 31dd33785ead6097e45045b7474a9202cbbdf323 Mon Sep 17 00:00:00 2001 From: Peter Rowlands Date: Mon, 3 Aug 2020 15:56:11 +0900 Subject: [PATCH 4/4] experiments: reject checkout if current workspace commit doesnt match expected baseline --- dvc/repo/experiments/__init__.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py index d0b1f3cdd7..bcabf99966 100644 --- a/dvc/repo/experiments/__init__.py +++ b/dvc/repo/experiments/__init__.py @@ -27,7 +27,15 @@ def hash_exp(stages): class UnchangedExperimentError(DvcException): def __init__(self, rev): - super().__init__("Experiment identical to baseline '{rev[:7]}'.") + super().__init__(f"Experiment identical to baseline '{rev[:7]}'.") + self.rev = rev + + +class BaselineMismatchError(DvcException): + def __init__(self, rev): + super().__init__( + f"Experiment is not derived from current baseline '{rev[:7]}'." + ) self.rev = rev @@ -128,7 +136,7 @@ def _scm_checkout(self, rev): self.scm.repo.heads[0].checkout() if not Git.is_sha(rev) or not self.scm.has_rev(rev): self.scm.pull() - logger.debug("Checking out base experiment commit '%s'", rev) + logger.debug("Checking out experiment commit '%s'", rev) self.scm.checkout(rev) def _stash_exp(self, *args, **kwargs): @@ -351,6 +359,7 @@ def checkout_exp(self, rev): from git.exc import GitCommandError from dvc.repo.checkout import _checkout as dvc_checkout + self._check_baseline(rev) self._scm_checkout(rev) tmp = tempfile.NamedTemporaryFile(delete=False).name @@ -375,6 +384,14 @@ def checkout_exp(self, rev): if need_checkout: dvc_checkout(self.repo) + def _check_baseline(self, exp_rev): + baseline_sha = self.repo.scm.get_rev() + exp_commit = self.scm.repo.rev_parse(exp_rev) + for parent in exp_commit.parents: + if parent.hexsha == baseline_sha: + return + raise BaselineMismatchError(baseline_sha) + def _unstash_workspace(self): # Essentially we want `git stash pop` with `-X ours` merge strategy # to prefer the applied experiment changes over stashed workspace