-
Notifications
You must be signed in to change notification settings - Fork 32
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
Disable VCS stamping #393
Disable VCS stamping #393
Conversation
This pull request does not have a backport label. Could you fix it @oakrizan? 🙏
|
@@ -7,6 +7,7 @@ env: | |||
DOCKER_REGISTRY: "docker.elastic.co" | |||
STAGING_IMAGE: "${DOCKER_REGISTRY}/observability-ci" | |||
BUILDX: 1 | |||
GOFLAGS: "-buildvcs=false" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, IIUC, the issue is in beats if that's the case, aren't these changes needed to be included in the consumer instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, the elastic-agent team did something similar and they are also consumers of this library (golang-crossbuild
):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, the elastic-agent team did something similar and they are also consumers of this library (
golang-crossbuild
):
FYI @v1v the above PR you mentioned is interesting; the problem didn't get fixed by setting export GOFLAGS='-buildvcs=false'
because this value gets overwritten and is hardcoded when docker is invoked here: https://github.com/elastic/beats/blob/main/dev-tools/mage/crossbuild.go#L330 (permalink).
The problem got fixed by avoiding having a nested repo (i.e. cloning beats within another cloned repo (elastic-agent): elastic/elastic-agent@cc81e4b#diff-f8bd7a8bc1f2826590a0bd7c0937871d04d15970ee75a830324888f63ce116f5R19 . A similar reproduction is described here.
vcs stamping issues seem to be rampant after go 1.18.0, e.g. see golang/go#49004 and specifically the commit description in golang/go@4569fe6
See also https://github.com/elastic/endgame-gameover/commit/a9d2c6ca12e81eace3802e39bbd933d4cac7cbc3
cc @alexsapran
In addition, #232 might be relevant too |
d9ac9a2
to
872fe33
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not fix the issue; those flags are only relevant for building the Golang Crossbuild in Buildkite. It will also not fix the problem in the consumers.
Can you point out where this is failing in the Buildkite builds for the Golang Crossbuild?
b0b60ca
to
5d3180c
Compare
💚 Build Succeeded
History
|
💚 Build Succeeded
History
|
💚 Build Succeeded
History
|
I think we should revisit this. I am not sure why this is failing on some and working on others.
So I am not 100% sure if we should by default disable it or have it in a way that allow someone to disable it, similar to how it was tried with the elastic-agent repo. |
I spent a little more time during the weekend and with a simpler reproducer, I have able to reliable hit the error which goes away when setting I also did many reruns of with buildvcs disabled and there was no flakiness. |
While migrating beats repo faced an error during packaging.
To fix issue it's required to disable vcs stamping.