fix(gazelle): correct runfiles path handling in gazelle_python_manifest test (#3398)
## Summary
This PR fixes the `gazelle_python_manifest.test` failure on Linux CI by
correcting the runfiles path handling in both the Bazel rule definition
and the Go test code.
Fixes #3397
## Problem
The test was failing on Linux but passing on macOS due to inconsistent
file path handling:
- Used `$(rootpath)` instead of `$(rlocationpath)` in the Bazel rule
- Resolved runfiles paths but then didn't use the resolved values
See issue #3397 for full technical details.
## Changes
### `gazelle/manifest/defs.bzl`
- Line 120: Changed `$(rootpath)` to `$(rlocationpath)` for
`_TEST_MANIFEST`
- Line 122: Changed `$(rootpath)` to `$(rlocationpath)` for
`_TEST_REQUIREMENTS`
This makes them consistent with the existing
`_TEST_MANIFEST_GENERATOR_HASH` which already used `$(rlocationpath)`.
### `gazelle/manifest/test/test.go`
- Line 53: Use `manifestPathResolved` instead of `manifestPath` in
`manifestFile.Decode()`
- Line 73: Use `requirementsPathResolved` instead of `requirementsPath`
in `os.Open()`
- Lines 84-86: Use `manifestPathResolved` instead of `manifestPath` in
error handling
The test was already calling `runfiles.Rlocation()` to resolve the
paths, but then wasn't using the resolved values.
## Testing
Tested on Linux by running:
```bash
cd gazelle/examples/bzlmod_build_file_generation
bazel test //:gazelle_python_manifest.test
```
Result: ✅ **PASSED**
## Notes
- This fix aligns with Bazel's recommended runfiles handling practices
- All changes follow the existing pattern used for
`_TEST_MANIFEST_GENERATOR_HASH`
- Some code generation was assisted by Claude AI, but the human author
has reviewed, tested, and takes full responsibility for all changes per
the [contribution
guidelines](https://github.com/bazel-contrib/rules_python/blob/main/CONTRIBUTING.md#ai-assisted-contributions)
**Disclaimer**: This is my first contribution to this project, so I'm
not entirely certain this is the correct contribution method. Please let
me know if any changes to the PR process are needed. Happy to make
adjustments!
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7403b4..5a71e5f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -75,6 +75,8 @@
underlying runtime.
* (performance) 90% reduction in py_binary/py_test analysis phase cost.
([#3381](https://github.com/bazel-contrib/rules_python/pull/3381)).
+* (gazelle) Fix `gazelle_python_manifest.test` so that it accesses manifest files via `runfile` path handling rather than directly ([#3397](https://github.com/bazel-contrib/rules_python/issues/3397)).
+
{#v0-0-0-added}
### Added
diff --git a/gazelle/manifest/defs.bzl b/gazelle/manifest/defs.bzl
index 45fdb32..b615c4e 100644
--- a/gazelle/manifest/defs.bzl
+++ b/gazelle/manifest/defs.bzl
@@ -117,9 +117,9 @@
if requirements:
attrs = {
"env": {
- "_TEST_MANIFEST": "$(rootpath {})".format(manifest),
+ "_TEST_MANIFEST": "$(rlocationpath {})".format(manifest),
"_TEST_MANIFEST_GENERATOR_HASH": "$(rlocationpath {})".format(manifest_generator_hash),
- "_TEST_REQUIREMENTS": "$(rootpath {})".format(requirements),
+ "_TEST_REQUIREMENTS": "$(rlocationpath {})".format(requirements),
},
"size": "small",
}
diff --git a/gazelle/manifest/test/test.go b/gazelle/manifest/test/test.go
index 5804a71..77b354b 100644
--- a/gazelle/manifest/test/test.go
+++ b/gazelle/manifest/test/test.go
@@ -26,23 +26,32 @@
"path/filepath"
"testing"
- "github.com/bazelbuild/rules_go/go/runfiles"
"github.com/bazel-contrib/rules_python/gazelle/manifest"
+ "github.com/bazelbuild/rules_go/go/runfiles"
)
-func TestGazelleManifestIsUpdated(t *testing.T) {
- requirementsPath := os.Getenv("_TEST_REQUIREMENTS")
- if requirementsPath == "" {
- t.Fatal("_TEST_REQUIREMENTS must be set")
+// getResolvedRunfile resolves an environment variable to a runfiles path.
+// It handles getting the env var, checking it's set, and resolving it through
+// the runfiles mechanism, providing detailed error messages if anything fails.
+func getResolvedRunfile(t *testing.T, envVar string) string {
+ t.Helper()
+ path := os.Getenv(envVar)
+ if path == "" {
+ t.Fatalf("%s must be set", envVar)
}
+ resolvedPath, err := runfiles.Rlocation(path)
+ if err != nil {
+ t.Fatalf("failed to resolve runfiles path for %s (%q): %v", envVar, path, err)
+ }
+ return resolvedPath
+}
- manifestPath := os.Getenv("_TEST_MANIFEST")
- if manifestPath == "" {
- t.Fatal("_TEST_MANIFEST must be set")
- }
+func TestGazelleManifestIsUpdated(t *testing.T) {
+ requirementsPathResolved := getResolvedRunfile(t, "_TEST_REQUIREMENTS")
+ manifestPathResolved := getResolvedRunfile(t, "_TEST_MANIFEST")
manifestFile := new(manifest.File)
- if err := manifestFile.Decode(manifestPath); err != nil {
+ if err := manifestFile.Decode(manifestPathResolved); err != nil {
t.Fatalf("decoding manifest file: %v", err)
}
@@ -50,11 +59,7 @@
t.Fatal("failed to find the Gazelle manifest file integrity")
}
- manifestGeneratorHashPath, err := runfiles.Rlocation(
- os.Getenv("_TEST_MANIFEST_GENERATOR_HASH"))
- if err != nil {
- t.Fatalf("failed to resolve runfiles path of manifest: %v", err)
- }
+ manifestGeneratorHashPath := getResolvedRunfile(t, "_TEST_MANIFEST_GENERATOR_HASH")
manifestGeneratorHash, err := os.Open(manifestGeneratorHashPath)
if err != nil {
@@ -62,9 +67,9 @@
}
defer manifestGeneratorHash.Close()
- requirements, err := os.Open(requirementsPath)
+ requirements, err := os.Open(requirementsPathResolved)
if err != nil {
- t.Fatalf("opening %q: %v", requirementsPath, err)
+ t.Fatalf("opening %q: %v", requirementsPathResolved, err)
}
defer requirements.Close()
@@ -73,9 +78,9 @@
t.Fatalf("verifying integrity: %v", err)
}
if !valid {
- manifestRealpath, err := filepath.EvalSymlinks(manifestPath)
+ manifestRealpath, err := filepath.EvalSymlinks(manifestPathResolved)
if err != nil {
- t.Fatalf("evaluating symlink %q: %v", manifestPath, err)
+ t.Fatalf("evaluating symlink %q: %v", manifestPathResolved, err)
}
t.Errorf(
"%q is out-of-date. Follow the update instructions in that file to resolve this",
