-
Notifications
You must be signed in to change notification settings - Fork 133
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
main windows-test workflow error handling is buggy #938
Comments
I think you might have merely run into https://www.githubstatus.com/incidents/cj7gzzj30411. I say that because the other matrix jobs did not fail, even if they executed the same definition.
I am glad you asked. Azure's agents indeed have a default Bash. On Windows, this is even the Git for Windows Bash. The problem with that Bash is that it has no usable GCC. Or at least it does not have the one we expect Git for Windows to build with, and the result to pass Git's test suite. Therefore we require Regarding this, and the question whether to use |
I'm not asking about the whole workflow, just this one stage. I don't think it needs gcc. |
It looks like that PR also addresses the main item here, so, yay! |
It might not need GCC, but it does need the minimal subset of Git for Windows SDK. What we want to ensure here is not that the compiled Git would run when copied into whatever Git for Windows version is installed in that build agent, but that the compiled Git would run when bundled as a new Git for Windows. That's what this guarantees. |
Looking at some runs, I noticed some odd failures...
This run especially:
https://github.com/check-spelling/git/actions/runs/762362339/workflow
Here's an excerpt of the log (eventually the run will be Garbage Collected, and at some point before or after that, I will probably delete the repository):
❌ actions/checkout@v1
🚫 download build artifacts
🚫 extract build artifacts
🚫 download git-sdk-64-minimal
🚫 test
❌ ci/print-test-failures.sh
The first step (checkout) failed (❌), and thus the next four steps were skipped (🚫). The ci/print-test-failures.sh step has an
if: failure()
so it will run even though the job is dead (in fact, only because...).Here's the version of the workflow used:
https://github.com/git/git/blob/0981a071e2e7d229ae547fe8db2875c0e3e38944/.github/workflows/main.yml
I wonder what value is gained by trying to use
git-sdk-64-minimal
'sbash
instead of doingshell: bash
. I'm pretty sure that azure runners will havebash
as well. I'd be shocked by a CI system that supports.github
and doesn't supportshell: bash
.A quick look at https://github.com/git/git/blob/0981a071e2e7d229ae547fe8db2875c0e3e38944/ci/print-test-failures.sh doesn't seem to show any particular benefit from trying to use the custom bash (which in the case of this run wasn't downloaded).
Admittedly, there's nothing useful to report since nothing actually ran. But this is an error in an error handler, and there's no need for it to error.
I'm not sure why the checkout itself failed. I mean, I can see what it says, but it doesn't particularly make sense to me. I'm puzzled by the pinned choice of
actions/checkout@v1
instead ofactions/checkout@v2
+with:
+fetch-depth: 0
(see actions/checkout#fetch-all-history-for-all-tags-and-branches), but that's an unrelated issue.The text was updated successfully, but these errors were encountered: