fix(gazelle): fix module map for old-style namespaces (#4135)
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
diff --git a/gazelle/modules_mapping/generator.py b/gazelle/modules_mapping/generator.py
index 611910c..e195473 100644
--- a/gazelle/modules_mapping/generator.py
+++ b/gazelle/modules_mapping/generator.py
@@ -54,23 +54,6 @@
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 @@
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 deb0cb2..a536dc2 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 @@
# 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 87c35f4..845e1f9 100644
--- a/gazelle/modules_mapping/test_merger.py
+++ b/gazelle/modules_mapping/test_merger.py
@@ -48,14 +48,54 @@
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()
diff --git a/news/4135.fixed.md b/news/4135.fixed.md
new file mode 100644
index 0000000..f0be693
--- /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)).
