Skip to content

Conversation

@casparvl
Copy link
Collaborator

@casparvl casparvl commented Feb 5, 2025

Fix bug when multiple modules where defined. In the old syntax, they were interpreted as a single string, with space. I.e. only one iteration of hte for loop was executed. Using bash arrays, we no longer have this issue

See #903 (comment)

…were interpreted as a single string, with space. I.e. only one iteration of hte for loop was executed. Using bash arrays, we no longer have this issue
@casparvl casparvl added the bug Something isn't working label Feb 5, 2025
@eessi-bot
Copy link

eessi-bot bot commented Feb 5, 2025

Instance eessi-bot-mc-aws is configured to build for:

  • architectures: x86_64/generic, x86_64/intel/haswell, x86_64/intel/sapphire_rapids, x86_64/intel/skylake_avx512, x86_64/amd/zen2, x86_64/amd/zen3, aarch64/generic, aarch64/neoverse_n1, aarch64/neoverse_v1
  • repositories: eessi.io-2023.06-software, eessi.io-2023.06-compat

@eessi-bot
Copy link

eessi-bot bot commented Feb 5, 2025

Instance eessi-bot-mc-azure is configured to build for:

  • architectures: x86_64/amd/zen4
  • repositories: eessi.io-2023.06-software, eessi.io-2023.06-compat

@gpu-bot-ugent
Copy link

gpu-bot-ugent bot commented Feb 5, 2025

Instance eessi-bot-vsc-ugent is configured to build for:

  • architectures: x86_64/amd/zen3
  • repositories: eessi-hpc.org-2023.06-compat, eessi.io-2023.06-software, eessi-hpc.org-2023.06-software, eessi.io-2023.06-compat

@eessi-bot-casparvl-eessi
Copy link

Instance eessi-bot-casparvl is configured to build for:

  • architectures: x86_64/amd/zen4, x86_64/amd/zen2
  • repositories: eessi.io-2023.06-compat, eessi.io-2023.06-software, eessi-hpc.org-2023.06-software, eessi-hpc.org-2023.06-compat

@trz42
Copy link
Collaborator

trz42 commented Feb 13, 2025

For me the original version works too 🤷

[trz@login1 ~]$ source /cvmfs/software.eessi.io/versions/2023.06/init/lmod/bash
EESSI/2023.06 loaded successfully
[trz@login1 ~]$ ml
Currently Loaded Modules:
  1) EESSI/2023.06
[trz@login1 ~]$ ml av easybuild
----------------------------- /cvmfs/software.eessi.io/versions/2023.06/software/linux/aarch64/neoverse_n1/modules/all ------------------------------
   EasyBuild/4.8.2    EasyBuild/4.9.1    EasyBuild/4.9.3        EESSI-extend/2023.06-easybuild
   EasyBuild/4.9.0    EasyBuild/4.9.2    EasyBuild/4.9.4 (D)
...
[trz@login1 ~]$ export LOAD_MODULES=EasyBuild/4.9.4,EESSI-extend/2023.06-easybuild
[trz@login1 ~]$ for mod in $(echo ${LOAD_MODULES} | tr ',' '\n'); do echo +$mod+; done
+EasyBuild/4.9.4+
+EESSI-extend/2023.06-easybuild+
[trz@login1 ~]$ IFS=',' read -r -a modules <<< "$(echo "${LOAD_MODULES}")"
[trz@login1 ~]$ for mod in "${modules[@]}"; do echo +$mod+; done
+EasyBuild/4.9.4+
+EESSI-extend/2023.06-easybuild+

If this works on our other bot instances, it doesn't hurt to change it.
Other bot instances may or may not have that issue. We don't know because they simply didn't need to load two modules. I'd suggest we add a note to the bot's app.cfg.example + README.md that we changed the processing in bot/build.sh and bot/test.sh, so in case one needs to load two modules one should verify that the processing works as intended.

@trz42
Copy link
Collaborator

trz42 commented Feb 13, 2025

Copy link
Collaborator

@trz42 trz42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works. Can't reproduce the problem with the original code though.

@trz42 trz42 merged commit 3f09caf into EESSI:2023.06-software.eessi.io Feb 13, 2025
49 checks passed
@eessi-bot
Copy link

eessi-bot bot commented Feb 13, 2025

PR merged! Moved [] to /project/def-users/SHARED/trash_bin/EESSI/software-layer/2025.02.13

1 similar comment
@eessi-bot
Copy link

eessi-bot bot commented Feb 13, 2025

PR merged! Moved [] to /project/def-users/SHARED/trash_bin/EESSI/software-layer/2025.02.13

@gpu-bot-ugent
Copy link

gpu-bot-ugent bot commented Feb 13, 2025

PR merged! Moved [] to /scratch/gent/vo/002/gvo00211/SHARED/trash_bin/EESSI/software-layer/2025.02.13

@riscv-eessi-io-bot
Copy link

PR merged! Moved [] to /home/eessibot/shared/trash_bin/EESSI/software-layer/2025.02.13

@casparvl
Copy link
Collaborator Author

Just as an FYI: I'm not entirely sure why it happened either, I couldn't reproduce it interactively. But what happened was very clear: it was interpreting the $(echo ${LOAD_MODULES} | tr ',' '\n') as a single item with a line-break, instead of looping over the modules. It almost must be something related to expansion / quoting /etc.
In any case, the bash array approach should be more robust. So, thanks for merging :)

@casparvl casparvl deleted the fix_load_modules branch February 18, 2025 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants