Handle head git ref not found error in gradle-check #485
Conversation
rishabh6788
requested review from
peterzhuamazon,
prudhvigodithi,
gaiksaya,
Divyaasm and
zelinh
as code owners
|
@peterzhuamazon @prudhvigodithi Let me know if this works. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #485 +/- ##
=========================================
Coverage 84.32% 84.32%
Complexity 80 80
=========================================
Files 108 108
Lines 523 523
Branches 61 61
=========================================
Hits 441 441
Misses 26 26
Partials 56 56 ☔ View full report in Codecov by Sentry. |
| @@ -10,6 +10,7 @@ | |||
| import jenkins.tests.BuildPipelineTest | |||
| import org.junit.Before | |||
| import org.junit.Test | |||
| import static org.junit.Assert.* | |||
There was a problem hiding this comment.
nit: Can we please avoid wildcard imports?
vars/runGradleCheck.groovy
Outdated
| @@ -40,6 +40,14 @@ void call(Map args = [:]) { | |||
| rm -rf search | |||
| git clone ${git_repo_url} search | |||
| cd search/ | |||
|
|
|||
| if \$((git rev-parse -q --verify \"${git_reference}^{commit}\" > /dev/null 2>&1); then | |||
| echo "Commit ${git_reference} exists in head repo" | |||
There was a problem hiding this comment.
AFAIK git rev-parse -q --verify <commit-d> verifies if a commit exists anywhere on all the branches. @prudhvigodithi please correct me if I'm wrong.
There was a problem hiding this comment.
Thanks @rishabh6788, I just took at this command the git rev-parse --verify will only check if the identifier is a valid hash, we can try example like git branch -a --contains ef87b397e3c4d9571e6a29a96e438c3e006a0ea7 which should be better for our case, if the output of echo $? is 0 we should be good to checkout and run the gradle check.
There was a problem hiding this comment.
@prudhvigodithi The git rev-parse command verifies if the provided commit sha exists anywhere in the repo and if not returns exit code 1, I have verified it by testing with amending commit.
Did not get your point, it does the same thing across all branches. See https://git-scm.com/docs/git-rev-parse
There was a problem hiding this comment.
Example for this commit opensearch-project/opensearch-build@717dd0f, the git rev-parse -q --verify 717dd0f0873c6d81ff5cd3abdbc966cf10a22693 gives 0 as exit status, the same for git rev-parse -q --verify 9d0f336dc0bf6989ee573f6f22b391d725fa115f even though that commit does not exist opensearch-project/opensearch-build@9d0f336, so looks to me like its only checking if its a valid hash (not necessarily that commit exists).
There was a problem hiding this comment.
the command is wrong, please use git rev-parse -q --verify 9d0f336dc0bf6989ee573f6f22b391d725fa115f^{commit} and check again for both. @prudhvigodithi
There was a problem hiding this comment.
Ya adding ^{commit} looks good.
vars/runGradleCheck.groovy
Outdated
| @@ -40,6 +40,14 @@ void call(Map args = [:]) { | |||
| rm -rf search | |||
| git clone ${git_repo_url} search | |||
| cd search/ | |||
|
|
|||
| if \$((git rev-parse -q --verify \"${git_reference}^{commit}\" > /dev/null 2>&1); then | |||
There was a problem hiding this comment.
This exist seems way too late.
Probably better check during the part of github actions instead.
Thoughts?
There was a problem hiding this comment.
Adding it in github actions will have little to no value, as the user can force push just after the check is done in actions. The probability and time gap is way more than adding this check just after the head repo is checked out for final processing.
There was a problem hiding this comment.
With this on Jenkins tho we probably waste a runner.
Probably better to have additional stage before the actual run, and use a alpine docker container to check.
There was a problem hiding this comment.
I'm okay with that if the runners are not re-usable. In that case no change in the library is needed, just write all the logic in a different stage.
If the runner is re-usable, then it will not make much of a difference as the same can be used to run a new job one the current one aborts.
Description
When a user does multiple force push on a PR the head git ref sha is changed and the previous sha no longer exists on the head branch. This causes
gradle-checkjob to fail with error similar tofatal: reference is not a tree: f6691d4af253fb5507a969edf04479b132717c7d. Ideally in this case the job should just abort if git ref is not found.This PR adds logic to handle such cases.
Issues Resolved
#2292
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.