Merge pull request #2852 from pared/2848_get_no_dvc_master

external repo: checkout revision before initializing dvc repo

Author: efiop

dvc/external_repo.py

@@ -61,11 +61,14 @@ def _external_repo(url=None, rev=None, cache_dir=None):
         REPO_CACHE[url, None, None] = clean_clone_path
 
     # Adjust new clone/copy to fit rev and cache_dir
+
+    # Checkout needs to be done first because current branch might not be
+    # DVC repository
+    if rev is not None:
+        _git_checkout(new_path, rev)
+
     repo = Repo(new_path)
     try:
-        if rev is not None:
-            repo.scm.checkout(rev)
-
         if cache_dir is not None:
             cache_config = CacheConfig(repo.config)
             cache_config.set_dir(cache_dir, level=Config.LEVEL_LOCAL)
@@ -77,6 +80,16 @@ def _external_repo(url=None, rev=None, cache_dir=None):
     return new_path
 
 
+def _git_checkout(repo_path, revision):
+    from dvc.scm import Git
+
+    git = Git(repo_path)
+    try:
+        git.checkout(revision)
+    finally:
+        git.close()
+
+
 def clean_repos():
     # Outside code should not see cache while we are removing
     repo_paths = list(REPO_CACHE.values())

tests/func/test_get.py

@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import filecmp
+import logging
 import os
 
 import pytest
@@ -11,6 +12,7 @@
 from dvc.repo import Repo
 from dvc.system import System
 from dvc.utils import makedirs
+from dvc.utils.compat import fspath
 from tests.utils import trees_equal
 
 
@@ -87,3 +89,19 @@ def test_get_to_dir(dname, erepo):
 
     assert os.path.isdir(dname)
     assert filecmp.cmp(erepo.FOO, dst, shallow=False)
+
+
+def test_get_from_non_dvc_master(erepo, tmp_path, monkeypatch, caplog):
+    monkeypatch.chdir(fspath(tmp_path))
+    erepo.dvc.scm.repo.index.remove([".dvc"], r=True)
+    erepo.dvc.scm.commit("remove .dvc")
+
+    caplog.clear()
+    imported_file = "foo_imported"
+    with caplog.at_level(logging.INFO, logger="dvc"):
+        Repo.get(erepo._root_dir, erepo.FOO, out=imported_file, rev="branch")
+
+    assert caplog.text == ""
+    assert filecmp.cmp(
+        os.path.join(erepo._root_dir, erepo.FOO), imported_file, shallow=False
+    )