Skip to content

Switch stamping detection to ctx.configuration.stamp_binaries()#3816

Merged
krasimirgg merged 6 commits intobazelbuild:mainfrom
justinhorvitz:main
Jan 26, 2026
Merged

Switch stamping detection to ctx.configuration.stamp_binaries()#3816
krasimirgg merged 6 commits intobazelbuild:mainfrom
justinhorvitz:main

Conversation

@justinhorvitz
Copy link
Contributor

Currently, rules_rust uses a config_setting that matches --stamp=1 to detect whether the --stamp option is enabled in the current configuration. This PR removes the config_setting in favor of calling ctx.configuration.stamp_binaries(), which is restricted, but available to rules rust because it is on the restricted starlark API allowlist: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java;l=73;drc=135a03ba5dbad5a1a26ef3bde9815ea244a093d7.

The motivation for making this change is to improve Google-internal detection of stamped actions for cross-user caching eligibility. The ctx.configuration.stamp_binaries() call is limited to the case of stamp = -1, whereas the config_setting dependency was unconditional. Notably, for rust_library rules with the default attribute of stamp = 0, we are now able to certify that the rule is not affected by the --stamp flag value.

Google-internal bug number: 419546090

Copy link
Collaborator

@krasimirgg krasimirgg left a comment

Choose a reason for hiding this comment

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

thank you!

@krasimirgg krasimirgg enabled auto-merge January 26, 2026 12:38
@krasimirgg krasimirgg added this pull request to the merge queue Jan 26, 2026
Merged via the queue into bazelbuild:main with commit 9586468 Jan 26, 2026
3 checks passed
@justinhorvitz
Copy link
Contributor Author

Thanks! How do we go about importing this to google3? Is there a schedule or do we have to do it manually?

@krasimirgg
Copy link
Collaborator

Thanks! How do we go about importing this to google3? Is there a schedule or do we have to do it manually?

Let's sync internally.

github-merge-queue bot pushed a commit that referenced this pull request Jan 29, 2026
Rules that don't have a `stamp` attribute, for example
`rust_cc_proto_library_aspect`, should not be treated as stamped. This
was broken by #3816 - prior
to that, we were returning false because they also don't have a
`_stamp_flag ` attribute.

By returning true for `rust_cc_proto_library_aspect`, we're
unnecessarily including the volatile stamp files as action inputs, which
harms build caching.

---------

Co-authored-by: Krasimir Georgiev <krasimir@google.com>
jason-rl pushed a commit to runloopai/rules_rust that referenced this pull request Feb 4, 2026
…3829)

Rules that don't have a `stamp` attribute, for example
`rust_cc_proto_library_aspect`, should not be treated as stamped. This
was broken by bazelbuild#3816 - prior
to that, we were returning false because they also don't have a
`_stamp_flag ` attribute.

By returning true for `rust_cc_proto_library_aspect`, we're
unnecessarily including the volatile stamp files as action inputs, which
harms build caching.

---------

Co-authored-by: Krasimir Georgiev <krasimir@google.com>
dzbarsky added a commit to dzbarsky/rules_rust that referenced this pull request Feb 7, 2026
dzbarsky added a commit to dzbarsky/rules_rust that referenced this pull request Feb 16, 2026
dzbarsky added a commit to dzbarsky/rules_rust that referenced this pull request Feb 18, 2026
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.

3 participants

Comments