Skip to content

Commit

Permalink
Always attempt to set RUNFILES_DIR and JAVA_RUNFILES in runfiles.Env
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fmeum committed Dec 7, 2023
1 parent b4b04b8 commit b8bb342
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 13 deletions.
7 changes: 5 additions & 2 deletions go/runfiles/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
17 changes: 16 additions & 1 deletion go/runfiles/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
18 changes: 8 additions & 10 deletions go/runfiles/runfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down
40 changes: 40 additions & 0 deletions tests/runfiles/runfiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"os"
"os/exec"
"path/filepath"
"reflect"
"runtime"
"strings"
"testing"
Expand Down Expand Up @@ -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)
}
}

0 comments on commit b8bb342

Please sign in to comment.