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

fix(pinning): use runfiles lookup #900

Merged
merged 2 commits into from
May 25, 2023
Merged

Conversation

alexeagle
Copy link
Contributor

This allows pinning to work with --noenable_runfiles, for example on a typical Windows setup.

Fixes #899

This allows pinning to work with --noenable_runfiles, for example on a typical Windows setup.

Fixes bazel-contrib#899
@alexeagle alexeagle requested a review from cheister as a code owner May 12, 2023 17:04
@alexeagle
Copy link
Contributor Author

Hm, this won't work with Bazel 4.0.0 - do you want a workaround so we can construct an rlocationpath the old way?

@shs96c
Copy link
Collaborator

shs96c commented May 19, 2023

We're still trying to support Bazel 4, so a fix would be appreciated. If it's not possible, we can discuss whether we want to bump the minimum version to 5.x

So, use execpath and massage the external/ portion from it.
@alexeagle
Copy link
Contributor Author

Actually Bazel 5.4.0 is what introduced $(rlocationpath). I changed to $(execpath) and trim off the external/ prefix in the shell script. My understanding is we know the file in this case is always a source file that we wrote into an external repo, so it happens that this will work.

Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you. I find it interesting to note that I can tell you're using a linux machine for development because of the change to the lock file

@shs96c shs96c merged commit 643f9c9 into bazel-contrib:master May 25, 2023
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 this pull request may close these issues.

[windows] pin: cannot find sorted_deps.json
2 participants