From f0c43b4e023a1cb5785864bfa5eb553e196371c4 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Fri, 13 Sep 2019 00:04:35 +0300 Subject: [PATCH] path_info: don't forget to override as_posix method Unlike original implementation [1] that uses `str()` we actually need to use `fspath`, because we've overriden `__str__` method to return relative paths, which will break original `as_posix`. [1] https://github.com/python/cpython/blob/v3.7.0/Lib/pathlib.py#L692 Fixes #2483 Signed-off-by: Ruslan Kuprieiev --- dvc/path_info.py | 13 +++++++++---- tests/unit/test_path_info.py | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/dvc/path_info.py b/dvc/path_info.py index c7d0051d8b..3db44a4393 100644 --- a/dvc/path_info.py +++ b/dvc/path_info.py @@ -36,6 +36,15 @@ def __new__(cls, *args): def from_posix(cls, s): return cls(PosixPathInfo(s)) + def as_posix(self): + f = self._flavour + # Unlike original implementation [1] that uses `str()` we actually need + # to use `fspath`, because we've overriden `__str__` method to return + # relative paths, which will break original `as_posix`. + # + # [1] https://github.com/python/cpython/blob/v3.7.0/Lib/pathlib.py#L692 + return self.fspath.replace(f.sep, "/") + def __str__(self): path = self.__fspath__() return relpath(path) @@ -94,10 +103,6 @@ def __fspath__(self): # noqa: F811 def with_name(self, name): return pathlib.PurePath.with_name(self, name.encode(fs_encoding)) - def as_posix(self): - f = self._flavour - return str(self).replace(f.sep, "/") - class WindowsPathInfo(PathInfo, pathlib.PureWindowsPath): pass diff --git a/tests/unit/test_path_info.py b/tests/unit/test_path_info.py index 9ec73389b4..a4d48ee0be 100644 --- a/tests/unit/test_path_info.py +++ b/tests/unit/test_path_info.py @@ -1,7 +1,11 @@ import pytest import copy -from dvc.path_info import URLInfo, CloudURLInfo +from dvc.utils.compat import pathlib +from dvc.path_info import PathInfo, URLInfo, CloudURLInfo + + +TEST_DEPTH = len(pathlib.Path(__file__).parents) + 1 @pytest.mark.parametrize("cls", [URLInfo, CloudURLInfo]) @@ -42,3 +46,32 @@ def test_url_info_deepcopy(cls): u1 = cls("ssh://user@test.com:/test1/test2/test3") u2 = copy.deepcopy(u1) assert u1 == u2 + + +@pytest.mark.parametrize( + "path, as_posix, osname", + [ + ("/some/abs/path", "/some/abs/path", "posix"), + ("some/rel/path", "some/rel/path", "posix"), + ("../some/rel/path", "../some/rel/path", "posix"), + ("windows\\relpath", "windows/relpath", "nt"), + ("..\\windows\\rel\\path", "../windows/rel/path", "nt"), + # These ones are to test that no matter how layered this relpath is, + # we don't accidentally round it over. E.g. how + # + # import os + # os.chdir("/") + # os.path.relpath("../../path") + # + # results in "path". + ("\\".join([".."] * TEST_DEPTH), "/".join([".."] * TEST_DEPTH), "nt"), + ( + "/".join([".."] * TEST_DEPTH), + "/".join([".."] * TEST_DEPTH), + "posix", + ), + ], +) +def test_path_info_as_posix(mocker, path, as_posix, osname): + mocker.patch("os.name", osname) + assert PathInfo(path).as_posix() == as_posix