Skip to content

fix: namespace package calculation on windows#3693

Merged
rickeylev merged 10 commits intobazel-contrib:mainfrom
rickeylev:fix.namespace.packages.windows
Apr 12, 2026
Merged

fix: namespace package calculation on windows#3693
rickeylev merged 10 commits intobazel-contrib:mainfrom
rickeylev:fix.namespace.packages.windows

Conversation

@rickeylev
Copy link
Copy Markdown
Collaborator

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.

@rickeylev rickeylev requested review from aignas and groodt as code owners April 11, 2026 08:36
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors path handling in repo_utils.bzl by introducing centralized normalization and relative path calculation functions, which are then utilized in pypi_repo_utils.bzl. However, the implementation of _is_relative_to contains reversed logic that incorrectly identifies paths as being outside the repository root. Additionally, _relative_to fails to preserve original casing on Windows and introduces unexpected trailing slashes, which may break downstream tools and imports.

@rickeylev rickeylev force-pushed the fix.namespace.packages.windows branch from 22f48cd to bcc6cab Compare April 11, 2026 08:41
@rickeylev rickeylev enabled auto-merge April 11, 2026 18:45
@rickeylev rickeylev added this pull request to the merge queue Apr 12, 2026
Merged via the queue into bazel-contrib:main with commit 32846c6 Apr 12, 2026
4 checks passed
@rickeylev rickeylev deleted the fix.namespace.packages.windows branch April 12, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants