diff --git a/dvc/commands/add.py b/dvc/commands/add.py index d5dd0d004..575c36ce1 100644 --- a/dvc/commands/add.py +++ b/dvc/commands/add.py @@ -62,6 +62,7 @@ class CmdAdd(CmdBase): remote=self.args.remote, to_remote=self.args.to_remote, jobs=self.args.jobs, + force=self.args.force, ) except DvcException: @@ -140,6 +141,13 @@ def add_parser(subparsers, parent_parser): ), metavar="", ) + parser.add_argument( + "-f", + "--force", + action="store_true", + default=False, + help="Override local file or folder if exists.", + ) _add_annotating_args(parser) parser.add_argument( diff --git a/dvc/commands/get.py b/dvc/commands/get.py index c8cc81795..9ede8bfd9 100644 --- a/dvc/commands/get.py +++ b/dvc/commands/get.py @@ -36,6 +36,7 @@ class CmdGet(CmdBaseNoRepo): out=self.args.out, rev=self.args.rev, jobs=self.args.jobs, + force=self.args.force, ) return 0 except CloneError: @@ -94,4 +95,11 @@ def add_parser(subparsers, parent_parser): ), metavar="", ) + get_parser.add_argument( + "-f", + "--force", + action="store_true", + default=False, + help="Override local file or folder if exists.", + ) get_parser.set_defaults(func=CmdGet) diff --git a/dvc/commands/get_url.py b/dvc/commands/get_url.py index 0b75fc47c..e96deb50e 100644 --- a/dvc/commands/get_url.py +++ b/dvc/commands/get_url.py @@ -14,7 +14,12 @@ class CmdGetUrl(CmdBaseNoRepo): from dvc.repo import Repo try: - Repo.get_url(self.args.url, out=self.args.out, jobs=self.args.jobs) + Repo.get_url( + self.args.url, + out=self.args.out, + jobs=self.args.jobs, + force=self.args.force, + ) return 0 except DvcException: logger.exception("failed to get '%s'", self.args.url) @@ -46,4 +51,11 @@ def add_parser(subparsers, parent_parser): ), metavar="", ) + get_parser.add_argument( + "-f", + "--force", + action="store_true", + default=False, + help="Override local file or folder if exists.", + ) get_parser.set_defaults(func=CmdGetUrl) diff --git a/dvc/commands/imp_url.py b/dvc/commands/imp_url.py index 30b72f9db..4fdeb17e3 100644 --- a/dvc/commands/imp_url.py +++ b/dvc/commands/imp_url.py @@ -25,6 +25,7 @@ class CmdImportUrl(CmdBase): labels=self.args.labels, meta=self.args.meta, jobs=self.args.jobs, + force=self.args.force, version_aware=self.args.version_aware, ) except DvcException: @@ -112,6 +113,13 @@ def add_parser(subparsers, parent_parser): ), metavar="", ) + import_parser.add_argument( + "-f", + "--force", + action="store_true", + default=False, + help="Override local file or folder if exists.", + ) import_parser.add_argument( "--version-aware", action="store_true", diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 84cc99f28..373ab83ad 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -1,4 +1,5 @@ """Exceptions raised by the dvc.""" +import errno from typing import Dict, List from dvc.utils import format_link @@ -206,6 +207,18 @@ class ETagMismatchError(DvcException): ) +class FileExistsLocallyError(FileExistsError, DvcException): + def __init__(self, path, hint=None): + import os.path + + self.path = path + hint = "" if hint is None else f". {hint}" + path_typ = "directory" if os.path.isdir(path) else "file" + msg = f"The {path_typ} '{path}' already exists locally{hint}" + super().__init__(msg) + self.errno = errno.EEXIST + + class FileMissingError(DvcException): def __init__(self, path, hint=None): self.path = path diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 8f20d523c..cfc38be62 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -247,11 +247,12 @@ def create_stages( fname: Optional[str] = None, transfer: bool = False, external: bool = False, + force: bool = False, **kwargs: Any, ) -> Iterator["Stage"]: for target in targets: if kwargs.get("out"): - target = resolve_output(target, kwargs["out"]) + target = resolve_output(target, kwargs["out"], force=force) path, wdir, out = resolve_paths( repo, target, always_local=transfer and not kwargs.get("out") ) @@ -263,6 +264,7 @@ def create_stages( wdir=wdir, outs=[out], external=external, + force=force, ) out_obj = stage.outs[0] diff --git a/dvc/repo/get.py b/dvc/repo/get.py index 7ef69ceac..cb397ee9e 100644 --- a/dvc/repo/get.py +++ b/dvc/repo/get.py @@ -20,14 +20,14 @@ class GetDVCFileError(DvcException): ) -def get(url, path, out=None, rev=None, jobs=None): +def get(url, path, out=None, rev=None, jobs=None, force=False): import shortuuid from dvc.dvcfile import is_valid_filename from dvc.external_repo import external_repo from dvc.fs.callbacks import Callback - out = resolve_output(path, out) + out = resolve_output(path, out, force=force) if is_valid_filename(out): raise GetDVCFileError() diff --git a/dvc/repo/get_url.py b/dvc/repo/get_url.py index 91fedf0f6..22acb5f0c 100644 --- a/dvc/repo/get_url.py +++ b/dvc/repo/get_url.py @@ -6,8 +6,8 @@ from dvc.fs import download, parse_external_url from dvc.utils import resolve_output -def get_url(url, out=None, *, config=None, jobs=None): - out = resolve_output(url, out) +def get_url(url, out=None, *, config=None, jobs=None, force=False): + out = resolve_output(url, out, force=force) out = os.path.abspath(out) (out,) = output.loads_from(None, [out], use_cache=False) diff --git a/dvc/repo/imp_url.py b/dvc/repo/imp_url.py index bf77f8c1a..96ad93dfd 100644 --- a/dvc/repo/imp_url.py +++ b/dvc/repo/imp_url.py @@ -30,10 +30,11 @@ def imp_url( # noqa: C901, PLR0913 labels=None, meta=None, jobs=None, + force=False, fs_config=None, version_aware: bool = False, ): - out = resolve_output(url, out) + out = resolve_output(url, out, force=force) path, wdir, out = resolve_paths(self, out, always_local=to_remote and not out) if to_remote and (no_exec or no_download or version_aware): diff --git a/dvc/stage/__init__.py b/dvc/stage/__init__.py index a7ff66360..d98285949 100644 --- a/dvc/stage/__init__.py +++ b/dvc/stage/__init__.py @@ -416,7 +416,7 @@ class Stage(params.StageParams): (out,) = self.outs out.transfer(source, odb=odb, jobs=kwargs.get("jobs")) if not to_remote: - out.checkout() + out.checkout(force=kwargs.get("force")) out.ignore() @rwlocked(read=["deps"], write=["outs"]) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 508b90137..5f939ff43 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -255,10 +255,11 @@ def env2bool(var, undefined=False): return bool(re.search("1|y|yes|true", var, flags=re.I)) -def resolve_output(inp, out): - import errno +def resolve_output(inp, out, force=False): from urllib.parse import urlparse + from dvc.exceptions import FileExistsLocallyError + name = os.path.basename(os.path.normpath(urlparse(inp).path)) if not out: ret = name @@ -267,8 +268,9 @@ def resolve_output(inp, out): else: ret = out - if os.path.exists(ret): - raise FileExistsError(errno.EEXIST, os.strerror(errno.EEXIST), ret) + if os.path.exists(ret) and not force: + hint = "\nTo override it, re-run with '--force'." + raise FileExistsLocallyError(ret, hint=hint) return ret diff --git a/tests/func/test_add.py b/tests/func/test_add.py index bc9e49896..5e65cd658 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -468,6 +468,28 @@ def test_should_throw_proper_exception_on_corrupted_stage_file(caplog, tmp_dir, assert expected_error in caplog.text +def test_should_throw_proper_exception_on_existing_out(caplog, tmp_dir, dvc): + tmp_dir.gen({"foo": "foo"}) + (tmp_dir / "out").write_text("old contents") + + assert main(["add", "foo", "--out", "out"]) == 1 + + assert (tmp_dir / "out").read_text() == "old contents" + expected_error_lines = [ + "Error: The file 'out' already exists locally.", + "To override it, re-run with '--force'.", + ] + assert all(line in caplog.text for line in expected_error_lines) + + +def test_add_force_overwrite_out(caplog, tmp_dir, dvc): + tmp_dir.gen({"foo": "foo"}) + (tmp_dir / "out").write_text("old contents") + + assert main(["add", "foo", "--out", "out", "--force"]) == 0 + assert (tmp_dir / "foo").read_text() == "foo" + + def test_add_filename(tmp_dir, dvc): tmp_dir.gen({"foo": "foo", "bar": "bar", "data": {"file": "file"}}) ret = main(["add", "foo", "bar", "--file", "error.dvc"]) diff --git a/tests/func/test_get.py b/tests/func/test_get.py index f26053aeb..cfc902f03 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -1,3 +1,4 @@ +import errno import logging import os @@ -5,6 +6,7 @@ import pytest from dvc.cachemgr import CacheManager from dvc.cli import main +from dvc.exceptions import FileExistsLocallyError from dvc.fs import system from dvc.repo import Repo from dvc.repo.get import GetDVCFileError @@ -21,18 +23,36 @@ def test_get_repo_file(tmp_dir, erepo_dir): assert (tmp_dir / "file_imported").read_text() == "contents" -def test_get_repo_file_replace_without_confirmation(tmp_dir, erepo_dir): +def test_get_repo_file_no_override(tmp_dir, erepo_dir): with erepo_dir.chdir(): - erepo_dir.dvc_gen("file", "contents", commit="create file") - erepo_dir.dvc_gen("file2", "something different", commit="create file2") + erepo_dir.dvc_gen("file1", "file1 contents", commit="create file") + erepo_dir.dvc_gen("file2", "file2 contents", commit="create file2") - Repo.get(os.fspath(erepo_dir), "file", "file_imported") + Repo.get(os.fspath(erepo_dir), "file1", "file_imported") # getting another file with a name that already exists in Repo. - with pytest.raises(FileExistsError): + with pytest.raises(FileExistsLocallyError) as exc_info: Repo.get(os.fspath(erepo_dir), "file2", "file_imported") + # Make sure it's a functional FileExistsError with errno + assert isinstance(exc_info.value, FileExistsError) + assert exc_info.value.errno == errno.EEXIST + assert os.path.isfile("file_imported") - assert (tmp_dir / "file_imported").read_text() == "contents" + assert (tmp_dir / "file_imported").read_text() == "file1 contents" + + +def test_get_repo_file_with_override(tmp_dir, erepo_dir): + with erepo_dir.chdir(): + erepo_dir.dvc_gen("file1", "file1 contents", commit="create file") + erepo_dir.dvc_gen("file2", "file2 contents", commit="create file2") + + Repo.get(os.fspath(erepo_dir), "file1", "file_imported") + + # override with the 2nd file + Repo.get(os.fspath(erepo_dir), "file2", "file_imported", force=True) + + assert os.path.isfile("file_imported") + assert (tmp_dir / "file_imported").read_text() == "file2 contents" def test_get_repo_dir(tmp_dir, erepo_dir): diff --git a/tests/func/test_get_url.py b/tests/func/test_get_url.py index ff7e5bf93..1733895ce 100644 --- a/tests/func/test_get_url.py +++ b/tests/func/test_get_url.py @@ -1,8 +1,9 @@ +import errno import os import pytest -from dvc.exceptions import URLMissingError +from dvc.exceptions import FileExistsLocallyError, URLMissingError from dvc.repo import Repo from dvc.testing.workspace_tests import TestGetUrl as _TestGetUrl @@ -16,6 +17,28 @@ def test_get_file(tmp_dir): assert (tmp_dir / "foo_imported").read_text() == "foo contents" +def test_get_file_conflict_and_override(tmp_dir): + tmp_dir.gen({"foo": "foo contents"}) + tmp_dir.gen({"bar": "bar contents"}) + + with pytest.raises(FileExistsLocallyError) as exc_info: + Repo.get_url("foo", "bar") + + # verify no override + assert (tmp_dir / "bar").is_file() + assert (tmp_dir / "bar").read_text() == "bar contents" + + # verify meaningful/BC exception type/errno + assert isinstance(exc_info.value, FileExistsError) + assert exc_info.value.errno == errno.EEXIST + + # now, override + Repo.get_url("foo", "bar", force=True) + + assert (tmp_dir / "bar").is_file() + assert (tmp_dir / "bar").read_text() == "foo contents" + + def test_get_dir(tmp_dir): tmp_dir.gen({"foo": {"foo": "foo contents"}}) diff --git a/tests/func/test_import_url.py b/tests/func/test_import_url.py index 1735d5e65..d9772f3d3 100644 --- a/tests/func/test_import_url.py +++ b/tests/func/test_import_url.py @@ -49,6 +49,21 @@ def test_should_remove_outs_before_import(tmp_dir, dvc, mocker, erepo_dir): assert remove_outs_call_counter.mock.call_count == 1 +def test_import_conflict_and_override(tmp_dir, dvc): + tmp_dir.gen("foo", "foo") + tmp_dir.gen("bar", "bar") + + # bar exists, fail + ret = main(["import-url", "foo", "bar"]) + assert ret != 0 + assert not os.path.exists("bar.dvc") + + # force override + ret = main(["import-url", "foo", "bar", "--force"]) + assert ret == 0 + assert os.path.exists("bar.dvc") + + def test_import_filename(tmp_dir, dvc, cloud): external_source = cloud / "file" (cloud / "file").write_text("content", encoding="utf-8") diff --git a/tests/unit/command/test_add.py b/tests/unit/command/test_add.py index e5efd31ee..6a35acb89 100644 --- a/tests/unit/command/test_add.py +++ b/tests/unit/command/test_add.py @@ -41,6 +41,7 @@ def test_add(mocker, dvc): labels=None, meta=None, jobs=None, + force=False, ) @@ -78,6 +79,7 @@ def test_add_to_remote(mocker): labels=None, meta=None, jobs=None, + force=False, ) diff --git a/tests/unit/command/test_get.py b/tests/unit/command/test_get.py index 5fa99c2b2..6a8a0a705 100644 --- a/tests/unit/command/test_get.py +++ b/tests/unit/command/test_get.py @@ -23,7 +23,9 @@ def test_get(mocker): assert cmd.run() == 0 - m.assert_called_once_with("repo_url", path="src", out="out", rev="version", jobs=4) + m.assert_called_once_with( + "repo_url", path="src", out="out", rev="version", jobs=4, force=False + ) def test_get_url(mocker, capsys): diff --git a/tests/unit/command/test_get_url.py b/tests/unit/command/test_get_url.py index 9340aafcd..a2b27495b 100644 --- a/tests/unit/command/test_get_url.py +++ b/tests/unit/command/test_get_url.py @@ -11,4 +11,4 @@ def test_get_url(mocker): assert cmd.run() == 0 - m.assert_called_once_with("src", out="out", jobs=5) + m.assert_called_once_with("src", out="out", jobs=5, force=False) diff --git a/tests/unit/command/test_imp_url.py b/tests/unit/command/test_imp_url.py index 29c958cdf..7ab09b99b 100644 --- a/tests/unit/command/test_imp_url.py +++ b/tests/unit/command/test_imp_url.py @@ -41,6 +41,7 @@ def test_import_url(mocker): labels=None, meta=None, jobs=4, + force=False, version_aware=False, ) @@ -94,10 +95,11 @@ def test_import_url_no_exec_download_flags(mocker, flag, expected): remote=None, to_remote=False, desc="description", - jobs=None, type=None, labels=None, meta=None, + jobs=None, + force=False, version_aware=False, **expected, ) @@ -136,6 +138,7 @@ def test_import_url_to_remote(mocker): labels=None, meta=None, jobs=None, + force=False, version_aware=False, )