Option to disallow rules from including stable-status/volatile-status as action inputs

[alexeagle]

In working on rules_docker, I found that regardless of any configuration of the build, actions were non-deterministic because the presence of a { character in certain attributes caused rules like container_image to always stamp outputs, and since the stable-status.txt file is an input to the action, it was non-deterministic and caused cache misses.

Since there's no real contract between Bazel and rulesets regarding the meaning of --stamp and how rules should present a non-deterministic stamping facility to users, there's a lot of inconsistency. (For starters, ctx.info_file and ctx.status_file are undocumented) I think we'd benefit from an --incompatible flag on Bazel itself.

As a strawman, call it --incompatible_prevent_status_files_without_stamp which, unless --stamp is also present, would either prevent the *-status.txt files from being passed as inputs to actions, or would error the build and point to the rule implementation that is including those files as inputs.

Related:

• Allow Starlark rules to observe the --stamp setting #11164
• ctx.info_file is not documented #9363
• Remove system specific information from stable-status.txt #10177

[brandjon]

+@comius, I don't really know much about stamp so maybe you can comment?

[brandjon]

Ping @comius for triage.

[brentleyjones]

You mention --stamp, but --embed_label should also be supported, right?

[alexeagle]

Yes, I think anything that produces non-deterministic outputs has to be user-controlled rather than rule-author-controlled.

[comius]

cc @buildbreaker2021

Trying to keep things simple, I'd prefer if it's possible not to introduce additional flag. That would mean if --stamp is False, ctx.info_file and ctx.version_file should just return a "constant" file. Returning a constant file wouldn't break rules that are already using the functions and would still generate hermetic builds.

[comius]

Assigning to @buildbreaker2021, because he's handling BuildInfo in Java and C++ Starlark rules atm.

[uri-canva]

Returning a constant file wouldn't break rules that are already using the functions and would still generate hermetic builds.

I don't think that's true, since it wouldn't be possible to know what a valid constant file would be in the presence of user defined keys, so the constant file could be missing keys that user defined rules expect, and break the rules.

The built in workspace status keys already come with a redacted value, (though there's a comment saying they should be removed

bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppBuildInfo.java

Lines 42 to 43 in 0373680

	// TODO(bazel-team): (2011) Get rid of the redacted build info. We should try to make
	// the linkstamping process handle the case where those values are undefined.

), but supporting redacted values for user defined keys would require changing the format of the output of the workspace status script, for example instead of being KEY1<space>VALUE1[\nKEYN<space>VALUEN...] it could be KEY1<space>VALUE1<space>REDACTED_VALUE1[\nKEYN<space>VALUEN<space>REDACTED_VALUEN...]. That would allow users to upgrade bazel while keeping their build compatible with the current version of rule sets, until they upgrade their rule sets to versions that support the new constant files.

[pauldraper]

I agree with this request. It's a pain now for Starlark to even read the --stamp value (#11164).

---

Returning a constant file wouldn't break rules that are already using the functions and would still generate hermetic builds.

Unfortunately, there are some caveats with that approach:

1. --stamp can be altered through transitions
2. Some rules/scripts (e.g. genrule commands) have hardcoded the bazel-out paths.

(How I have used stamp transitions: I can get both a stamped artifact for deployment, and the digest of the unstamped artifact to detect if a deployment is actually necessary.)

[AustinSchuh]

Just got bit by this this week.