Fix: Prettifying unexpected error on FileExists for get/get-url/imp-url/add and introducing --force (#9365)

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="<number>",
     )
+    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(

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="<number>",
     )
+    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)

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="<number>",
     )
+    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)

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="<number>",
     )
+    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",

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

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]

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()

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)
 

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):

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"])

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
 

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"])

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):

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"}})
 

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")

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,
     )
 
 

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):

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)

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,
     )