Skip to content

Commit 179e2cb

Browse files
authored
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!
1 parent e40b609 commit 179e2cb

File tree

3 files changed

+28
-21
lines changed

3 files changed

+28
-21
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ END_UNRELEASED_TEMPLATE
7575
underlying runtime.
7676
* (performance) 90% reduction in py_binary/py_test analysis phase cost.
7777
([#3381](https://github.com/bazel-contrib/rules_python/pull/3381)).
78+
* (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)).
79+
7880

7981
{#v0-0-0-added}
8082
### Added

gazelle/manifest/defs.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ def gazelle_python_manifest(
117117
if requirements:
118118
attrs = {
119119
"env": {
120-
"_TEST_MANIFEST": "$(rootpath {})".format(manifest),
120+
"_TEST_MANIFEST": "$(rlocationpath {})".format(manifest),
121121
"_TEST_MANIFEST_GENERATOR_HASH": "$(rlocationpath {})".format(manifest_generator_hash),
122-
"_TEST_REQUIREMENTS": "$(rootpath {})".format(requirements),
122+
"_TEST_REQUIREMENTS": "$(rlocationpath {})".format(requirements),
123123
},
124124
"size": "small",
125125
}

gazelle/manifest/test/test.go

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,45 +26,50 @@ import (
2626
"path/filepath"
2727
"testing"
2828

29-
"github.com/bazelbuild/rules_go/go/runfiles"
3029
"github.com/bazel-contrib/rules_python/gazelle/manifest"
30+
"github.com/bazelbuild/rules_go/go/runfiles"
3131
)
3232

33-
func TestGazelleManifestIsUpdated(t *testing.T) {
34-
requirementsPath := os.Getenv("_TEST_REQUIREMENTS")
35-
if requirementsPath == "" {
36-
t.Fatal("_TEST_REQUIREMENTS must be set")
33+
// getResolvedRunfile resolves an environment variable to a runfiles path.
34+
// It handles getting the env var, checking it's set, and resolving it through
35+
// the runfiles mechanism, providing detailed error messages if anything fails.
36+
func getResolvedRunfile(t *testing.T, envVar string) string {
37+
t.Helper()
38+
path := os.Getenv(envVar)
39+
if path == "" {
40+
t.Fatalf("%s must be set", envVar)
3741
}
38-
39-
manifestPath := os.Getenv("_TEST_MANIFEST")
40-
if manifestPath == "" {
41-
t.Fatal("_TEST_MANIFEST must be set")
42+
resolvedPath, err := runfiles.Rlocation(path)
43+
if err != nil {
44+
t.Fatalf("failed to resolve runfiles path for %s (%q): %v", envVar, path, err)
4245
}
46+
return resolvedPath
47+
}
48+
49+
func TestGazelleManifestIsUpdated(t *testing.T) {
50+
requirementsPathResolved := getResolvedRunfile(t, "_TEST_REQUIREMENTS")
51+
manifestPathResolved := getResolvedRunfile(t, "_TEST_MANIFEST")
4352

4453
manifestFile := new(manifest.File)
45-
if err := manifestFile.Decode(manifestPath); err != nil {
54+
if err := manifestFile.Decode(manifestPathResolved); err != nil {
4655
t.Fatalf("decoding manifest file: %v", err)
4756
}
4857

4958
if manifestFile.Integrity == "" {
5059
t.Fatal("failed to find the Gazelle manifest file integrity")
5160
}
5261

53-
manifestGeneratorHashPath, err := runfiles.Rlocation(
54-
os.Getenv("_TEST_MANIFEST_GENERATOR_HASH"))
55-
if err != nil {
56-
t.Fatalf("failed to resolve runfiles path of manifest: %v", err)
57-
}
62+
manifestGeneratorHashPath := getResolvedRunfile(t, "_TEST_MANIFEST_GENERATOR_HASH")
5863

5964
manifestGeneratorHash, err := os.Open(manifestGeneratorHashPath)
6065
if err != nil {
6166
t.Fatalf("opening %q: %v", manifestGeneratorHashPath, err)
6267
}
6368
defer manifestGeneratorHash.Close()
6469

65-
requirements, err := os.Open(requirementsPath)
70+
requirements, err := os.Open(requirementsPathResolved)
6671
if err != nil {
67-
t.Fatalf("opening %q: %v", requirementsPath, err)
72+
t.Fatalf("opening %q: %v", requirementsPathResolved, err)
6873
}
6974
defer requirements.Close()
7075

@@ -73,9 +78,9 @@ func TestGazelleManifestIsUpdated(t *testing.T) {
7378
t.Fatalf("verifying integrity: %v", err)
7479
}
7580
if !valid {
76-
manifestRealpath, err := filepath.EvalSymlinks(manifestPath)
81+
manifestRealpath, err := filepath.EvalSymlinks(manifestPathResolved)
7782
if err != nil {
78-
t.Fatalf("evaluating symlink %q: %v", manifestPath, err)
83+
t.Fatalf("evaluating symlink %q: %v", manifestPathResolved, err)
7984
}
8085
t.Errorf(
8186
"%q is out-of-date. Follow the update instructions in that file to resolve this",

0 commit comments

Comments
 (0)