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

Support workspace status values a.k.a. "stamping" in run_binary #185

Closed
jfirebaugh opened this issue Jul 14, 2022 · 8 comments
Closed

Support workspace status values a.k.a. "stamping" in run_binary #185

jfirebaugh opened this issue Jul 14, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@jfirebaugh
Copy link
Contributor

genrule has an (undocumented) stamp attribute that gives the command access to bazel-out/stable-status.txt and bazel-out/volatile-status.txt.

run_binary should have some way to access workspace status values as well, but in a more ergonomic way. That is, don't make the binary itself responsible for finding and parsing the .txt files -- build that into run_binary. Maybe stamp = True puts each workspace status key-value into the environment? Or the stamp value could be a list of workspace status keys to expose as environment variables.

@gregmagolan
Copy link
Member

Agreed. We'll add this.

@gregmagolan gregmagolan added the enhancement New feature or request label Jul 14, 2022
@gregmagolan gregmagolan added this to the 1.6 milestone Jul 14, 2022
@alexeagle
Copy link
Member

alexeagle commented Jul 15, 2022

I've come around to the opinion that we should just use a integer for the stamping value on rules rather than a custom config setting. It's just easier to type. Like rules_python.

@alexeagle
Copy link
Member

Actually I think I'll take this one, I've already implemented it a few times.

@alexeagle alexeagle assigned alexeagle and unassigned kormide Jul 15, 2022
@alexeagle
Copy link
Member

#186 is the first step, allowing custom rules to use stamping.

I'm thinking about your proposal, and there isn't a way to do exactly what you've suggested. The two status files are only paths at analysis time, and only an action can read their content. run_binary has no wrapper script, it runs the provided tool directly with ctx.actions.run, so it doesn't have a chance inside of the action to read the two status files at all. The first moment when those files can be read is when the provided tool is running.

I think the best we can do is set an environment for that tool like BAZEL_STAMP=1 BAZEL_STABLE_STATUS=bazel-out/stable_status.txt BAZEL_VOLATILE_STATUS=bazel-out/volatile-status.txt

alexeagle added a commit that referenced this issue Jul 15, 2022
gregmagolan added a commit that referenced this issue Jul 15, 2022
gregmagolan pushed a commit that referenced this issue Jul 15, 2022
@jfirebaugh
Copy link
Contributor Author

Thanks for the quick implementation!

Will this ability be inherited by js_run_binary from rules_js?

I'm thinking about your proposal, and there isn't a way to do exactly what you've suggested.

Ah, too bad, but understandable.

@gregmagolan
Copy link
Member

It will be inherited implicitly via **kwargs. I've put up a PR to add it to the attributes so it gets a docstring in the js_run_binary documentation, aspect-build/rules_js#290

@gregmagolan
Copy link
Member

FYI, I convinced Alex to change the default stamp value to 0 so that just landed and I cut a v1.7.0 release of bazel-lib

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

No branches or pull requests

4 participants