-
Notifications
You must be signed in to change notification settings - Fork 43
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
Fix rebuild check in bot/build.sh
script, and always resume from the correct temporary directory
#518
Fix rebuild check in bot/build.sh
script, and always resume from the correct temporary directory
#518
Conversation
Instance
|
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
PR has been extensively tested in #519 The only scenario that is currently not supported is when a single PR both rebuilds and adds some package. This can be fixed by the following (tested in #519 (comment)) diff --git a/EESSI-install-software.sh b/EESSI-install-software.sh
index 470663e..63b68a5 100755
--- a/EESSI-install-software.sh
+++ b/EESSI-install-software.sh
@@ -204,11 +204,14 @@ ${EESSI_PREFIX}/scripts/gpu_support/nvidia/install_cuda_host_injections.sh -c 12
# use PR patch file to determine in which easystack files stuff was added
changed_easystacks=$(cat ${pr_diff} | grep '^+++' | cut -f2 -d' ' | sed 's@^[a-z]/@@g' | grep '^easystacks/.*yml$' | egrep -v 'known-issues|missing')
-if [ -z ${changed_easystacks} ]; then
+if [ -z "${changed_easystacks}" ]; then
echo "No missing installations, party time!" # Ensure the bot report success, as there was nothing to be build here
else
- for easystack_file in ${changed_easystacks}; do
+ # first process rebuilds if any, then easystack files for new installations
+ rebuild_easystacks=$(echo "${changed_easystacks}" | grep /rebuilds/ || true)
+ new_easystacks=$(echo "${changed_easystacks}" | grep -v /rebuilds/ || true)
+ for easystack_file in ${rebuild_easystacks} ${new_easystacks}; do
echo -e "Processing easystack file ${easystack_file}...\n\n" or a similar change in #507 |
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.
Looks good to me. Tested extensively in #519. Issues discovered therein have been addressed here too.
Due to the
set -e
, the grep command that checks for rebuild easystacks would cause the entire script to fail if there is nothing to be rebuilt. Fixed that by adding a true command to the end.Also included some other fixes, e.g. one to make sure that the tarball step resumes from the right temp dir (either from the rebuild step or build step, depending on whether a rebuild was done or not).