From 7d14acb5a7fc4b6d4378ae83367f26cf07fe4213 Mon Sep 17 00:00:00 2001 From: skshetry <18718008+skshetry@users.noreply.github.com> Date: Fri, 10 Jan 2025 18:39:38 +0545 Subject: [PATCH] 3.13 support for Windows (#10667) python/cpython#113829 changed `os.path.isabs` on Windows to not consider a path starting a single backslash to be an absolute path. We do a lot of naive `posixpath` to `ntpath` conversions, and it broke DVC in multiple places. DVC is likely broken in many other places, and these bugs are difficult to identify. In this commit, I have tried to fix in the places where the tests failed. And mostly by trying to imitate pre-3.13 behaviour. Some of the changes may not be correct but keeps the old behaviour in place. Also re-enabled the CI for all Python versions supported for Windows. --- .github/workflows/tests.yaml | 36 ++++++++++++++++++++++++++++++++++++ dvc/config.py | 10 ++++++++++ dvc/fs/dvc.py | 5 +++-- tests/unit/test_ignore.py | 3 ++- 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 2ae28c7506..0b1fe733a1 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -69,6 +69,30 @@ jobs: - os: windows-latest pyv: "3.9" pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 4" + - os: windows-latest + pyv: "3.10" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 1" + - os: windows-latest + pyv: "3.10" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 2" + - os: windows-latest + pyv: "3.10" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 3" + - os: windows-latest + pyv: "3.10" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 4" + - os: windows-latest + pyv: "3.11" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 1" + - os: windows-latest + pyv: "3.11" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 2" + - os: windows-latest + pyv: "3.11" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 3" + - os: windows-latest + pyv: "3.11" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 4" - os: windows-latest pyv: "3.12" pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 1" @@ -81,6 +105,18 @@ jobs: - os: windows-latest pyv: "3.12" pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 4" + - os: windows-latest + pyv: "3.13" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 1" + - os: windows-latest + pyv: "3.13" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 2" + - os: windows-latest + pyv: "3.13" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 3" + - os: windows-latest + pyv: "3.13" + pytestargs: "--splitting-algorithm=least_duration --splits 4 --group 4" steps: - uses: actions/checkout@v4 with: diff --git a/dvc/config.py b/dvc/config.py index 89d9fb4891..ff227f9f5c 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -1,6 +1,8 @@ """DVC config objects.""" +import ntpath import os +import posixpath import re from contextlib import contextmanager from functools import partial @@ -268,12 +270,17 @@ def _resolve(conf_dir, path): if re.match(r"\w+://", path): return path + if os.name == "nt" and posixpath.isabs(path) and ntpath.sep not in path: + return path + if os.path.isabs(path): return path # on windows convert slashes to backslashes # to have path compatible with abs_conf_dir if os.path.sep == "\\" and "/" in path: + if path.startswith("/"): + path = path.replace("/", "\\\\", 1) path = path.replace("/", "\\") expanded = os.path.expanduser(path) @@ -305,6 +312,9 @@ def _to_relpath(conf_dir, path): if os.path.expanduser(path) != path: return localfs.as_posix(path) + if os.name == "nt" and posixpath.isabs(path) and ntpath.sep not in path: + return path + if isinstance(path, RelPath) or not os.path.isabs(path): path = relpath(path, conf_dir) return localfs.as_posix(path) diff --git a/dvc/fs/dvc.py b/dvc/fs/dvc.py index fbf3f99c0f..6d79cb8f9f 100644 --- a/dvc/fs/dvc.py +++ b/dvc/fs/dvc.py @@ -732,9 +732,10 @@ def repo_url(self) -> str: return self.fs.repo_url def from_os_path(self, path: str) -> str: - if os.path.isabs(path): + if os.path.isabs(path) or ( + os.name == "nt" and posixpath.isabs(path) and ntpath.sep not in path + ): path = os.path.relpath(path, self.repo.root_dir) - return as_posix(path) def close(self): diff --git a/tests/unit/test_ignore.py b/tests/unit/test_ignore.py index 7118f3e4fb..e24a39e798 100644 --- a/tests/unit/test_ignore.py +++ b/tests/unit/test_ignore.py @@ -168,8 +168,9 @@ def test_match_ignore_from_file( ): from dvc.fs import localfs + root = r"\\" if os.name == "nt" else "/" dvcignore_path = os.path.join( - os.path.sep, "full", "path", "to", "ignore", "file", ".dvcignore" + root, "full", "path", "to", "ignore", "file", ".dvcignore" ) dvcignore_dirname = os.path.dirname(dvcignore_path)