-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Use actual current commit id for build scan custom value #52487
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
Use actual current commit id for build scan custom value #52487
Conversation
Signed-off-by: Mark Vieira <[email protected]>
|
Pinging @elastic/es-core-infra (:Core/Infra/Build) |
gradle/build-scan.gradle
Outdated
| link 'Source', "https://github.com/elastic/elasticsearch/tree/${BuildParams.gitRevision}" | ||
| if (System.getenv('GIT_PREVIOUS_COMMIT')) { | ||
| background { | ||
| def changes = "git diff --name-only ${System.getenv('GIT_PREVIOUS_COMMIT')}..${System.getenv('GIT_COMMIT')}".execute().text.trim() |
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.
Should the use of GIT_COMMIT here also be replaced with BuildParams.gitRevision?
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.
I didn't change this because GIT_PREVIOUS_COMMIT is subject to the same "problem". I think if anything we might just want to remove this entirely if it's not guaranteed to be accurate.
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.
So yeah, since we can't rely on GIT_COMMIT or GIT_PREVIOUS_COMMIT from Jenkins being actually correct I've just ditched this entirely.
Signed-off-by: Mark Vieira <[email protected]>
rjernst
left a comment
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.
LGTM
When using runbld with
--last-good-committhe commit id reported by Jenkins via build environment variables is not guaranteed to be correct, and in most cases, it isn't. This is because runbld performs a second checkout during the build execution of the "last good commit".This pull request changes the commit id reported in build scans to the real commit, and not what Jenkins is incorrectly reporting.