dvc get can can delete the current directory

[Persedes]

First off, I saw it coming, had my stuff backed up etc, so no harm done.
I was trying to test out dvc get to pull models from a different project and the cache must not have been present in the remote:

models/mlr/production is defined as the output of a dvc file.

$ dvc get <internal_repo> models/mlr/production/ --rev <name of branch>
WARNING: Some of the cache files do not exist neither locally nor on remote. Missing cache files:    
name: models/mlr/production, md5: ebbb5c351028a6e5485dfdec639fbb57.dir
Missing cache for directory '../../../tmp/tmp5jtl3fccdvc-erepo/models/mlr/production'. Cache for files inside will be lost. Would you like to continue? Use '-f' to force. [y/n] y            
WARNING: Some of the cache files do not exist neither locally nor on remote. Missing cache files:   
name: models/mlr/production, md5: ebbb5c351028a6e5485dfdec639fbb57.dir
WARNING: Cache 'ebbb5c351028a6e5485dfdec639fbb57.dir' not found. File '.' won't be created.                                                                                                   
file '.' is going to be removed. Are you sure you want to proceed? [y/n] y                                                                                                                    
ERROR: unexpected error - [Errno 2] No such file or directory

Once I saw the File '.' won't be created. file '.' is going to be removed. I knew what was going to happen, but was surprised that dvc lets you do that. (It wiped out my whole git repo ;))

DVC version 0.80.0,
Platform WSL
method of installation: pip

[efiop]

Hi @Persedes !

Wow, that is awful. Investigating right now. Thank you for reporting it! 🙏

[efiop]

Indeed, the issue is caused by dvc being confused by the trailing / in the models/mlr/production/. A workaround for now is to not include the trailing slash. Will send a fix soon...

[pared]

Reproduction script:

#!/bin/bash

rm -rf erepo repo git_repo
mkdir erepo repo git_repo

maindir=$(pwd)
set -ex
pushd git_repo
git init --bare --quiet
popd

pushd erepo
git init --quiet
git remote add origin $maindir/git_repo
dvc init -q

mkdir models
echo model >> models/1

dvc add models
git add -A
git commit -am "add stuff"

git push origin master
rm models/1
rm -rf .dvc/cache

popd

pushd repo
dvc get $maindir/erepo models/

Expected behavior:

• do not remove whole repo for starters
• we should probably rethink the output in this case, seems like the problem related to some recent UI issues dvc import: misleading warnings and outputs #2839, import: importing directories output is ugly, confusing, not informative #2691

[Persedes]

I have found another oddity, which might be related to the bug above. dvc get keeps nesting outputs.

The first get pulls a folder with two tsv files.
The second one creates a prepared folder inside that one. I would have expected dvc get to either do nothing or overwrite the existing tsv files when pulling from a different branch/ commit.

# start in an empty folder
$ ls

# pull output from a folder and define the output path
$ dvc get https://github.com/iterative/example-get-started data/prepared -o dvc_files/
$ ls dvc_files/
test.tsv  train.tsv

# running it again creates a prepared folder inside of the defined output_path
$ dvc get https://github.com/iterative/example-get-started data/prepared -o dvc_files/
# prepared gets nested inside of dvc_files
$ ls dvc_files/
prepared  test.tsv  train.tsv

@efiop , are you working on this one?

#3105 (comment)

Nesting only happens once, this is due to resolve_output. Can't tell what was the original intention behind those lines:

dvc/dvc/utils/__init__.py

Lines 336 to 344 in fe635a5

	def resolve_output(inp, out):
	from urllib.parse import urlparse
	name = os.path.basename(urlparse(inp).path)
	if not out:
	return name
	if os.path.isdir(out):
	return os.path.join(out, name)
	return out

@efiop ?

@pared , I was trying to write a test based on your reproduction script. But didn't understand why you were git pushing. Could you explain it for me?

By the way, here's what I have so far:

def test_error(tmp_dir, dvc, erepo_dir):
    with tempfile.TemporaryDirectory() as remote:
        git.Repo.init(remote)

        with erepo_dir.chdir():
            erepo_dir.scm_gen("dir/file", "text", commit="create file")
            erepo_dir.dvc_add("dir")
            origin = erepo_dir.scm.repo.create_remote("origin", remote)
            origin.push()

        os.remove(fspath(erepo_dir / "dir" / "file"))
        shutil.rmtree(fspath(erepo_dir / ".dvc" / "cache"))

        dvc.get(erepo_dir, "dir")
        assert git.Repo(remote)

It passes, so I'm not sure if I'm translating it correctly 😕

[pared]

@MrOutis, git push is unnecessary, dunno how it got there, removing it still makes the bug reproducible.

[EDIT]
@MrOutis, I modified your test a little bit:

def test_error(tmp_dir, dvc, erepo_dir):
    with erepo_dir.chdir():
        erepo_dir.gen("dir/file", "text") # just gen here, not scm_gen
        erepo_dir.dvc_add("dir", commit="add dir")

    os.remove(fspath(erepo_dir / "dir" / "file"))
    shutil.rmtree(fspath(erepo_dir / ".dvc" / "cache"))

    try:
        with mock.patch("dvc.prompt.confirm", return_value=True):
            dvc.get(fspath(erepo_dir), "dir/") # note that we get "dir/" not "dir"
    except Exception:
        pass
    finally:
        assert tmp_dir.exists()

@pared , great! Thanks for clearing it out)

@pared , I still don't get it. Tests are passing on my side, I'm loosing it on this one 😅