Skip to content

Commit 32846c6

Browse files
authored
fix: namespace package calculation on windows (#3693)
Currently, when the repo computes the namespace package files on Windows, it incorrectly computes the filename because of `C:` handling and case-sensitivity, resulting in a repo-phase error. To fix, modify the relative path computation functions to detect if the platform is Windows and make comparisions case-insensitive.
1 parent ad74ef7 commit 32846c6

5 files changed

Lines changed: 125 additions & 11 deletions

File tree

python/private/pypi/pypi_repo_utils.bzl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ def _find_namespace_package_files(rctx, install_dir):
177177
to namespace packages.
178178
"""
179179

180-
repo_root = str(rctx.path(".")) + "/"
181180
namespace_package_files = []
182181
for top_level_dir in install_dir.readdir():
183182
if not is_importable_name(top_level_dir.basename):
@@ -192,7 +191,9 @@ def _find_namespace_package_files(rctx, install_dir):
192191
if ("__path__ =" in content and
193192
"pkgutil" in content and
194193
"extend_path(" in content):
195-
namespace_package_files.append(str(init_py).removeprefix(repo_root))
194+
namespace_package_files.append(
195+
repo_utils.repo_root_relative_path(rctx, init_py),
196+
)
196197

197198
return namespace_package_files
198199

python/private/repo_utils.bzl

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ def _mkdir(mrctx, path):
334334
repo_root = str(mrctx.path("."))
335335
path_str = str(path)
336336

337-
if not path_str.startswith(repo_root):
337+
if not _is_relative_to(mrctx, path_str, repo_root):
338338
mkdir_bin = mrctx.which("mkdir")
339339
if not mkdir_bin:
340340
return None
@@ -348,6 +348,30 @@ def _mkdir(mrctx, path):
348348
mrctx.delete(placeholder)
349349
return path
350350

351+
def _norm_path(mrctx, p):
352+
p = str(p)
353+
354+
# Windows is case-insensitive
355+
if _get_platforms_os_name(mrctx) == "windows":
356+
return p.lower()
357+
return p
358+
359+
def _relative_to(mrctx, path, parent, fail = fail):
360+
path_str = str(path)
361+
parent_str = str(parent)
362+
path_d = _norm_path(mrctx, path_str) + "/"
363+
parent_d = _norm_path(mrctx, parent_str) + "/"
364+
if path_d.startswith(parent_d):
365+
return path_str[len(parent_str):].removeprefix("/")
366+
else:
367+
fail("{} is not relative to {}".format(path, parent))
368+
369+
def _is_relative_to(mrctx, path, parent):
370+
"""Tell if `path` is equal to or beneath `parent`."""
371+
path_d = _norm_path(mrctx, path) + "/"
372+
parent_d = _norm_path(mrctx, parent) + "/"
373+
return path_d.startswith(parent_d)
374+
351375
def _repo_root_relative_path(mrctx, path):
352376
"""Takes a path object and returns a repo-relative path string.
353377
@@ -360,14 +384,7 @@ def _repo_root_relative_path(mrctx, path):
360384
"""
361385
repo_root = str(mrctx.path("."))
362386
path_str = str(path)
363-
relative_path = path_str[len(repo_root):]
364-
if relative_path[0] != "/":
365-
fail("{path} not under {repo_root}".format(
366-
path = path,
367-
repo_root = repo_root,
368-
))
369-
relative_path = relative_path[1:]
370-
return relative_path
387+
return _relative_to(mrctx, path_str, repo_root)
371388

372389
def _args_to_str(arguments):
373390
return " ".join([_arg_repr(a) for a in arguments])
@@ -516,6 +533,9 @@ repo_utils = struct(
516533
is_repo_debug_enabled = _is_repo_debug_enabled,
517534
logger = _logger,
518535
mkdir = _mkdir,
536+
norm_path = _norm_path,
537+
relative_to = _relative_to,
538+
is_relative_to = _is_relative_to,
519539
repo_root_relative_path = _repo_root_relative_path,
520540
which_checked = _which_checked,
521541
which_unchecked = _which_unchecked,

tests/repo_utils/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
load(":repo_utils_test.bzl", "repo_utils_test_suite")
2+
3+
repo_utils_test_suite(name = "repo_utils_tests")
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
"""Unit tests for repo_utils.bzl."""
2+
3+
load("@rules_testing//lib:test_suite.bzl", "test_suite")
4+
load("//python/private:repo_utils.bzl", "repo_utils") # buildifier: disable=bzl-visibility
5+
load("//tests/support:mocks.bzl", "mocks")
6+
7+
_tests = []
8+
9+
def _test_get_platforms_os_name(env):
10+
mock_mrctx = mocks.rctx(os_name = "Mac OS X")
11+
got = repo_utils.get_platforms_os_name(mock_mrctx)
12+
env.expect.that_str(got).equals("osx")
13+
14+
_tests.append(_test_get_platforms_os_name)
15+
16+
def _test_relative_to(env):
17+
mock_mrctx_linux = mocks.rctx(os_name = "linux")
18+
mock_mrctx_win = mocks.rctx(os_name = "windows")
19+
20+
# Case-sensitive matching (Linux)
21+
got = repo_utils.relative_to(mock_mrctx_linux, "foo/bar/baz", "foo/bar")
22+
env.expect.that_str(got).equals("baz")
23+
24+
# Case-insensitive matching (Windows)
25+
got = repo_utils.relative_to(mock_mrctx_win, "C:/Foo/Bar/Baz", "c:/foo/bar")
26+
env.expect.that_str(got).equals("Baz")
27+
28+
# Failure case
29+
failures = []
30+
31+
def _mock_fail(msg):
32+
failures.append(msg)
33+
34+
repo_utils.relative_to(mock_mrctx_linux, "foo/bar/baz", "qux", fail = _mock_fail)
35+
env.expect.that_collection(failures).contains_exactly(["foo/bar/baz is not relative to qux"])
36+
37+
_tests.append(_test_relative_to)
38+
39+
def _test_is_relative_to(env):
40+
mock_mrctx_linux = mocks.rctx(os_name = "linux")
41+
mock_mrctx_win = mocks.rctx(os_name = "windows")
42+
43+
# Case-sensitive matching (Linux)
44+
env.expect.that_bool(repo_utils.is_relative_to(mock_mrctx_linux, "foo/bar/baz", "foo/bar")).equals(True)
45+
env.expect.that_bool(repo_utils.is_relative_to(mock_mrctx_linux, "foo/bar/baz", "qux")).equals(False)
46+
47+
# Case-insensitive matching (Windows)
48+
env.expect.that_bool(repo_utils.is_relative_to(mock_mrctx_win, "C:/Foo/Bar/Baz", "c:/foo/bar")).equals(True)
49+
env.expect.that_bool(repo_utils.is_relative_to(mock_mrctx_win, "C:/Foo/Bar/Baz", "D:/Foo")).equals(False)
50+
51+
_tests.append(_test_is_relative_to)
52+
53+
def repo_utils_test_suite(name):
54+
"""Create the test suite.
55+
56+
Args:
57+
name: the name of the test suite
58+
"""
59+
test_suite(name = name, basic_tests = _tests)

tests/support/mocks.bzl

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
"""Mocks for testing."""
2+
3+
def _rctx(os_name = "linux", os_arch = "x86_64", environ = None, **kwargs):
4+
"""Creates a mock of repository_ctx or module_ctx.
5+
6+
Args:
7+
os_name: The OS name to mock (e.g., "linux", "Mac OS X", "windows").
8+
os_arch: The OS architecture to mock (e.g., "x86_64", "aarch64").
9+
environ: A dictionary representing the environment variables.
10+
**kwargs: Additional attributes to add to the mock struct.
11+
12+
Returns:
13+
A struct mocking repository_ctx.
14+
"""
15+
if environ == None:
16+
environ = {}
17+
18+
attrs = {
19+
"getenv": environ.get,
20+
"os": struct(
21+
name = os_name,
22+
arch = os_arch,
23+
),
24+
}
25+
attrs.update(kwargs)
26+
27+
return struct(**attrs)
28+
29+
mocks = struct(
30+
rctx = _rctx,
31+
)

0 commit comments

Comments
 (0)