From 9a404636812d7d72ad1532bada90b5a28c815662 Mon Sep 17 00:00:00 2001 From: Garrett Holmstrom Date: Fri, 28 Aug 2026 15:52:49 -0700 Subject: [PATCH 1/2] fix(gazelle): fix module map for old-style namespaces The performance tweak in #3415 which parallelized gazelle module map generation calls simplify() on wheels individually, which always results in entries which collapse to the highest level modules which contain __init__.py files. But when two wheels do old-style namespace packages by both including that file under their top-level module, they simplify down to the top-level module, clobber each other when merged, and wind up with one wheel getting mapped for the entire namespace. This causes bug #3528. We fix this by deferring our call to simplify() until after we've merged all the wheels' module info. Fixes #3528 --- gazelle/modules_mapping/generator.py | 18 ---------- gazelle/modules_mapping/merger.py | 32 +++++++++++++++++- gazelle/modules_mapping/test_merger.py | 46 ++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 22 deletions(-) diff --git a/gazelle/modules_mapping/generator.py b/gazelle/modules_mapping/generator.py index 611910c669..e195473d69 100644 --- a/gazelle/modules_mapping/generator.py +++ b/gazelle/modules_mapping/generator.py @@ -54,23 +54,6 @@ def dig_wheel(self, whl): else: self.module_for_path(path, whl) - def simplify(self): - simplified = {} - for module, wheel_name in sorted(self.mapping.items(), key=lambda x: x[0]): - mod = module - while True: - if mod in simplified: - if simplified[mod] != wheel_name: - break - wheel_name = "" - break - if mod.count(".") == 0: - break - mod = mod.rsplit(".", 1)[0] - if wheel_name: - simplified[module] = wheel_name - self.mapping = simplified - def module_for_path(self, path, whl): ext = pathlib.Path(path).suffix if ext == ".py" or ext == ".so": @@ -118,7 +101,6 @@ def run(self, wheel: pathlib.Path) -> int: except AssertionError as error: print(error, file=self.stderr) return 1 - self.simplify() mapping_json = json.dumps(self.mapping) with open(self.output_file, "w") as f: f.write(mapping_json) diff --git a/gazelle/modules_mapping/merger.py b/gazelle/modules_mapping/merger.py index deb0cb2666..a536dc2976 100644 --- a/gazelle/modules_mapping/merger.py +++ b/gazelle/modules_mapping/merger.py @@ -6,6 +6,36 @@ from pathlib import Path +def simplify(mapping: dict) -> dict: + """Collapse entries for submodules into entries for their parents + where possible. For example, "a.b" and "a.c" become "a". + + This must run over the fully merged mapping; a submodule is redundant only + when some ancestor resolves to the same wheel, and we can determine that + only once every wheel has contributed. + + Args: + mapping: The fully merged module-to-wheel mapping to simplify. + Returns: + A new, simplified module-to-wheel mapping. + """ + simplified = {} + for module, wheel_name in sorted(mapping.items(), key=lambda x: x[0]): + mod = module + while True: + if mod in simplified: + if simplified[mod] != wheel_name: + break + wheel_name = "" + break + if mod.count(".") == 0: + break + mod = mod.rsplit(".", 1)[0] + if wheel_name: + simplified[module] = wheel_name + return simplified + + def merge_modules_mappings(input_files: list[Path], output_file: Path) -> None: """Merge multiple modules_mapping.json files into one. @@ -20,7 +50,7 @@ def merge_modules_mappings(input_files: list[Path], output_file: Path) -> None: # if there are conflicts merged_mapping.update(mapping) - output_file.write_text(json.dumps(merged_mapping)) + output_file.write_text(json.dumps(simplify(merged_mapping))) if __name__ == "__main__": diff --git a/gazelle/modules_mapping/test_merger.py b/gazelle/modules_mapping/test_merger.py index 87c35f4404..845e1f9829 100644 --- a/gazelle/modules_mapping/test_merger.py +++ b/gazelle/modules_mapping/test_merger.py @@ -48,14 +48,54 @@ def test_merger(self): self.assertEqual( { "_pytest": "pytest", - "_pytest.__init__": "pytest", - "_pytest._argcomplete": "pytest", - "_pytest.config.argparsing": "pytest", "django_types": "django_types", }, json.loads(output_path.read_text()), ) + def test_merger_keeps_distinct_namespace_package_submodules(self): + # Regression test for https://github.com/bazel-contrib/rules_python/issues/3528. + # + # Two wheels ("bosdyn_client" and "bosdyn_orbit") both contribute to the + # "bosdyn" namespace package, each shipping their own "bosdyn" entry + # (from the namespace package's __init__.py) alongside their own + # wheel-specific submodule. Since https://github.com/bazel-contrib/rules_python/pull/3415, + # each wheel's mapping is generated (and, before this fix, simplified) + # independently, so the merge must not let one wheel's "bosdyn" entry + # clobber the other's more specific submodule entries. + output_path = self.tmppath / "output.json" + merge_modules_mappings( + [ + self.make_input( + { + "bosdyn": "bosdyn_client", + "bosdyn.client": "bosdyn_client", + "bosdyn.client.control": "bosdyn_client", + } + ), + self.make_input( + { + "bosdyn": "bosdyn_orbit", + "bosdyn.orbit": "bosdyn_orbit", + "bosdyn.orbit.util": "bosdyn_orbit", + } + ), + ], + output_path, + ) + + # "bosdyn.orbit"/"bosdyn.orbit.util" are redundant with the top-level + # "bosdyn" -> "bosdyn_orbit" entry and get collapsed away, but + # "bosdyn.client" must survive since it resolves to a different wheel + # than the top-level "bosdyn" entry. + self.assertEqual( + { + "bosdyn": "bosdyn_orbit", + "bosdyn.client": "bosdyn_client", + }, + json.loads(output_path.read_text()), + ) + if __name__ == "__main__": unittest.main() From 210d0f5c1088b0fcd5d0bd7f179e769e79a149fc Mon Sep 17 00:00:00 2001 From: Garrett Holmstrom Date: Mon, 31 Aug 2026 23:57:27 -0700 Subject: [PATCH 2/2] Add news file --- news/4135.fixed.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 news/4135.fixed.md diff --git a/news/4135.fixed.md b/news/4135.fixed.md new file mode 100644 index 0000000000..f0be6936ba --- /dev/null +++ b/news/4135.fixed.md @@ -0,0 +1,3 @@ +(gazelle) Fixed a regression from version 1.8.0 which broke module map +generation for old-style namespace packages. +([#4135](https://github.com/bazel-contrib/rules_python/pull/4135)).