diff --git a/dvc/api.py b/dvc/api.py index e31d4506a..28b5c0438 100644 --- a/dvc/api.py +++ b/dvc/api.py @@ -19,15 +19,14 @@ def get_url(path, repo=None, rev=None, remote=None): directory in the remote storage. """ with Repo.open(repo, rev=rev, subrepos=True, uninitialized=True) as _repo: - fs_path = _repo.fs.path.join(_repo.root_dir, path) with reraise(FileNotFoundError, PathMissingError(path, repo)): - info = _repo.repo_fs.info(fs_path) + info = _repo.repo_fs.info(path) if not info["isdvc"]: raise OutputNotFoundError(path, repo) cloud = info["repo"].cloud - dvc_path = _repo.fs.path.relpath(fs_path, info["repo"].root_dir) + _, _, dvc_fs, dvc_path = _repo.repo_fs._get_fs_pair(path) if not os.path.isabs(path): dvc_path = dvc_path.replace("\\", "/") diff --git a/dvc/data/reference.py b/dvc/data/reference.py index a3438e390..0cd28274e 100644 --- a/dvc/data/reference.py +++ b/dvc/data/reference.py @@ -30,6 +30,7 @@ class ReferenceHashFile(HashFile): checksum: Optional[str] = None, **kwargs, ): + from dvc.fs.repo import RepoFileSystem super().__init__(fs_path, fs, hash_info, **kwargs) self.checksum = checksum or fs.checksum(fs_path) @@ -60,9 +61,6 @@ class ReferenceHashFile(HashFile): # ReferenceHashFiles should currently only be serialized in # memory and not to disk fs_path = self.fs_path - if isinstance(self.fs, RepoFileSystem): - fs_path = self.fs.path.relpath(fs_path, self.fs.root_dir) - dict_ = { self.PARAM_PATH: fs_path, self.PARAM_HASH: self.hash_info, @@ -95,11 +93,10 @@ class ReferenceHashFile(HashFile): if not fs: config = dict(config_pairs) if RepoFileSystem.PARAM_REPO_URL in config: - fs = RepoFileSystem(**config) - fs_path = fs.path.join(fs.root_dir, fs_path) + fs_cls = RepoFileSystem else: fs_cls = get_fs_cls(config, scheme=scheme) - fs = fs_cls(**config) + fs = fs_cls(**config) return ReferenceHashFile( fs_path, fs, diff --git a/dvc/dependency/repo.py b/dvc/dependency/repo.py index 6737b4f31..e4dfec357 100644 --- a/dvc/dependency/repo.py +++ b/dvc/dependency/repo.py @@ -112,11 +112,14 @@ class RepoDependency(Dependency): if locked and self.def_repo.get(self.PARAM_REV_LOCK) is None: self.def_repo[self.PARAM_REV_LOCK] = rev - path = os.path.abspath(os.path.join(repo.root_dir, self.def_path)) if not obj_only: try: for odb, obj_ids in repo.used_objs( - [path], + [ + os.path.abspath( + os.path.join(repo.root_dir, self.def_path) + ) + ], force=True, jobs=kwargs.get("jobs"), recursive=True, @@ -132,7 +135,7 @@ class RepoDependency(Dependency): try: staging, _, staged_obj = stage( local_odb, - path, + self.def_path, repo.repo_fs, local_odb.fs.PARAM_CHECKSUM, ) @@ -172,17 +175,17 @@ class RepoDependency(Dependency): continue if ( obj.fs.repo_url in checked_urls - or obj.fs.root_dir in checked_urls + or obj.fs._root_dir in checked_urls ): continue self_url = self.repo.url or self.repo.root_dir if ( obj.fs.repo_url is not None and obj.fs.repo_url == self_url - or obj.fs.root_dir == self.repo.root_dir + or obj.fs._root_dir == self.repo.root_dir ): raise CircularImportError(self, obj.fs.repo_url, self_url) - checked_urls.update([obj.fs.repo_url, obj.fs.root_dir]) + checked_urls.update([obj.fs.repo_url, obj.fs._root_dir]) def get_obj(self, filter_info=None, **kwargs): locked = kwargs.get("locked", True) diff --git a/dvc/fs/path.py b/dvc/fs/path.py index 7545fb7a8..c06dc2a59 100644 --- a/dvc/fs/path.py +++ b/dvc/fs/path.py @@ -81,7 +81,8 @@ class Path: return self.isin_or_eq(left, right) or self.isin(right, left) def relpath(self, path, start): - assert start + if not start: + return path return self.flavour.relpath(path, start=start) def relparts(self, path, base): diff --git a/dvc/fs/repo.py b/dvc/fs/repo.py index 3551b549f..4230b4ddf 100644 --- a/dvc/fs/repo.py +++ b/dvc/fs/repo.py @@ -107,19 +107,19 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method self._main_repo = repo self.hash_jobs = repo.fs.hash_jobs - self.root_dir: str = repo.root_dir + self._root_dir: str = repo.root_dir self._traverse_subrepos = subrepos self._subrepos_trie = PathStringTrie() """Keeps track of each and every path with the corresponding repo.""" - self._subrepos_trie[self.root_dir] = repo + self._subrepos_trie[self._root_dir] = repo self._dvcfss = {} """Keep a dvcfs instance of each repo.""" if hasattr(repo, "dvc_dir"): - self._dvcfss[repo.root_dir] = DvcFileSystem(repo=repo) + self._dvcfss[self._root_dir] = DvcFileSystem(repo=repo) @property def repo_url(self): @@ -131,7 +131,7 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method def config(self): return { self.PARAM_REPO_URL: self.repo_url, - self.PARAM_REPO_ROOT: self.root_dir, + self.PARAM_REPO_ROOT: self._root_dir, self.PARAM_REV: getattr(self._main_repo.fs, "rev", None), self.PARAM_CACHE_DIR: os.path.abspath( self._main_repo.odb.local.cache_dir @@ -202,13 +202,13 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method prefix, repo = self._subrepos_trie.longest_prefix(path) if not prefix: - return None + return self._main_repo parents = (parent for parent in self.path.parents(path)) dirs = [path] + list(takewhile(lambda p: p != prefix, parents)) dirs.reverse() self._update(dirs, starting_repo=repo) - return self._subrepos_trie.get(path) + return self._subrepos_trie.get(path) or self._main_repo @wrap_with(threading.Lock()) def _update(self, dirs, starting_repo): @@ -240,22 +240,31 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method """ Returns a pair of fss based on repo the path falls in, using prefix. """ - path = os.path.abspath(path) + if os.path.isabs(path): + return None, None, self._main_repo.dvcfs, path - # fallback to the top-level repo if repo was not found - # this can happen if the path is outside of the repo - repo = self._get_repo(path) or self._main_repo + parts = self.path.parts(path) + if parts and parts[0] == os.curdir: + parts = parts[1:] + + fs_path = self._main_repo.fs.path.join( + self._main_repo.root_dir, *parts + ) + repo = self._get_repo(fs_path) + fs = repo.fs + + repo_parts = fs.path.relparts(repo.root_dir, self._main_repo.root_dir) + if repo_parts[0] == os.curdir: + repo_parts = repo_parts[1:] + + dvc_parts = parts[len(repo_parts) :] + if dvc_parts and dvc_parts[0] == os.curdir: + dvc_parts = dvc_parts[1:] dvc_fs = self._dvcfss.get(repo.root_dir) + dvc_path = dvc_fs.path.join(*dvc_parts) if dvc_parts else "" - if path.startswith(repo.root_dir): - dvc_path = path[len(repo.root_dir) + 1 :] - - dvc_path = dvc_path.replace("\\", "/") - else: - dvc_path = path - - return repo.fs, path, dvc_fs, dvc_path + return fs, fs_path, dvc_fs, dvc_path def open( self, path, mode="r", encoding="utf-8", **kwargs @@ -397,14 +406,16 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method self, from_info, to_file, callback=DEFAULT_CALLBACK, **kwargs ): fs, fs_path, dvc_fs, dvc_path = self._get_fs_pair(from_info) - try: - fs.get_file( # pylint: disable=protected-access - fs_path, to_file, callback=callback, **kwargs - ) - return - except FileNotFoundError: - if not dvc_fs: - raise + + if fs: + try: + fs.get_file( # pylint: disable=protected-access + fs_path, to_file, callback=callback, **kwargs + ) + return + except FileNotFoundError: + if not dvc_fs: + raise dvc_fs.get_file( # pylint: disable=protected-access dvc_path, to_file, callback=callback, **kwargs @@ -417,25 +428,28 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method dvcignore = repo.dvcignore ignore_subrepos = kwargs.get("ignore_subrepos", True) - try: - dvc_info = dvc_fs.info(dvc_path) - except FileNotFoundError: - dvc_info = None + dvc_info = None + if dvc_fs: + try: + dvc_info = dvc_fs.info(dvc_path) + except FileNotFoundError: + pass - try: - fs_info = fs.info(fs_path) - if dvcignore.is_ignored( - fs, fs_path, ignore_subrepos=ignore_subrepos - ): - fs_info = None - except (FileNotFoundError, NotADirectoryError): - if not dvc_info: - raise - fs_info = None + fs_info = None + if fs: + try: + fs_info = fs.info(fs_path) + if dvcignore.is_ignored( + fs, fs_path, ignore_subrepos=ignore_subrepos + ): + fs_info = None + except (FileNotFoundError, NotADirectoryError): + if not dvc_info: + raise # NOTE: if some parent in fs_path turns out to be a file, it means # that that whole repofs branch doesn't exist. - if not fs_info and dvc_info: + if fs and not fs_info and dvc_info: for parent in self.path.parents(fs_path): try: if fs.info(parent)["type"] != "directory": diff --git a/dvc/output.py b/dvc/output.py index a532f9860..d4bc27432 100644 --- a/dvc/output.py +++ b/dvc/output.py @@ -386,6 +386,11 @@ class Output: os.path.realpath(self.fs_path), self.repo.root_dir ) + @property + def repo_path(self): + assert self.is_in_repo + return relpath(self.fs_path, self.repo.root_dir) + @property def use_scm_ignore(self): if not self.is_in_repo: diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index bba3ce1b9..7147b1abc 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -494,7 +494,6 @@ class Repo: from dvc.fs.repo import RepoFileSystem fs = RepoFileSystem(self, subrepos=True) - path = self.fs.path.join(self.root_dir, path) try: with fs.open( path, mode=mode, encoding=encoding, remote=remote diff --git a/dvc/repo/collect.py b/dvc/repo/collect.py index 2ba1895fa..c942bf9e9 100644 --- a/dvc/repo/collect.py +++ b/dvc/repo/collect.py @@ -34,22 +34,22 @@ def _collect_paths( from dvc.fs.repo import RepoFileSystem from dvc.utils import relpath - fs_paths = [os.path.abspath(target) for target in targets] fs = RepoFileSystem(repo) target_paths = [] - for fs_path in fs_paths: + for target in targets: + if os.path.isabs(target): + target = relpath(target, repo.root_dir) - if recursive and fs.isdir(fs_path): - target_paths.extend(repo.dvcignore.find(fs, fs_path)) + if recursive and fs.isdir(target): + target_paths.extend(repo.dvcignore.find(fs, target)) - if not fs.exists(fs_path): - rel = relpath(fs_path) + if not fs.exists(target): if rev == "workspace" or rev == "": - logger.warning("'%s' was not found in current workspace.", rel) + logger.warning("'%s' was not found in current workspace.", target) else: - logger.warning("'%s' was not found at: '%s'.", rel, rev) - target_paths.append(fs_path) + logger.warning("'%s' was not found at: '%s'.", target, rev) + target_paths.append(target) return target_paths @@ -58,12 +58,14 @@ def _filter_duplicates( ) -> Tuple[Outputs, StrPaths]: res_outs: Outputs = [] fs_res_paths = fs_paths + from dvc.utils import relpath for out in outs: - if out.fs_path in fs_paths: + rel = relpath(out.fs_path, out.stage.repo.root_dir) + if rel in fs_paths: res_outs.append(out) # MUTATING THE SAME LIST!! - fs_res_paths.remove(out.fs_path) + fs_res_paths.remove(rel) return res_outs, fs_res_paths diff --git a/dvc/repo/get.py b/dvc/repo/get.py index 0b15eb9a4..f1690a40f 100644 --- a/dvc/repo/get.py +++ b/dvc/repo/get.py @@ -49,9 +49,6 @@ def get(url, path, out=None, rev=None, jobs=None): with external_repo( url=url, rev=rev, cache_dir=tmp_dir, cache_types=cache_types ) as repo: - from_fs_path = os.path.abspath(os.path.join(repo.root_dir, path)) - repo.repo_fs.download( - from_fs_path, os.path.abspath(out), jobs=jobs - ) + repo.repo_fs.download(path, os.path.abspath(out), jobs=jobs) finally: remove(tmp_dir) diff --git a/dvc/repo/ls.py b/dvc/repo/ls.py index 083b7aedc..eba585578 100644 --- a/dvc/repo/ls.py +++ b/dvc/repo/ls.py @@ -29,9 +29,7 @@ def ls(url, path=None, rev=None, recursive=None, dvc_only=False): from . import Repo with Repo.open(url, rev=rev, subrepos=True, uninitialized=True) as repo: - fs_path = repo.root_dir - if path: - fs_path = os.path.abspath(repo.fs.path.join(fs_path, path)) + fs_path = path or "" ret = _ls(repo, fs_path, recursive, dvc_only) diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 6bb78194e..784faacf9 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -80,11 +80,12 @@ def _read_metrics(repo, metrics, rev, onerror=None): res = {} for metric in metrics: - if not fs.isfile(metric): + fs_path = repo.fs.path.relpath(metric, repo.root_dir) + if not fs.isfile(fs_path): continue res[fs.path.relpath(metric, os.getcwd())] = _read_metric( - metric, fs, rev, onerror=onerror + fs_path, fs, rev, onerror=onerror ) return res diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index 303308d63..ef684d6ce 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -251,8 +251,8 @@ def _collect_plots( recursive=recursive, ) - result = {plot.fs_path: _plot_props(plot) for plot in plots} - result.update({fs_path: {} for fs_path in fs_paths}) + result = {relpath(plot.fs_path, repo.root_dir): _plot_props(plot) for plot in plots} + result.update({relpath(fs_path, repo.root_dir): {} for fs_path in fs_paths}) return result diff --git a/dvc/repo/reproduce.py b/dvc/repo/reproduce.py index d4fa759ba..c57b3000f 100644 --- a/dvc/repo/reproduce.py +++ b/dvc/repo/reproduce.py @@ -63,7 +63,7 @@ def _get_stage_files(stage: "Stage") -> typing.Iterator[str]: if ( not dep.use_scm_ignore and dep.is_in_repo - and not stage.repo.repo_fs.isdvc(dep.fs_path) + and not stage.repo.repo_fs.isdvc(dep.fs.path.relpath(dep.fs_path, stage.repo.root_dir)) ): yield dep.fs_path for out in stage.outs: diff --git a/tests/func/experiments/test_checkpoints.py b/tests/func/experiments/test_checkpoints.py index 3535a1e72..eab79e18c 100644 --- a/tests/func/experiments/test_checkpoints.py +++ b/tests/func/experiments/test_checkpoints.py @@ -32,9 +32,9 @@ def test_new_checkpoint( if rev == "workspace": continue fs = dvc.repo_fs - with fs.open((tmp_dir / "foo").fs_path) as fobj: + with fs.open("foo") as fobj: assert fobj.read().strip() == str(checkpoint_stage.iterations) - with fs.open((tmp_dir / "metrics.yaml").fs_path) as fobj: + with fs.open("metrics.yaml") as fobj: assert fobj.read().strip() == "foo: 2" if workspace: @@ -81,9 +81,9 @@ def test_resume_checkpoint( if rev == "workspace": continue fs = dvc.repo_fs - with fs.open((tmp_dir / "foo").fs_path) as fobj: + with fs.open("foo") as fobj: assert fobj.read().strip() == str(2 * checkpoint_stage.iterations) - with fs.open((tmp_dir / "metrics.yaml").fs_path) as fobj: + with fs.open("metrics.yaml") as fobj: assert fobj.read().strip() == "foo: 2" if workspace: @@ -112,9 +112,9 @@ def test_reset_checkpoint( if rev == "workspace": continue fs = dvc.repo_fs - with fs.open((tmp_dir / "foo").fs_path) as fobj: + with fs.open("foo") as fobj: assert fobj.read().strip() == str(checkpoint_stage.iterations) - with fs.open((tmp_dir / "metrics.yaml").fs_path) as fobj: + with fs.open("metrics.yaml") as fobj: assert fobj.read().strip() == "foo: 2" if workspace: @@ -151,18 +151,18 @@ def test_resume_branch(tmp_dir, scm, dvc, checkpoint_stage, workspace): if rev == "workspace": continue fs = dvc.repo_fs - with fs.open((tmp_dir / "foo").fs_path) as fobj: + with fs.open("foo") as fobj: assert fobj.read().strip() == str(2 * checkpoint_stage.iterations) - with fs.open((tmp_dir / "metrics.yaml").fs_path) as fobj: + with fs.open("metrics.yaml") as fobj: assert fobj.read().strip() == "foo: 2" for rev in dvc.brancher([checkpoint_b]): if rev == "workspace": continue fs = dvc.repo_fs - with fs.open((tmp_dir / "foo").fs_path) as fobj: + with fs.open("foo") as fobj: assert fobj.read().strip() == str(2 * checkpoint_stage.iterations) - with fs.open((tmp_dir / "metrics.yaml").fs_path) as fobj: + with fs.open("metrics.yaml") as fobj: assert fobj.read().strip() == "foo: 100" with pytest.raises(MultipleBranchError): diff --git a/tests/func/experiments/test_experiments.py b/tests/func/experiments/test_experiments.py index c16e68806..082c5ca08 100644 --- a/tests/func/experiments/test_experiments.py +++ b/tests/func/experiments/test_experiments.py @@ -675,9 +675,9 @@ def test_modified_data_dep(tmp_dir, scm, dvc, workspace, params, target): for rev in dvc.brancher(revs=[exp]): if rev != exp: continue - with dvc.repo_fs.open((tmp_dir / "metrics.yaml").fs_path) as fobj: + with dvc.repo_fs.open("metrics.yaml") as fobj: assert fobj.read().strip() == params - with dvc.repo_fs.open((tmp_dir / "data").fs_path) as fobj: + with dvc.repo_fs.open("data") as fobj: assert fobj.read().strip() == "modified" if workspace: diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index 147faf1f4..ae51096e1 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -96,7 +96,7 @@ def test_pull_subdir_file(tmp_dir, erepo_dir): dest = tmp_dir / "file" with external_repo(os.fspath(erepo_dir)) as repo: repo.repo_fs.download( - os.path.join(repo.root_dir, "subdir", "file"), + os.path.join("subdir", "file"), os.fspath(dest), ) @@ -192,7 +192,7 @@ def test_subrepos_are_ignored(tmp_dir, erepo_dir): with external_repo(os.fspath(erepo_dir)) as repo: repo.repo_fs.download( - os.path.join(repo.root_dir, "dir"), + "dir", os.fspath(tmp_dir / "out"), ) expected_files = {"foo": "foo", "bar": "bar", ".gitignore": "/foo\n"} @@ -206,7 +206,7 @@ def test_subrepos_are_ignored(tmp_dir, erepo_dir): staging, _, obj = stage( repo.odb.local, - os.path.join(repo.root_dir, "dir"), + "dir", repo.repo_fs, "md5", dvcignore=repo.dvcignore, @@ -238,7 +238,7 @@ def test_subrepos_are_ignored_for_git_tracked_dirs(tmp_dir, erepo_dir): with external_repo(os.fspath(erepo_dir)) as repo: repo.repo_fs.download( - os.path.join(repo.root_dir, "dir"), + "dir", os.fspath(tmp_dir / "out"), ) # subrepo files should not be here diff --git a/tests/func/test_fs.py b/tests/func/test_fs.py index 85d439d2b..663e563d3 100644 --- a/tests/func/test_fs.py +++ b/tests/func/test_fs.py @@ -335,7 +335,7 @@ def test_callback_on_repo_fs(tmp_dir, dvc, scm): callback = fsspec.Callback() fs.download( - (tmp_dir / "dir").fs_path, + "dir", (tmp_dir / "dir2").fs_path, callback=callback, ) @@ -346,7 +346,7 @@ def test_callback_on_repo_fs(tmp_dir, dvc, scm): callback = fsspec.Callback() fs.download( - (tmp_dir / "dir" / "foo").fs_path, + os.path.join("dir", "foo"), (tmp_dir / "foo").fs_path, callback=callback, ) @@ -358,7 +358,7 @@ def test_callback_on_repo_fs(tmp_dir, dvc, scm): callback = fsspec.Callback() fs.download( - (tmp_dir / "dir" / "bar").fs_path, + os.path.join("dir", "bar"), (tmp_dir / "bar").fs_path, callback=callback, ) diff --git a/tests/unit/fs/test_repo.py b/tests/unit/fs/test_repo.py index b0c6d178e..ea3f39600 100644 --- a/tests/unit/fs/test_repo.py +++ b/tests/unit/fs/test_repo.py @@ -25,7 +25,7 @@ def test_open(tmp_dir, dvc): (tmp_dir / "foo").unlink() fs = RepoFileSystem(repo=dvc) - with fs.open((tmp_dir / "foo").fs_path, "r") as fobj: + with fs.open("foo", "r") as fobj: assert fobj.read() == "foo" @@ -34,7 +34,7 @@ def test_open_dirty_hash(tmp_dir, dvc): (tmp_dir / "file").write_text("something") fs = RepoFileSystem(repo=dvc) - with fs.open((tmp_dir / "file").fs_path, "r") as fobj: + with fs.open("file", "r") as fobj: assert fobj.read() == "something" @@ -43,7 +43,7 @@ def test_open_dirty_no_hash(tmp_dir, dvc): (tmp_dir / "file.dvc").write_text("outs:\n- path: file\n") fs = RepoFileSystem(repo=dvc) - with fs.open((tmp_dir / "file").fs_path, "r") as fobj: + with fs.open("file", "r") as fobj: assert fobj.read() == "file" @@ -63,7 +63,7 @@ def test_open_in_history(tmp_dir, scm, dvc): continue fs = RepoFileSystem(repo=dvc) - with fs.open((tmp_dir / "foo").fs_path, "r") as fobj: + with fs.open("foo", "r") as fobj: assert fobj.read() == "foo" @@ -130,30 +130,30 @@ def test_exists_isdir_isfile_dirty(tmp_dir, dvc): (tmp_dir / "datafile").unlink() root = tmp_dir - assert fs.exists(root / "datafile") - assert fs.exists(root / "datadir") - assert fs.exists(root / "datadir" / "foo") - assert fs.isfile(root / "datafile") - assert not fs.isfile(root / "datadir") - assert fs.isfile(root / "datadir" / "foo") - assert not fs.isdir(root / "datafile") - assert fs.isdir(root / "datadir") - assert not fs.isdir(root / "datadir" / "foo") + assert fs.exists("datafile") + assert fs.exists("datadir") + assert fs.exists(os.path.join("datadir", "foo")) + assert fs.isfile("datafile") + assert not fs.isfile("datadir") + assert fs.isfile(os.path.join("datadir", "foo")) + assert not fs.isdir("datafile") + assert fs.isdir("datadir") + assert not fs.isdir(os.path.join("datadir", "foo")) # NOTE: creating file instead of dir and dir instead of file tmp_dir.gen({"datadir": "data", "datafile": {"foo": "foo", "bar": "bar"}}) - assert fs.exists(root / "datafile") - assert fs.exists(root / "datadir") - assert not fs.exists(root / "datadir" / "foo") - assert fs.exists(root / "datafile" / "foo") - assert not fs.isfile(root / "datafile") - assert fs.isfile(root / "datadir") - assert not fs.isfile(root / "datadir" / "foo") - assert fs.isfile(root / "datafile" / "foo") - assert fs.isdir(root / "datafile") - assert not fs.isdir(root / "datadir") - assert not fs.isdir(root / "datadir" / "foo") - assert not fs.isdir(root / "datafile" / "foo") + assert fs.exists("datafile") + assert fs.exists("datadir") + assert not fs.exists(os.path.join("datadir", "foo")) + assert fs.exists(os.path.join("datafile", "foo")) + assert not fs.isfile("datafile") + assert fs.isfile("datadir") + assert not fs.isfile(os.path.join("datadir", "foo")) + assert fs.isfile(os.path.join("datafile", "foo")) + assert fs.isdir("datafile") + assert not fs.isdir("datadir") + assert not fs.isdir(os.path.join("datadir", "foo")) + assert not fs.isdir(os.path.join("datafile", "foo")) def test_isdir_mixed(tmp_dir, dvc): @@ -257,11 +257,11 @@ def test_walk_dirty_cached_dir(tmp_dir, scm, dvc): data = tmp_dir / "data" actual = [] - for root, dirs, files in fs.walk(data): + for root, dirs, files in fs.walk("data"): for entry in dirs + files: actual.append(os.path.join(root, entry)) - expected = [(data / "foo").fs_path, (data / "bar").fs_path] + expected = [os.path.join("data", "foo"), os.path.join("data", "bar")] assert set(actual) == set(expected) assert len(actual) == len(expected) @@ -365,30 +365,30 @@ def test_subrepos(tmp_dir, scm, dvc, mocker): with mock.patch.object( fs, "_get_repo", side_effect=assert_fs_belongs_to_repo(subrepo1.dvc) ): - assert fs.exists((subrepo1 / "foo").fs_path) is True - assert fs.exists((subrepo1 / "bar").fs_path) is False + assert fs.exists(os.path.join("dir", "repo", "foo")) is True + assert fs.exists(os.path.join("dir", "repo", "bar")) is False - assert fs.isfile((subrepo1 / "foo").fs_path) is True - assert fs.isfile((subrepo1 / "dir1" / "bar").fs_path) is True - assert fs.isfile((subrepo1 / "dir1").fs_path) is False + assert fs.isfile(os.path.join("dir", "repo", "foo")) is True + assert fs.isfile(os.path.join("dir", "repo", "dir1", "bar")) is True + assert fs.isfile(os.path.join("dir", "repo", "dir1")) is False - assert fs.isdir((subrepo1 / "dir1").fs_path) is True - assert fs.isdir((subrepo1 / "dir1" / "bar").fs_path) is False - assert fs.isdvc((subrepo1 / "foo").fs_path) is True + assert fs.isdir(os.path.join("dir", "repo", "dir1")) is True + assert fs.isdir(os.path.join("dir", "repo", "dir1", "bar")) is False + assert fs.isdvc(os.path.join("dir", "repo", "foo")) is True with mock.patch.object( fs, "_get_repo", side_effect=assert_fs_belongs_to_repo(subrepo2.dvc) ): - assert fs.exists((subrepo2 / "lorem").fs_path) is True - assert fs.exists((subrepo2 / "ipsum").fs_path) is False + assert fs.exists(os.path.join("dir", "repo2", "lorem")) is True + assert fs.exists(os.path.join("dir", "repo2", "ipsum")) is False - assert fs.isfile((subrepo2 / "lorem").fs_path) is True - assert fs.isfile((subrepo2 / "dir2" / "ipsum").fs_path) is True - assert fs.isfile((subrepo2 / "dir2").fs_path) is False + assert fs.isfile(os.path.join("dir", "repo2", "lorem")) is True + assert fs.isfile(os.path.join("dir", "repo2", "dir2", "ipsum")) is True + assert fs.isfile(os.path.join("dir", "repo2", "dir2")) is False - assert fs.isdir((subrepo2 / "dir2").fs_path) is True - assert fs.isdir((subrepo2 / "dir2" / "ipsum").fs_path) is False - assert fs.isdvc((subrepo2 / "lorem").fs_path) is True + assert fs.isdir(os.path.join("dir", "repo2", "dir2")) is True + assert fs.isdir(os.path.join("dir", "repo2", "dir2", "ipsum")) is False + assert fs.isdvc(os.path.join("dir", "repo2", "lorem")) is True @pytest.mark.parametrize( @@ -447,16 +447,14 @@ def test_subrepo_walk(tmp_dir, scm, dvc, dvcfiles, extra_expected): actual = [] for root, dirs, files in fs.walk( - os.path.join(fs.root_dir, "dir"), + "dir", dvcfiles=dvcfiles, ignore_subrepos=False, ): for entry in dirs + files: actual.append(os.path.join(root, entry)) - expected = [ - os.path.join(fs.root_dir, path) for path in expected + extra_expected - ] + expected += extra_expected assert set(actual) == set(expected) assert len(actual) == len(expected) @@ -478,44 +476,43 @@ def test_repo_fs_no_subrepos(tmp_dir, dvc, scm): dvc._reset() fs = RepoFileSystem(repo=dvc) expected = [ - tmp_dir / ".dvcignore", - tmp_dir / ".gitignore", - tmp_dir / "lorem", - tmp_dir / "lorem.dvc", - tmp_dir / "dir", - tmp_dir / "dir" / "repo.txt", + ".dvcignore", + ".gitignore", + "lorem", + "lorem.dvc", + "dir", + os.path.join("dir", "repo.txt"), ] actual = [] - for root, dirs, files in fs.walk(tmp_dir.fs_path, dvcfiles=True): + for root, dirs, files in fs.walk("", dvcfiles=True): for entry in dirs + files: - actual.append(os.path.normpath(os.path.join(root, entry))) + actual.append(os.path.join(root, entry)) - expected = [str(path) for path in expected] assert set(actual) == set(expected) assert len(actual) == len(expected) - assert fs.isfile(tmp_dir / "lorem") is True - assert fs.isfile(tmp_dir / "dir" / "repo" / "foo") is False - assert fs.isdir(tmp_dir / "dir" / "repo") is False - assert fs.isdir(tmp_dir / "dir") is True + assert fs.isfile("lorem") is True + assert fs.isfile(os.path.join("dir", "repo", "foo")) is False + assert fs.isdir(os.path.join("dir", "repo")) is False + assert fs.isdir("dir") is True - assert fs.isdvc(tmp_dir / "lorem") is True - assert fs.isdvc(tmp_dir / "dir" / "repo" / "dir1") is False + assert fs.isdvc("lorem") is True + assert fs.isdvc(os.path.join("dir", "repo", "dir1")) is False - assert fs.exists(tmp_dir / "dir" / "repo.txt") is True - assert fs.exists(tmp_dir / "repo" / "ipsum") is False + assert fs.exists(os.path.join("dir", "repo.txt")) is True + assert fs.exists(os.path.join("repo", "ipsum")) is False def test_get_hash_cached_file(tmp_dir, dvc, mocker): tmp_dir.dvc_gen({"foo": "foo"}) fs = RepoFileSystem(repo=dvc) expected = "acbd18db4cc2f85cedef654fccc4a4d8" - assert fs.info((tmp_dir / "foo").fs_path).get("md5") is None - _, _, obj = stage(dvc.odb.local, (tmp_dir / "foo").fs_path, fs, "md5") + assert fs.info("foo").get("md5") is None + _, _, obj = stage(dvc.odb.local, "foo", fs, "md5") assert obj.hash_info == HashInfo("md5", expected) (tmp_dir / "foo").unlink() - assert fs.info((tmp_dir / "foo").fs_path)["md5"] == expected + assert fs.info("foo")["md5"] == expected def test_get_hash_cached_dir(tmp_dir, dvc, mocker): @@ -524,15 +521,15 @@ def test_get_hash_cached_dir(tmp_dir, dvc, mocker): ) fs = RepoFileSystem(repo=dvc) expected = "8761c4e9acad696bee718615e23e22db.dir" - assert fs.info((tmp_dir / "dir").fs_path).get("md5") is None - _, _, obj = stage(dvc.odb.local, (tmp_dir / "dir").fs_path, fs, "md5") + assert fs.info("dir").get("md5") is None + _, _, obj = stage(dvc.odb.local, "dir", fs, "md5") assert obj.hash_info == HashInfo( "md5", "8761c4e9acad696bee718615e23e22db.dir" ) shutil.rmtree(tmp_dir / "dir") - assert fs.info((tmp_dir / "dir").fs_path)["md5"] == expected - _, _, obj = stage(dvc.odb.local, (tmp_dir / "dir").fs_path, fs, "md5") + assert fs.info("dir")["md5"] == expected + _, _, obj = stage(dvc.odb.local, "dir", fs, "md5") assert obj.hash_info == HashInfo( "md5", "8761c4e9acad696bee718615e23e22db.dir" ) @@ -543,18 +540,18 @@ def test_get_hash_cached_granular(tmp_dir, dvc, mocker): {"dir": {"foo": "foo", "bar": "bar", "subdir": {"data": "data"}}} ) fs = RepoFileSystem(repo=dvc) - subdir = tmp_dir / "dir" / "subdir" - assert fs.info(subdir.fs_path).get("md5") is None - _, _, obj = stage(dvc.odb.local, subdir.fs_path, fs, "md5") + subdir = os.path.join("dir", "subdir") + assert fs.info(subdir).get("md5") is None + _, _, obj = stage(dvc.odb.local, subdir, fs, "md5") assert obj.hash_info == HashInfo( "md5", "af314506f1622d107e0ed3f14ec1a3b5.dir" ) - assert fs.info((subdir / "data").fs_path).get("md5") is None - _, _, obj = stage(dvc.odb.local, (subdir / "data").fs_path, fs, "md5") + assert fs.info(os.path.join(subdir, "data")).get("md5") is None + _, _, obj = stage(dvc.odb.local, os.path.join(subdir, "data"), fs, "md5") assert obj.hash_info == HashInfo("md5", "8d777f385d3dfec8815d20f7496026dc") (tmp_dir / "dir" / "subdir" / "data").unlink() assert ( - fs.info((subdir / "data").fs_path)["md5"] + fs.info(os.path.join(subdir, "data"))["md5"] == "8d777f385d3dfec8815d20f7496026dc" ) @@ -573,7 +570,7 @@ def test_get_hash_mixed_dir(tmp_dir, scm, dvc): clean_staging() fs = RepoFileSystem(repo=dvc) - _, _, obj = stage(dvc.odb.local, (tmp_dir / "dir").fs_path, fs, "md5") + _, _, obj = stage(dvc.odb.local, "dir", fs, "md5") assert obj.hash_info == HashInfo( "md5", "e1d9e8eae5374860ae025ec84cfd85c7.dir" ) @@ -595,9 +592,9 @@ def test_get_hash_dirty_file(tmp_dir, dvc): # file is modified in workspace # get_file_hash(file) should return workspace hash, not DVC cached hash fs = RepoFileSystem(repo=dvc) - assert fs.info((tmp_dir / "file").fs_path).get("md5") is None + assert fs.info("file").get("md5") is None staging, _, obj = stage( - dvc.odb.local, (tmp_dir / "file").fs_path, fs, "md5" + dvc.odb.local, "file", fs, "md5" ) assert obj.hash_info == something_hash_info check(staging, obj) @@ -609,15 +606,15 @@ def test_get_hash_dirty_file(tmp_dir, dvc): check(staging, obj) # get_file_hash(file) should return DVC cached hash - assert fs.info((tmp_dir / "file").fs_path)["md5"] == file_hash_info.value + assert fs.info("file")["md5"] == file_hash_info.value _, hash_info = get_file_hash( - (tmp_dir / "file").fs_path, fs, "md5", state=dvc.state + "file", fs, "md5", state=dvc.state ) assert hash_info == file_hash_info # tmp_dir/file can be staged even though it is missing in workspace since # repofs will use the DVC cached hash (and refer to the local cache object) - _, _, obj = stage(dvc.odb.local, (tmp_dir / "file").fs_path, fs, "md5") + _, _, obj = stage(dvc.odb.local, "file", fs, "md5") assert obj.hash_info == file_hash_info @@ -627,7 +624,7 @@ def test_get_hash_dirty_dir(tmp_dir, dvc): clean_staging() fs = RepoFileSystem(repo=dvc) - _, meta, obj = stage(dvc.odb.local, (tmp_dir / "dir").fs_path, fs, "md5") + _, meta, obj = stage(dvc.odb.local, "dir", fs, "md5") assert obj.hash_info == HashInfo( "md5", "ba75a2162ca9c29acecb7957105a0bc2.dir" ) @@ -665,23 +662,24 @@ def test_walk_nested_subrepos(tmp_dir, dvc, scm, traverse_subrepos): repo_dir.dvc_gen(dvc_files, commit=f"dvc add in {repo_dir}") if traverse_subrepos or repo_dir == tmp_dir: - expected[str(repo_dir)] = set( + repo_dir_path = repo_dir.relative_to(tmp_dir).fs_path if repo_dir != tmp_dir else "" + expected[repo_dir_path] = set( scm_files.keys() | dvc_files.keys() | extras ) # files inside a dvc directory - expected[str(repo_dir / f"dvc-{base}")] = {f"ipsum-{base}"} + expected[os.path.join(repo_dir_path, f"dvc-{base}")] = {f"ipsum-{base}"} # files inside a git directory - expected[str(repo_dir / f"dir-{base}")] = {f"bar-{base}"} + expected[os.path.join(repo_dir_path, f"dir-{base}")] = {f"bar-{base}"} if traverse_subrepos: # update subrepos - expected[str(tmp_dir)].update(["subrepo1", "subrepo2"]) - expected[str(tmp_dir / "subrepo1")].add("subrepo3") + expected[""].update(["subrepo1", "subrepo2"]) + expected["subrepo1"].add("subrepo3") actual = {} fs = RepoFileSystem(repo=dvc) for root, dirs, files in fs.walk( - str(tmp_dir), ignore_subrepos=not traverse_subrepos + "", ignore_subrepos=not traverse_subrepos ): actual[root] = set(dirs + files) assert expected == actual diff --git a/tests/unit/fs/test_repo_info.py b/tests/unit/fs/test_repo_info.py index bc7b285d5..79af258bb 100644 --- a/tests/unit/fs/test_repo_info.py +++ b/tests/unit/fs/test_repo_info.py @@ -61,7 +61,7 @@ def test_info_not_existing(repo_fs): def test_info_git_tracked_file(repo_fs, path): info = repo_fs.info(path) - assert info["repo"].root_dir == repo_fs.root_dir + assert info["repo"].root_dir == repo_fs._root_dir assert not info["isdvc"] assert info["type"] == "file" assert not info["isexec"] @@ -80,7 +80,7 @@ def test_info_git_tracked_file(repo_fs, path): def test_info_dvc_tracked_file(repo_fs, path): info = repo_fs.info(path) - assert info["repo"].root_dir == repo_fs.root_dir + assert info["repo"].root_dir == repo_fs._root_dir assert info["isdvc"] assert info["type"] == "file" assert not info["isexec"] @@ -90,7 +90,7 @@ def test_info_dvc_tracked_file(repo_fs, path): def test_info_git_only_dirs(repo_fs, path): info = repo_fs.info(path) - assert info["repo"].root_dir == repo_fs.root_dir + assert info["repo"].root_dir == repo_fs._root_dir assert not info["isdvc"] assert info["type"] == "directory" assert not info["isexec"] @@ -98,9 +98,9 @@ def test_info_git_only_dirs(repo_fs, path): @pytest.mark.parametrize("path", [".", "models"]) def test_info_git_dvc_mixed_dirs(repo_fs, path): - info = repo_fs.info(os.path.join(repo_fs.root_dir, path)) + info = repo_fs.info(path) - assert info["repo"].root_dir == repo_fs.root_dir + assert info["repo"].root_dir == repo_fs._root_dir assert not info["isdvc"] assert info["type"] == "directory" assert not info["isexec"] @@ -115,9 +115,9 @@ def test_info_git_dvc_mixed_dirs(repo_fs, path): ], ) def test_info_dvc_only_dirs(repo_fs, path): - info = repo_fs.info(os.path.join(repo_fs.root_dir, path)) + info = repo_fs.info(path) - assert info["repo"].root_dir == repo_fs.root_dir + assert info["repo"].root_dir == repo_fs._root_dir assert info["isdvc"] assert info["type"] == "directory" assert not info["isexec"] @@ -135,7 +135,7 @@ def test_info_on_subrepos(make_tmp_dir, tmp_dir, dvc, scm, repo_fs): os.path.join("subrepo", "foo"), os.path.join("subrepo", "foobar"), ]: - info = repo_fs.info(tmp_dir / path) + info = repo_fs.info(path) assert info["repo"].root_dir == str( subrepo ), f"repo root didn't match for {path}" diff --git a/tests/unit/test_external_repo.py b/tests/unit/test_external_repo.py index 10fc790b1..c4ff4d464 100644 --- a/tests/unit/test_external_repo.py +++ b/tests/unit/test_external_repo.py @@ -28,7 +28,7 @@ def test_hook_is_called(tmp_dir, erepo_dir, mocker): with external_repo(str(erepo_dir)) as repo: spy = mocker.spy(repo.repo_fs, "repo_factory") - list(repo.repo_fs.walk(repo.root_dir, ignore_subrepos=False)) # drain + list(repo.repo_fs.walk("", ignore_subrepos=False)) # drain assert spy.call_count == len(subrepos) paths = [os.path.join(repo.root_dir, path) for path in subrepo_paths] @@ -65,7 +65,7 @@ def test_subrepo_is_constructed_properly( ) as repo: spy = mocker.spy(repo.repo_fs, "repo_factory") - list(repo.repo_fs.walk(repo.root_dir, ignore_subrepos=False)) # drain + list(repo.repo_fs.walk("", ignore_subrepos=False)) # drain assert spy.call_count == 1 subrepo = spy.spy_return