CI: Fix git-validation.sh
#2121
Conversation
Honny1
force-pushed
the
git-validation
branch
9 times, most recently
from
c19aa4e to
48dc9a7
Compare
|
LGTM |
|
lgtm. I'll defer to Ed and Chris (if he still bothers with PRs here) :D for final review and merge. |
|
/approve |
|
@kwilczynski: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
hack/git-validation.sh
Outdated
| EPOCH_TEST_COMMIT=$CIRRUS_BASE_SHA | ||
| if [ -z "${CIRRUS_CI}" ]; then |
There was a problem hiding this comment.
CIRRUS_BASE_SHA is, as we determined earlier this year, worthless.
I need to understand a lot more of this, like, what happened, what changed, why is main missing, what's the deal with DEST_BRANCH, and it's gonna take me a long time because I'm not familiar with this repo. Blocking until I get some time to understand.
There was a problem hiding this comment.
What a rabbit hole! TL;DR this appears to have been broken from the very beginning, and was only caught by happenstance because the error was not (and still is not) causing a CI failure.
Furthermore, the reason this works in podman is because of superconvoluted clone_script and other setup that has presumably been fine-tuned and tested over the years. We no longer have local expertise in such arcana.
Proper solution is too complicated to get into here. It'll involve doing a git clone with proper branch tags for the given branch, and testing it under different branches, and under different conditions (CIRRUS_PR vs TAG vs MERGE vs PURPLEBUTTERFLIES or whatever) and different merge bases. I don't see a good ROI.
Good-enough solution: this approach, but please change as follows:
- if -z CIRRUS_CI
+ if -z EPOCH_TEST_COMMITReason: yes, there's an implicit connection between CIRRUS_BASE_SHA and CIRRUS_CI, but it's much cleaner safer better to just actually test the actual actual, not some sort of distant cousin.
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, Honny1, kwilczynski The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
This PR fixes
git-validation.sh, failing in Cirrus CI. The problem was caused by the missingmainbranch. Cirrus CI performs a clone of a single branch with full Git history and no tags.Fixes: #2067