From b8bb342bc30a6c441bd9d68c20048705f9f3a2b1 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 7 Dec 2023 09:58:45 +0100 Subject: [PATCH] Always attempt to set RUNFILES_DIR and JAVA_RUNFILES in `runfiles.Env` These variables are needed by e.g. the Java launcher even when running with a manifest file. Match the logic of the Bazel-provided runfiles libraries. --- go/runfiles/directory.go | 7 ++++-- go/runfiles/manifest.go | 17 +++++++++++++- go/runfiles/runfiles.go | 18 +++++++-------- tests/runfiles/runfiles_test.go | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/go/runfiles/directory.go b/go/runfiles/directory.go index 3a67e7eae2..456cc16409 100644 --- a/go/runfiles/directory.go +++ b/go/runfiles/directory.go @@ -23,8 +23,11 @@ type Directory string func (d Directory) new(sourceRepo SourceRepo) (*Runfiles, error) { r := &Runfiles{ - impl: d, - env: directoryVar + "=" + string(d), + impl: d, + env: []string{ + directoryVar + "=" + string(d), + legacyDirectoryVar + "=" + string(d), + }, sourceRepo: string(sourceRepo), } err := r.loadRepoMapping() diff --git a/go/runfiles/manifest.go b/go/runfiles/manifest.go index 4d8e87987f..1c094df0d3 100644 --- a/go/runfiles/manifest.go +++ b/go/runfiles/manifest.go @@ -33,9 +33,24 @@ func (f ManifestFile) new(sourceRepo SourceRepo) (*Runfiles, error) { if err != nil { return nil, err } + env := []string{ + manifestFileVar + "=" + string(f), + } + // Certain tools (e.g., Java tools) may need the runfiles directory, so try to find it even if + // running with a manifest file. + if strings.HasSuffix(string(f), ".runfiles_manifest") || + strings.HasSuffix(string(f), "/MANIFEST") || + strings.HasSuffix(string(f), "\\MANIFEST") { + // Cut off either "_manifest" or "/MANIFEST" or "\\MANIFEST", all of length 9, from the end + // of the path to obtain the runfiles directory. + d := string(f)[:len(string(f))-len("_manifest")] + env = append(env, + directoryVar+"="+d, + legacyDirectoryVar+"="+d) + } r := &Runfiles{ impl: m, - env: manifestFileVar + "=" + string(f), + env: env, sourceRepo: string(sourceRepo), } err = r.loadRepoMapping() diff --git a/go/runfiles/runfiles.go b/go/runfiles/runfiles.go index bc57b17bd4..3e4792f5b0 100644 --- a/go/runfiles/runfiles.go +++ b/go/runfiles/runfiles.go @@ -14,19 +14,19 @@ // Package runfiles provides access to Bazel runfiles. // -// Usage +// # Usage // // This package has two main entry points, the global functions Rlocation and Env, // and the Runfiles type. // -// Global functions +// # Global functions // // For simple use cases that don’t require hermetic behavior, use the Rlocation and // Env functions to access runfiles. Use Rlocation to find the filesystem location // of a runfile, and use Env to obtain environmental variables to pass on to // subprocesses. // -// Runfiles type +// # Runfiles type // // If you need hermetic behavior or want to change the runfiles discovery // process, use New to create a Runfiles object. New accepts a few options to @@ -45,8 +45,9 @@ import ( ) const ( - directoryVar = "RUNFILES_DIR" - manifestFileVar = "RUNFILES_MANIFEST_FILE" + directoryVar = "RUNFILES_DIR" + legacyDirectoryVar = "JAVA_RUNFILES" + manifestFileVar = "RUNFILES_MANIFEST_FILE" ) type repoMappingKey struct { @@ -62,7 +63,7 @@ type Runfiles struct { // We don’t need concurrency control since Runfiles objects are // immutable once created. impl runfiles - env string + env []string repoMapping map[repoMappingKey]string sourceRepo string } @@ -198,10 +199,7 @@ func (r *Runfiles) loadRepoMapping() error { // The return value is a newly-allocated slice; you can modify it at will. If // r is the zero Runfiles object, the return value is nil. func (r *Runfiles) Env() []string { - if r.env == "" { - return nil - } - return []string{r.env} + return r.env } // WithSourceRepo returns a Runfiles instance identical to the current one, diff --git a/tests/runfiles/runfiles_test.go b/tests/runfiles/runfiles_test.go index 093a683c60..e86371f03b 100644 --- a/tests/runfiles/runfiles_test.go +++ b/tests/runfiles/runfiles_test.go @@ -19,6 +19,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "runtime" "strings" "testing" @@ -147,3 +148,42 @@ func TestRunfiles_manifestWithDir(t *testing.T) { }) } } + +func TestRunfiles_dirEnv(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows doesn't have a runfiles directory by default") + } + + dir := t.TempDir() + r, err := runfiles.New(runfiles.Directory(dir)) + if err != nil { + t.Fatal(err) + } + + want := []string{"RUNFILES_DIR=" + dir, "JAVA_RUNFILES=" + dir} + if !reflect.DeepEqual(r.Env(), want) { + t.Errorf("Env: got %v, want %v", r.Env(), want) + } +} + +func TestRunfiles_manifestEnv(t *testing.T) { + tmp := t.TempDir() + dir := filepath.Join(tmp, "foo.runfiles") + err := os.Mkdir(dir, 0o755) + if err != nil { + t.Fatal(err) + } + manifest := filepath.Join(tmp, "foo.runfiles_manifest") + if err = os.WriteFile(manifest, []byte("foo/dir path/to/foo/dir\n"), 0o600); err != nil { + t.Fatal(err) + } + r, err := runfiles.New(runfiles.ManifestFile(manifest)) + if err != nil { + t.Fatal(err) + } + + want := []string{"RUNFILES_MANIFEST_FILE=" + manifest, "RUNFILES_DIR=" + dir, "JAVA_RUNFILES=" + dir} + if !reflect.DeepEqual(r.Env(), want) { + t.Errorf("Env: got %v, want %v", r.Env(), want) + } +}