external: refactor external_repo() and its users

dvc/api.py

@@ -10,7 +10,7 @@
 from voluptuous import Schema, Required, Invalid
 
 from dvc.repo import Repo
-from dvc.exceptions import DvcException, NotDvcRepoError
+from dvc.exceptions import DvcException
 from dvc.external_repo import external_repo
 
 
@@ -60,14 +60,11 @@ def get_url(path, repo=None, rev=None, remote=None):
     `repo`.
     NOTE: There is no guarantee that the file actually exists in that location.
     """
-    try:
-        with _make_repo(repo, rev=rev) as _repo:
-            abspath = os.path.join(_repo.root_dir, path)
-            out, = _repo.find_outs_by_path(abspath)
-            remote_obj = _repo.cloud.get_remote(remote)
-            return str(remote_obj.checksum_to_path_info(out.checksum))
-    except NotDvcRepoError:
-        raise UrlNotDvcRepoError(repo)
+    with _make_repo(repo, rev=rev) as _repo:
+        _require_dvc(_repo)
+        out = _repo.find_out_by_relpath(path)
+        remote_obj = _repo.cloud.get_remote(remote)
+        return str(remote_obj.checksum_to_path_info(out.checksum))
 
 
 def open(path, repo=None, rev=None, remote=None, mode="r", encoding=None):
@@ -96,9 +93,8 @@ def __getattr__(self, name):
 
 def _open(path, repo=None, rev=None, remote=None, mode="r", encoding=None):
     with _make_repo(repo, rev=rev) as _repo:
-        abspath = os.path.join(_repo.root_dir, path)
-        with _repo.open(
-            abspath, remote=remote, mode=mode, encoding=encoding
+        with _repo.open_by_relpath(
+            path, remote=remote, mode=mode, encoding=encoding
         ) as fd:
             yield fd
 
@@ -113,10 +109,7 @@ def read(path, repo=None, rev=None, remote=None, mode="r", encoding=None):
 
 @contextmanager
 def _make_repo(repo_url, rev=None):
-    if not repo_url or os.path.exists(repo_url):
-        assert (
-            rev is None
-        ), "Git revisions are not supported for local DVC projects."
+    if rev is None and (not repo_url or os.path.exists(repo_url)):
         yield Repo(repo_url)
     else:
         with external_repo(url=repo_url, rev=rev) as repo:
@@ -149,6 +142,7 @@ def prepare_summon(name, repo=None, rev=None, summon_file="dvcsummon.yaml"):
     named object specification and resolved paths to deps.
     """
     with _make_repo(repo, rev=rev) as _repo:
+        _require_dvc(_repo)
         try:
             path = os.path.join(_repo.root_dir, summon_file)
             obj = _get_object_spec(name, path)
@@ -242,3 +236,8 @@ def _import_string(import_name):
     else:
         return importlib.import_module(import_name)
     return getattr(importlib.import_module(module), obj)
+
+
+def _require_dvc(repo):
+    if not isinstance(repo, Repo):
+        raise UrlNotDvcRepoError(repo.url)

dvc/dependency/repo.py

@@ -1,20 +1,8 @@
-import copy
 import os
-from contextlib import contextmanager
-
-from funcy import merge
 
 from .local import DependencyLOCAL
-from dvc.external_repo import cached_clone
-from dvc.external_repo import external_repo
-from dvc.exceptions import NotDvcRepoError
 from dvc.exceptions import OutputNotFoundError
-from dvc.exceptions import NoOutputInExternalRepoError
-from dvc.exceptions import PathMissingError
-from dvc.config import NoRemoteError
-from dvc.utils.fs import fs_copy
 from dvc.path_info import PathInfo
-from dvc.scm import SCM
 
 
 class DependencyREPO(DependencyLOCAL):
@@ -44,34 +32,25 @@ def repo_pair(self):
     def __str__(self):
         return "{} ({})".format(self.def_path, self.def_repo[self.PARAM_URL])
 
-    @contextmanager
-    def _make_repo(self, **overrides):
-        with external_repo(**merge(self.def_repo, overrides)) as repo:
-            yield repo
+    def _make_repo(self, *, locked=True):
+        from dvc.external_repo import external_repo
 
-    def _get_checksum(self, updated=False):
-        rev_lock = None
-        if not updated:
-            rev_lock = self.def_repo.get(self.PARAM_REV_LOCK)
+        d = self.def_repo
+        rev = (d.get("rev_lock") if locked else None) or d.get("rev")
+        return external_repo(d["url"], rev=rev)
 
-        try:
-            with self._make_repo(rev_lock=rev_lock) as repo:
+    def _get_checksum(self, locked=True):
+        with self._make_repo(locked=locked) as repo:
+            try:
                 return repo.find_out_by_relpath(self.def_path).info["md5"]
-        except (NotDvcRepoError, NoOutputInExternalRepoError):
-            # Fall through and clone
-            pass
-
-        repo_path = cached_clone(
-            self.def_repo[self.PARAM_URL],
-            rev=rev_lock or self.def_repo.get(self.PARAM_REV),
-        )
-        path = PathInfo(os.path.join(repo_path, self.def_path))
-
-        return self.repo.cache.local.get_checksum(path)
+            except OutputNotFoundError:
+                path = PathInfo(os.path.join(repo.root_dir, self.def_path))
+                # We are polluting our repo cache with some dir listing here
+                return self.repo.cache.local.get_checksum(path)
 
     def status(self):
-        current_checksum = self._get_checksum(updated=False)
-        updated_checksum = self._get_checksum(updated=True)
+        current_checksum = self._get_checksum(locked=True)
+        updated_checksum = self._get_checksum(locked=False)
 
         if current_checksum != updated_checksum:
             return {str(self): "update available"}
@@ -84,71 +63,16 @@ def save(self):
     def dumpd(self):
         return {self.PARAM_PATH: self.def_path, self.PARAM_REPO: self.def_repo}
 
-    def fetch(self):
-        with self._make_repo(
-            cache_dir=self.repo.cache.local.cache_dir
-        ) as repo:
-            self.def_repo[self.PARAM_REV_LOCK] = repo.scm.get_rev()
+    def download(self, to):
+        with self._make_repo() as repo:
+            if self.def_repo.get(self.PARAM_REV_LOCK) is None:
+                self.def_repo[self.PARAM_REV_LOCK] = repo.scm.get_rev()
 
-            out = repo.find_out_by_relpath(self.def_path)
-            with repo.state:
-                try:
-                    repo.cloud.pull(out.get_used_cache())
-                except NoRemoteError:
-                    # It would not be good idea to raise exception if the
-                    # file is already present in the cache
-                    if not self.repo.cache.local.changed_cache(out.checksum):
-                        return out
-                    raise
-
-        return out
-
-    @staticmethod
-    def _is_git_file(repo_dir, path):
-        from dvc.repo import Repo
-
-        if os.path.isabs(path):
-            return False
-
-        try:
-            repo = Repo(repo_dir)
-        except NotDvcRepoError:
-            return True
-
-        try:
-            output = repo.find_out_by_relpath(path)
-            return not output.use_cache
-        except OutputNotFoundError:
-            return True
-        finally:
-            repo.close()
-
-    def _copy_if_git_file(self, to_path):
-        src_path = self.def_path
-        repo_dir = cached_clone(**self.def_repo)
-
-        if not self._is_git_file(repo_dir, src_path):
-            return False
-
-        src_full_path = os.path.join(repo_dir, src_path)
-        dst_full_path = os.path.abspath(to_path)
-        fs_copy(src_full_path, dst_full_path)
-        self.def_repo[self.PARAM_REV_LOCK] = SCM(repo_dir).get_rev()
-        return True
+            if hasattr(repo, "cache"):
+                repo.cache.local.cache_dir = self.repo.cache.local.cache_dir
 
-    def download(self, to):
-        try:
-            if self._copy_if_git_file(to.fspath):
-                return
-
-            out = self.fetch()
-            to.info = copy.copy(out.info)
-            to.checkout()
-        except (FileNotFoundError):
-            raise PathMissingError(
-                self.def_path, self.def_repo[self.PARAM_URL]
-            )
+            repo.pull_to(self.def_path, to.path_info)
 
     def update(self):
-        with self._make_repo(rev_lock=None) as repo:
+        with self._make_repo(locked=False) as repo:
             self.def_repo[self.PARAM_REV_LOCK] = repo.scm.get_rev()

dvc/exceptions.py

@@ -227,6 +227,7 @@ def __init__(self, etag, cached_etag):
 
 class FileMissingError(DvcException):
     def __init__(self, path):
+        self.path = path
         super().__init__(
             "Can't find '{}' neither locally nor on remote".format(path)
         )