Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imp_url: make default stage fname accompany import target #3312

Merged
merged 1 commit into from
Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion dvc/repo/add.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ def _create_stages(repo, targets, fname, pbar=None):
disable=True if len(targets) < LARGE_DIR_SIZE else None,
unit="file",
):
stage = Stage.create(repo, outs=[out], add=True, fname=fname)
stage = Stage.create(
repo, outs=[out], accompany_outs=True, fname=fname
)

if not stage:
if pbar is not None:
Expand Down
8 changes: 7 additions & 1 deletion dvc/repo/imp_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ def imp_url(self, url, out=None, fname=None, erepo=None, locked=True):
out = resolve_output(url, out)

stage = Stage.create(
self, cmd=None, deps=[url], outs=[out], fname=fname, erepo=erepo
self,
cmd=None,
deps=[url],
outs=[out],
fname=fname,
erepo=erepo,
accompany_outs=True,
)

if stage is None:
Expand Down
7 changes: 3 additions & 4 deletions dvc/stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,12 +502,11 @@ def is_cached(self):
return True

@staticmethod
def create(repo, **kwargs):
def create(repo, accompany_outs=False, **kwargs):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was the one to introduce the kwargs instead of list of arguments, but it seems to me that I overdone that, arguments required in this method should be present in arguments list and if necessary, passed down. kwargs occludes what is necessary for this method.


wdir = kwargs.get("wdir", None)
cwd = kwargs.get("cwd", None)
fname = kwargs.get("fname", None)
add = kwargs.get("add", False)

# Backward compatibility for `cwd` option
if wdir is None and cwd is not None:
Expand Down Expand Up @@ -539,12 +538,12 @@ def create(repo, **kwargs):
stage._check_duplicated_arguments()

if not fname:
fname = Stage._stage_fname(stage.outs, add)
fname = Stage._stage_fname(stage.outs, accompany_outs)
stage._check_dvc_filename(fname)

# Autodetecting wdir for add, we need to create outs first to do that,
# so we start with wdir = . and remap out paths later.
if add and kwargs.get("wdir") is None and cwd is None:
if accompany_outs and kwargs.get("wdir") is None and cwd is None:
wdir = os.path.dirname(fname)

for out in chain(stage.outs, stage.deps):
Expand Down
12 changes: 12 additions & 0 deletions tests/func/test_import_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,15 @@ def test_import_url_to_dir(dname, tmp_dir, dvc):
assert stage.outs[0].path_info == dst
assert os.path.isdir(dname)
assert dst.read_text() == "file content"


def test_import_stage_accompanies_target(tmp_dir, dvc, erepo_dir):
with erepo_dir.chdir():
erepo_dir.dvc_gen("file1", "file1 content", commit="commit file")

tmp_dir.gen({"dir": {}})
erepo = {"url": fspath(erepo_dir)}
dvc.imp_url("file1", out=os.path.join("dir", "imported_file"), erepo=erepo)

assert (tmp_dir / "dir" / "imported_file").exists()
assert (tmp_dir / "dir" / "imported_file.dvc").exists()