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() 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)).