Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New runfiles library is missing listRunfiles #3375

Closed
gonzojive opened this issue Dec 4, 2022 · 3 comments · Fixed by #3969
Closed

New runfiles library is missing listRunfiles #3375

gonzojive opened this issue Dec 4, 2022 · 3 comments · Fixed by #3969

Comments

@gonzojive
Copy link

What version of rules_go are you using?

v0.36.0

What version of Bazel are you using?

5.3.2

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Linux

Any other potentially useful information about your toolchain?

What did you do?

Started using the new recommended runfiles library and tried to call ListRunfiles().

What did you expect to see?

Same feature available as in the github.com/bazelbuild/rules_go/go/tools/bazel library.

What did you see instead?

A listRunfiles function in the new library.

My typical usage

I often mess up runfile paths. To make the error more readable, I suggest a similarly named file in the error message.

package runfileutil

import (
	"errors"
	"fmt"
	"os"
	"path"
	"strings"

	runfiles1p "github.com/bazelbuild/rules_go/go/runfiles"
	"github.com/bazelbuild/rules_go/go/tools/bazel"
)

// Runfile returns an absolute path to the file named by "path", which should be
// a relative path from the workspace root to the file within the bazel
// workspace.
//
// Runfile may be called from tests invoked with 'bazel test' and binaries
// invoked with 'bazel run'. On Windows, only tests invoked with 'bazel test'
// are supported.
//
// If the runfile isn't found, an error is returned that suggests
// similarly-named runfiles.
func Runfile(filePath string) (string, error) {
	got, err := runfiles1p.Rlocation(filePath)
	if err == nil {
		return got, nil
	}
	if err != nil && strings.Contains(err.Error(), "could not locate file") {
		err = fmt.Errorf("%s: %w", err, os.ErrNotExist)
	}

	if !errors.Is(err, os.ErrNotExist) {
		return "", fmt.Errorf("unknown error locating runfile %q: %w", filePath, err)
	}
	base := path.Base(filePath)
	entries, err := bazel.ListRunfiles()
	if err != nil {
		return "", fmt.Errorf("could not locate %q: failed to list runfile entries: %w", filePath, err)
	}
	sameBaseEntries := filter(entries, func(entry bazel.RunfileEntry) bool {
		return path.Base(entry.ShortPath) == base
	})

	if len(sameBaseEntries) == 0 {
		return "", fmt.Errorf("could not locate %q among %d runfiles:\n  %s", filePath, len(entries),
			strings.Join(mapSlice(entries, func(e bazel.RunfileEntry) string { return e.ShortPath }), "\n  "))
	}
	return "", fmt.Errorf("could not locate %q among %d runfiles; suggested match(es):\n  %s", filePath, len(entries),
		strings.Join(mapSlice(sameBaseEntries, func(e bazel.RunfileEntry) string {
			return fmt.Sprintf("%q", e.ShortPath)
		}), "\n  "))

	// var sameBaseShortPaths []*bazel.RunfileEntry
	// for _, e := range entries {
	// 	if path.Base(e.ShortPath)
	// }
}

func mapSlice[T, R any](s []T, fn func(T) R) []R {
	var out []R
	for _, t := range s {
		out = append(out, fn(t))
	}
	return out
}

func filter[T any](s []T, fn func(T) bool) []T {
	var out []T
	for _, t := range s {
		if fn(t) {
			out = append(out, t)
		}
	}
	return out
}
@fmeum
Copy link
Member

fmeum commented Dec 4, 2022

bazel.ListRunfiles() doesn't work with manifest-based runfiles lookups (e.g. on Windows) and also doesn't take repository mappings into account. If you want to emulate its behavior, you could walk the RUNFILES_DIR returned by runfiles.Env().

That said, I believe that the right approach to make something like this work is to implement ReadDirFS for runfiles, similar to how it already implements StatFS and FS itself. This allows for the right level of abstraction and also interacts nicely with other parts of the Go stdlib.

@sluongng Is this something you are still interested in?

@gonzojive Would you be interested in contributing support for this. Doing that only for the directory-based implementation at first would definitely be an option.

If you are tired of hardcoding runfiles paths, you might also be interested in the new $(rlocationpath ...) expansion available in 5.4.0 and 6.0.0 (not yet released though).

@gonzojive
Copy link
Author

Note that without reading the implementation, I don't understand what the proper way to handle "repo mapping" is in runfiles. I suggest documenting it more in the godoc docstrings and updating the design doc. I can file a new feature request for that if desired.

2022-07-21-locating-runfiles-with-bzlmod.md looks relevant, too.


I can probably eventually send a PR with this support. I'm not sure when I will get to that, though.

@fmeum
Copy link
Member

fmeum commented Dec 4, 2022

@gonzojive Yes, as the author of https://github.com/bazelbuild/proposals/blob/main/designs/2022-07-21-locating-runfiles-with-bzlmod.md, I should probably take some time to merge it into the design docs. I am also working on a reference implementation that may actually be more helpful than a verbose natural language spec.

Let me know when you find some time and I can provide more details about repo mappings to get you started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants