fix(gazelle): Correctly resolve deps that have top-level module overlap with a gazelle_python.yaml dep module (#2160)
In https://github.com/bazelbuild/rules_python/pull/2048,
`FindThirdPartyDependency` was updated to walk up the module import path
to try to find a match in gazelle_python.yaml. This is unnecessary, as
the main resolve loop will accomplish the same task.
Additionally, the change broke the ability to configure a more specific
resolve override or resolve more specific indexed libraries.
For a real-world example of where this is a problem, `pytype` has a
`third_party` module at its top-level. In a repo that also has a
`third_party` directory, we can no longer resolve our indexed libraries
in `third_party`. When the resolve loop tried to resolve
`third_party.foo.bar`, `FindThirdPartyDependency` will immediately match
`third_party` and not give the resolve loop a chance to look in the rule
index for `third_party.foo.bar`.
The same issue appears for providing overrides that are more specific
(see the updated testcase).
This PR reverts the change to FindThirdPartyDependency and updates the
testcases to ensure that we can still resolve specific indexed packages,
explicit resolve overrides, and third party modules even when there is
an overlap in the top-level module name.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 59dfbe4..387b55b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -41,6 +41,7 @@
* (toolchain) Omit third-party python packages from coverage reports from
stage2 bootstrap template.
* (bzlmod) Properly handle relative path URLs in parse_simpleapi_html.bzl
+* (gazelle) Correctly resolve deps that have top-level module overlap with a gazelle_python.yaml dep module
### Added
* Nothing yet
diff --git a/gazelle/python/testdata/dependency_resolution_order/BUILD.in b/gazelle/python/testdata/dependency_resolution_order/BUILD.in
index 71a5c5a..aaf45f4 100644
--- a/gazelle/python/testdata/dependency_resolution_order/BUILD.in
+++ b/gazelle/python/testdata/dependency_resolution_order/BUILD.in
@@ -1 +1,2 @@
# gazelle:resolve py bar //somewhere/bar
+# gazelle:resolve py third_party.foo //third_party/foo
diff --git a/gazelle/python/testdata/dependency_resolution_order/BUILD.out b/gazelle/python/testdata/dependency_resolution_order/BUILD.out
index eebe6c3..58fd266 100644
--- a/gazelle/python/testdata/dependency_resolution_order/BUILD.out
+++ b/gazelle/python/testdata/dependency_resolution_order/BUILD.out
@@ -1,6 +1,7 @@
load("@rules_python//python:defs.bzl", "py_library")
# gazelle:resolve py bar //somewhere/bar
+# gazelle:resolve py third_party.foo //third_party/foo
py_library(
name = "dependency_resolution_order",
@@ -9,6 +10,9 @@
deps = [
"//baz",
"//somewhere/bar",
+ "//third_party",
+ "//third_party/foo",
+ "@gazelle_python_test//other_pip_dep",
"@gazelle_python_test//some_foo",
],
)
diff --git a/gazelle/python/testdata/dependency_resolution_order/__init__.py b/gazelle/python/testdata/dependency_resolution_order/__init__.py
index d9c6504..e2d0a8a 100644
--- a/gazelle/python/testdata/dependency_resolution_order/__init__.py
+++ b/gazelle/python/testdata/dependency_resolution_order/__init__.py
@@ -18,6 +18,14 @@
import baz
import foo
+# Ensure that even though @gazelle_python_test//other_pip_dep provides "third_party",
+# we can still override "third_party.foo.bar"
+import third_party.foo.bar
+
+from third_party import baz
+
+import third_party
+
_ = sys
_ = bar
_ = baz
diff --git a/gazelle/python/testdata/dependency_resolution_order/gazelle_python.yaml b/gazelle/python/testdata/dependency_resolution_order/gazelle_python.yaml
index 8615181..e62ad33 100644
--- a/gazelle/python/testdata/dependency_resolution_order/gazelle_python.yaml
+++ b/gazelle/python/testdata/dependency_resolution_order/gazelle_python.yaml
@@ -15,4 +15,5 @@
manifest:
modules_mapping:
foo: some_foo
+ third_party: other_pip_dep
pip_deps_repository_name: gazelle_python_test
diff --git a/gazelle/python/testdata/dependency_resolution_order/third_party/BUILD.in b/gazelle/python/testdata/dependency_resolution_order/third_party/BUILD.in
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/gazelle/python/testdata/dependency_resolution_order/third_party/BUILD.in
diff --git a/gazelle/python/testdata/dependency_resolution_order/third_party/BUILD.out b/gazelle/python/testdata/dependency_resolution_order/third_party/BUILD.out
new file mode 100644
index 0000000..2c130d7
--- /dev/null
+++ b/gazelle/python/testdata/dependency_resolution_order/third_party/BUILD.out
@@ -0,0 +1,7 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+ name = "third_party",
+ srcs = ["baz.py"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/dependency_resolution_order/third_party/baz.py b/gazelle/python/testdata/dependency_resolution_order/third_party/baz.py
new file mode 100644
index 0000000..e01d49c
--- /dev/null
+++ b/gazelle/python/testdata/dependency_resolution_order/third_party/baz.py
@@ -0,0 +1,17 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+
+_ = os
diff --git a/gazelle/pythonconfig/pythonconfig.go b/gazelle/pythonconfig/pythonconfig.go
index 166b575..a24a90e 100644
--- a/gazelle/pythonconfig/pythonconfig.go
+++ b/gazelle/pythonconfig/pythonconfig.go
@@ -282,23 +282,16 @@
for currentCfg := c; currentCfg != nil; currentCfg = currentCfg.parent {
if currentCfg.gazelleManifest != nil {
gazelleManifest := currentCfg.gazelleManifest
- for {
- if distributionName, ok := gazelleManifest.ModulesMapping[modName]; ok {
- var distributionRepositoryName string
- if gazelleManifest.PipDepsRepositoryName != "" {
- distributionRepositoryName = gazelleManifest.PipDepsRepositoryName
- } else if gazelleManifest.PipRepository != nil {
- distributionRepositoryName = gazelleManifest.PipRepository.Name
- }
+ if distributionName, ok := gazelleManifest.ModulesMapping[modName]; ok {
+ var distributionRepositoryName string
+ if gazelleManifest.PipDepsRepositoryName != "" {
+ distributionRepositoryName = gazelleManifest.PipDepsRepositoryName
+ } else if gazelleManifest.PipRepository != nil {
+ distributionRepositoryName = gazelleManifest.PipRepository.Name
+ }
- lbl := currentCfg.FormatThirdPartyDependency(distributionRepositoryName, distributionName)
- return lbl.String(), true
- }
- i := strings.LastIndex(modName, ".")
- if i == -1 {
- break
- }
- modName = modName[:i]
+ lbl := currentCfg.FormatThirdPartyDependency(distributionRepositoryName, distributionName)
+ return lbl.String(), true
}
}
}
