diff --git a/dvc/fs/base.py b/dvc/fs/base.py index 9ffbe69a4..891207bdb 100644 --- a/dvc/fs/base.py +++ b/dvc/fs/base.py @@ -283,6 +283,9 @@ class BaseFileSystem: self, from_info, to_info, name, no_progress_bar, jobs, **kwargs, ): from_infos = list(self.walk_files(from_info, **kwargs)) + if not from_infos: + makedirs(to_info, exist_ok=True) + return None to_infos = ( to_info / info.relative_to(from_info) for info in from_infos ) diff --git a/dvc/fs/fsspec_wrapper.py b/dvc/fs/fsspec_wrapper.py index b4c730ce7..1550d69ec 100644 --- a/dvc/fs/fsspec_wrapper.py +++ b/dvc/fs/fsspec_wrapper.py @@ -1,5 +1,6 @@ import os import shutil +from functools import lru_cache from dvc.progress import Tqdm @@ -11,6 +12,7 @@ class FSSpecWrapper(BaseFileSystem): def fs(self): raise NotImplementedError + @lru_cache(512) def _with_bucket(self, path): if isinstance(path, self.PATH_CLS): return f"{path.bucket}/{path.path}" @@ -37,10 +39,32 @@ class FSSpecWrapper(BaseFileSystem): return entry def isdir(self, path_info): - return self.fs.isdir(self._with_bucket(path_info)) + if not self.exists(path_info): + return False + + # Directory in object storages are interpreted differently + # among different fsspec providers, so this logic is a temporary + # measure for us to adapt as of now. It checks whether it is a + # directory (as in a prefix with contents) or whether it is an empty + # file where it's name ends with a forward slash + entry = self.info(path_info) + return entry["type"] == "directory" or ( + entry["size"] == 0 + and entry["type"] == "file" + and entry["name"].endswith("/") + ) def isfile(self, path_info): - return self.fs.isfile(self._with_bucket(path_info)) + if not self.exists(path_info): + return False + + return not self.isdir(path_info) + + def is_empty(self, path_info): + entry = self.info(path_info) + if entry["type"] == "directory": + return not self.fs.ls(self._with_bucket(path_info)) + return entry["size"] == 0 def open( self, path_info, mode="r", **kwargs @@ -56,6 +80,9 @@ class FSSpecWrapper(BaseFileSystem): def ls( self, path_info, detail=False, recursive=False ): # pylint: disable=arguments-differ + if self.isdir(path_info) and self.is_empty(path_info): + return None + path = self._with_bucket(path_info) if recursive: for root, _, files in self.fs.walk(path, detail=detail): diff --git a/tests/func/test_import_url.py b/tests/func/test_import_url.py index 738b05521..28d03ab23 100644 --- a/tests/func/test_import_url.py +++ b/tests/func/test_import_url.py @@ -353,3 +353,42 @@ def test_import_url_to_remote_absolute( def test_import_url_to_remote_invalid_combinations(dvc): with pytest.raises(InvalidArgumentError, match="--no-exec"): dvc.imp_url("s3://bucket/foo", no_exec=True, to_remote=True) + + +empty_xfail = pytest.mark.xfail( + reason="https://github.com/iterative/dvc/issues/5521" +) + + +@pytest.mark.parametrize( + "workspace", + [ + pytest.lazy_fixture("s3"), + pytest.lazy_fixture("hdfs"), + pytest.param(pytest.lazy_fixture("gs"), marks=empty_xfail), + pytest.param(pytest.lazy_fixture("azure"), marks=empty_xfail), + pytest.param( + pytest.lazy_fixture("ssh"), + marks=pytest.mark.skipif( + os.name == "nt", reason="disabled on windows" + ), + ), + ], + indirect=True, +) +def test_import_url_empty_directory(tmp_dir, dvc, workspace): + # prefix based storage services (e.g s3) doesn't have the real concept + # of directories. So instead we create an empty file that ends with a + # trailing slash in order to actually support this operation + if workspace.IS_OBJECT_STORAGE: + contents = "" + else: + contents = {} + + workspace.gen({"empty_dir/": contents}) + + dvc.imp_url("remote://workspace/empty_dir/") + + empty_dir = tmp_dir / "empty_dir" + assert empty_dir.is_dir() + assert tuple(empty_dir.iterdir()) == () diff --git a/tests/remotes/azure.py b/tests/remotes/azure.py index 14306a472..c288302a5 100644 --- a/tests/remotes/azure.py +++ b/tests/remotes/azure.py @@ -22,6 +22,7 @@ TEST_AZURE_CONNECTION_STRING = ( class Azure(Base, CloudURLInfo): + IS_OBJECT_STORAGE = True CONNECTION_STRING = None @cached_property diff --git a/tests/remotes/base.py b/tests/remotes/base.py index 704503f3f..ea5fb79bb 100644 --- a/tests/remotes/base.py +++ b/tests/remotes/base.py @@ -7,6 +7,9 @@ from dvc.path_info import URLInfo class Base(URLInfo): + + IS_OBJECT_STORAGE = False + def is_file(self): raise NotImplementedError diff --git a/tests/remotes/gs.py b/tests/remotes/gs.py index 0099ccc84..0418fc94e 100644 --- a/tests/remotes/gs.py +++ b/tests/remotes/gs.py @@ -23,6 +23,9 @@ os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = TEST_GCP_CREDS_FILE class GCP(Base, CloudURLInfo): + + IS_OBJECT_STORAGE = True + @staticmethod def should_test(): do_test = env2bool("DVC_TEST_GCP", undefined=None) diff --git a/tests/remotes/oss.py b/tests/remotes/oss.py index 43fbb4a77..e84f3948c 100644 --- a/tests/remotes/oss.py +++ b/tests/remotes/oss.py @@ -16,6 +16,9 @@ EMULATOR_OSS_ACCESS_KEY_SECRET = "AccessKeySecret" class OSS(Base, CloudURLInfo): + + IS_OBJECT_STORAGE = True + @staticmethod def get_url(): return f"oss://{TEST_OSS_REPO_BUCKET}/{uuid.uuid4()}" diff --git a/tests/remotes/s3.py b/tests/remotes/s3.py index 22f8c9a3d..b0517c221 100644 --- a/tests/remotes/s3.py +++ b/tests/remotes/s3.py @@ -15,6 +15,9 @@ TEST_AWS_REPO_BUCKET = os.environ.get("DVC_TEST_AWS_REPO_BUCKET", "dvc-temp") class S3(Base, CloudURLInfo): + + IS_OBJECT_STORAGE = True + @staticmethod def should_test(): do_test = env2bool("DVC_TEST_AWS", undefined=None)