npm_package: version placeholder sometimes not populated properly

[devversion]

🐞 bug report

Affected Rule

npm_package

Is this a regression?

No

Description

Currently the version placeholder in the npm_package is populated by retrieving the version through a volatile stamp variable called: BUILD_SCM_VERSION.

This is not ideal because whenever the version changes, the npm_package targets are not re-built. Causing the old version to be part of the npm_package output.

Ideally the version placeholder would be a stable/non-volatile stamp variable that will cause the npm_package targets to be rebuilt if the version changes. Since this happens rarely and intentionally causes the version to be updated, it sounds like a reasonable solution.

Read more about stamp variables here: https://docs.bazel.build/versions/master/user-manual.html#flag--workspace_status_command

[alexeagle]

It's subtle - see a good writeup bazelbuild/bazel#4842

I always write release scripts with --output_base=$(mktemp -d) - you really have to make a clean build when releasing to ensure all the artifacts picked up a new tag from version control. I don't think there's any way for Bazel to fix this.

Maybe you're proposing that we stop using Bazel's stamping mechanism altogether and do something like --define=BUILD_SCM_VERSION=1.2.3 and let that env var leak through to all node programs? Still won't work if you're cutting a full-stack release where the normal bazel stamping mechanism is used for one of your non-node binaries.

[devversion]

@alexeagle Maybe I miss something, but my idea was that we just use the non-volatile stamping file and add it as an action input. That way, if one value of the stable keys changes, the npm_package will be re-assembled since the action has been invalidated.

If the contents of bazel-out/stable-status.txt change, Bazel invalidates the actions that depend on > them. In other words, if a stable key's value changes, Bazel will rerun stamped actions.

This can be achieved by using ctx.info_file instead of ctx.version_file and by renaming the key to STABLE_VERSION_PLACEHOLDER.

[alexeagle]

@devversion I should have listened to you! Yes the stable-version file ctx.info_file is better. #2157 fixes it, including using STABLE_* keys in the stamping documentation.
Next release Angular can stop doing --output_base=$(mktemp -d) which is bad because your whole build is non-incremental -only the terminal actions ought to be affected by stamping