add, import-url: always resolve the output in the local path for --to-remote (#5473)

* add, import-url: always resolve the output in the local path for --to-remote

* windows

* use the proper fixture

dvc/repo/add.py

@@ -52,6 +52,8 @@ def add(  # noqa: C901
             invalid_opt = "--no-commit option"
         elif recursive:
             invalid_opt = "--recursive option"
+        elif kwargs.get("external"):
+            invalid_opt = "--external option"
     else:
         message = "{option} can't be used without --to-remote"
         if kwargs.get("remote"):
@@ -88,7 +90,12 @@ def add(  # noqa: C901
                 )
 
             stages = _create_stages(
-                repo, sub_targets, fname, pbar=pbar, **kwargs,
+                repo,
+                sub_targets,
+                fname,
+                pbar=pbar,
+                transfer=to_remote or to_cache,
+                **kwargs,
             )
 
             try:
@@ -230,6 +237,7 @@ def _create_stages(
     external=False,
     glob=False,
     desc=None,
+    transfer=False,
     **kwargs,
 ):
     from dvc.dvcfile import Dvcfile
@@ -246,7 +254,9 @@ def _create_stages(
     ):
         if kwargs.get("out"):
             out = resolve_output(out, kwargs["out"])
-        path, wdir, out = resolve_paths(repo, out)
+        path, wdir, out = resolve_paths(
+            repo, out, always_local=transfer and not kwargs.get("out")
+        )
         stage = create_stage(
             Stage,
             repo,

dvc/repo/imp_url.py

@@ -27,7 +27,9 @@ def imp_url(
     from dvc.stage import Stage, create_stage, restore_meta
 
     out = resolve_output(url, out)
-    path, wdir, out = resolve_paths(self, out)
+    path, wdir, out = resolve_paths(
+        self, out, always_local=to_remote and not out
+    )
 
     if to_remote and no_exec:
         raise InvalidArgumentError(

dvc/utils/__init__.py

@@ -357,7 +357,7 @@ def resolve_output(inp, out):
     return ret
 
 
-def resolve_paths(repo, out):
+def resolve_paths(repo, out, always_local=False):
     from urllib.parse import urlparse
 
     from ..dvcfile import DVC_FILE_SUFFIX
@@ -393,6 +393,9 @@ def resolve_paths(repo, out):
         wdir = dirname
         out = base
 
+    if always_local:
+        out = base
+
     path = os.path.join(wdir, base + DVC_FILE_SUFFIX)
 
     return (path, wdir, out)

tests/func/test_add.py

@@ -27,7 +27,10 @@ from dvc.objects.db import ODBManager
 from dvc.output.base import OutputAlreadyTrackedError, OutputIsStageFileError
 from dvc.repo import Repo as DvcRepo
 from dvc.stage import Stage
-from dvc.stage.exceptions import StagePathNotFoundError
+from dvc.stage.exceptions import (
+    StageExternalOutputsError,
+    StagePathNotFoundError,
+)
 from dvc.system import System
 from dvc.utils import LARGE_DIR_SIZE, file_md5, relpath
 from dvc.utils.fs import path_isin
@@ -324,8 +327,6 @@ def test_add_filtered_files_in_dir(
     indirect=["workspace"],
 )
 def test_add_external_file(tmp_dir, dvc, workspace, hash_name, hash_value):
-    from dvc.stage.exceptions import StageExternalOutputsError
-
     workspace.gen("file", "file")
 
     with pytest.raises(StageExternalOutputsError):
@@ -1013,12 +1014,34 @@ def test_add_to_remote(tmp_dir, dvc, local_cloud, local_remote):
     assert local_remote.hash_to_path_info(hash_info.value).read_text() == "foo"
 
 
+def test_add_to_remote_absolute(tmp_dir, make_tmp_dir, dvc, local_remote):
+    tmp_abs_dir = make_tmp_dir("abs")
+    tmp_foo = tmp_abs_dir / "foo"
+    tmp_foo.write_text("foo")
+
+    dvc.add(str(tmp_foo), to_remote=True)
+    tmp_foo.unlink()
+
+    foo = tmp_dir / "foo"
+    assert foo.with_suffix(".dvc").exists()
+    assert not os.path.exists(tmp_foo)
+
+    dvc.pull("foo")
+    assert not os.path.exists(tmp_foo)
+    assert foo.read_text() == "foo"
+
+    with pytest.raises(StageExternalOutputsError):
+        tmp_bar = tmp_abs_dir / "bar"
+        dvc.add(str(tmp_foo), out=str(tmp_bar), to_remote=True)
+
+
 @pytest.mark.parametrize(
     "invalid_opt, kwargs",
     [
         ("multiple targets", {"targets": ["foo", "bar", "baz"]}),
         ("--no-commit", {"targets": ["foo"], "no_commit": True}),
         ("--recursive", {"targets": ["foo"], "recursive": True},),
+        ("--external", {"targets": ["foo"], "external": True}),
     ],
 )
 def test_add_to_remote_invalid_combinations(dvc, invalid_opt, kwargs):

tests/func/test_import_url.py

@@ -340,6 +340,21 @@ def test_import_url_to_remote_directory(tmp_dir, dvc, workspace, local_remote):
         )
 
 
+def test_import_url_to_remote_absolute(
+    tmp_dir, make_tmp_dir, dvc, local_remote
+):
+    tmp_abs_dir = make_tmp_dir("abs")
+    tmp_foo = tmp_abs_dir / "foo"
+    tmp_foo.write_text("foo")
+
+    stage = dvc.imp_url(str(tmp_foo), to_remote=True)
+
+    foo = tmp_dir / "foo"
+    assert stage.deps[0].fspath == str(tmp_foo)
+    assert stage.outs[0].fspath == os.fspath(foo)
+    assert foo.with_suffix(".dvc").exists()
+
+
 def test_import_url_to_remote_invalid_combinations(dvc):
     with pytest.raises(InvalidArgumentError, match="--no-exec"):
         dvc.imp_url("s3://bucket/foo", no_exec=True, to_remote=True)