rules_go upgrade from v0.18.6 => 0.19.4 results in stamping not expanding variables

[achew22]

What version of rules_go are you using?

Upgrading from v0.18.6 to v0.19.4

n.b. that commit 37896e3 is not included in the v0.18.6 release, but is in the v0.19.4 release so I think this is possibly user error from #2110, but I'm still confused.

What version of Bazel are you using?

$ bazelisk version
Bazelisk version: development
Starting local Bazel server and connecting to it...
WARNING: ignoring LD_PRELOAD in environment.
Build label: 0.29.1
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Tue Sep 10 13:44:39 2019 (1568123079)
Build timestamp: 1568123079
Build timestamp as int: 1568123079

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Linux amd64

Any other potentially useful information about your toolchain?

No additional toolchains.

What did you do?

Here is the difference https://github.com/bazelbuild/bazel-watcher/compare/master...achew22:v0.10.4?expand=1

To run this on your local box:

THIS WILL INVOKE RANDOM CODE THAT YOU DOWNLOADED FROM THE INTERNET

git clone https://github.com/achew22/bazel-watcher && cd bazel-watcher && git fetch origin v0.10.4 && git reset --hard FETCH_HEAD && bazelisk build -s --stamp --workspace_status_command="python tools/workplace_status.py" --config=release //release/npm:package.json

I'm explicitly passing --stamp and --workspace_status_command which, when invoked with python tools/workplace_status.py prints out a STABLE_GIT_VERSION.

What did you expect to see?

I expect to have the build succeed

What did you see instead?

I catch an assert statement in my code

panic: The version string was not expanded. Got: {STABLE_GIT_VERSION}

[jayconrod]

I took a quick look at this. I was initially able to reproduce the error, but as soon as I made any change at all, it would stop reproducing.

Looking at tools/workplace_status.py, if there's no tag at the current commit (which was the case for me because of the way I cloned), it just prints:

STABLE_GIT_VERSION 

That seemed to prevent the stamp from happening. But when I made a change:

STABLE_GIT_VERSION -dirty

Here, the version got stamped with "-dirty".

After I fetched tags, the script printed a version and I couldn't reproduce the issue anymore.

Is it possible this is the issue you're seeing? If so, the link action probably just needs to be better at reading status files.

[achew22]

Did you try invoking the workplace_status command? When I try it, I get

$ python tools/workplace_status.py 
STABLE_GIT_COMMIT v0.10.3-dirty

Which, I think, comes from git describe --tags --abbrev=0 which should just show the most recent tag you're affiliated with. If you're in an empty git repo you'll get the error fatal: No names found, cannot describe anything. Maybe the issue is workplace_status.py? Can you help me reproduce that?

As another datapoint, I think it is running approximately correctly on my corp box:

$ cat bazel-out/stable-status.txt 
BUILD_EMBED_LABEL 
BUILD_USER achew22
STABLE_GIT_COMMIT v0.10.3-dirty

is the result after a blaze clean and build with --stamp.

WDYT?

[jayconrod]

A discovery: I created a trivial go_binary with a stamp. It successfully gets stamped it is built directly with the --stamp flag. It does not get stamped when it is consumed by a genrule, as is the case in this repo.

I suspect that the problem is the binary is built for the host platform and the stamp flag does not appear to be on in that configuration.

[achew22]

I can confirm this is related. when I bazel run --config=release release/npm -- $(pwd)/CONTRIBUTORS (the go binary directly with the arguments passed in explicitly) it works perfectly and prints out the correct version string.

I will send in a PR with a failing test case for rules_go

[siddharthab]

FWIW, I think the --stamp flag in bazel is not very well understood or documented (more on this in a previous comment).

[siddharthab]

I wonder if we can do something about this issue without removing our reliance on the --stamp flag.

The documentation for the --stamp flag says,

Bazel never stamps binaries that are built for the host configuration, regardless of the stamp attribute.

https://docs.bazel.build/versions/master/user-manual.html#flag--stamp

[jayconrod]

@achew22 Restarting discussion here since you pushed #2305...

It's been a while since I looked at this, but it seemed like this was an issue with Bazel. If Bazel doesn't stamp host binaries, and this is a host binary, isn't this working as intended?

[jayconrod]

Closing this issue. I assume Bazel's behavior of not stamping host binaries is WAI, but if not, it would be better to raise an issue there.