-
Notifications
You must be signed in to change notification settings - Fork 41
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
Improve error reported by bot when using unmerged EasyBuild PR in easystack file #494
Improve error reported by bot when using unmerged EasyBuild PR in easystack file #494
Conversation
…only for items added to the EasyStack file in _this_ PR to software-layer. If that succeeds, the reason must be that one of the EasyBuild PRs wasn't merged yet, and we can add that information to the fail_msg to make it more explicit
…e printed on a build failure here
Instance
|
bot: build repo:eessi.io-2023.06-software arch:x86_64/amd/zen3 |
Updates by the bot instance
|
New job on instance
|
bot: build repo:eessi.io-2023.06-software arch:x86_64/amd/zen3 |
Updates by the bot instance
|
New job on instance
|
bot: build repo:eessi.io-2023.06-software arch:x86_64/amd/zen3 |
Updates by the bot instance
|
New job on instance
|
bot: build repo:eessi.io-2023.06-software arch:x86_64/amd/zen3 |
Updates by the bot instance
|
New job on instance
|
Co-authored-by: ocaisa <alan.ocais@cecam.org>
…nt in running the eb --missing again in that case, since there were no --from-pr's in this PR to begin with, so unmerged PRs cannot be the reason for failing
…ftware-layer into improve_error_on_unmerged_pr
bot: build repo:eessi.io-2023.06-software arch:x86_64/amd/zen3 |
Updates by the bot instance
|
|
bot: build repo:eessi.io-2023.06-software arch:x86_64/amd/zen3 |
Updates by the bot instance
|
New job on instance
|
- CFITSIO-4.3.1-GCCcore-13.2.0.eb: | ||
options: | ||
from-pr: 19840 |
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.
Assuming this needs to be removed before we consider merging
easystacks/software.eessi.io/2023.06/eessi-2023.06-eb-4.9.0-2023b.yml
Outdated
Show resolved
Hide resolved
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
@casparvl Do you think there is something wrong with this as merged? We were planning to test it via #499 but it is not actually commenting in the tests (see #499 (comment) for example) |
301ab59 prevents deploying software for which the PR in EasyBuild is still open. In principle, that's good, but it would lead to failures in the EESSI PR which are unclear to the end user, e.g. as in #479 (comment) . In these failing cases, the only thing you see is the status is failure, and the details reflect that
ERROR:
was found, as well as a non-zero excit code oncheck_missing_installations
.This is quite unspecific if the only issue is an umerged PR. It should be clear to the end user that the build succeeded, that all the software ended up in the tarball, but the only issue is an unmerged PR.
In this PR, I use inspiration from @ocaisa 's #493 . He made it possible to make an exception for
--from-pr
statements that are added in thesoftware-layer
PR that tries to add software. In this PR, I use that conditionally:develop
. If there is an unmerged EasyBuild PR, this will fail.--from-pr
statements added in thesoftware-layer
PR. If that does pass, it is clear that some of the EasyBuild PR's referred to in thissoftware-layer
PR are not merged yet. We can then print a clear message in the output file.check-build.sh
, I pick up on this pattern. if it is present, I add areason
for the failure.Note that I also refactored the
check-build.sh
a bit. The definition and useage of certain variables was far apart, making it hard to trace what was happening. I've refactored that so that things are defined and used 'in order', which makes it easier to understand what is happening in the code.