Skip to content

determine easystack files to process via PR patch file#351

Merged
trz42 merged 2 commits intoEESSI:2023.06from
boegel:2023.06_pr_patch
Oct 4, 2023
Merged

determine easystack files to process via PR patch file#351
trz42 merged 2 commits intoEESSI:2023.06from
boegel:2023.06_pr_patch

Conversation

@boegel
Copy link
Copy Markdown
Contributor

@boegel boegel commented Oct 3, 2023

We're starting to hit GitHub rate limits more often because we're using --from-pr and --include-easyblocks-from-pr in various easystack files now.

Currently, the EESSI-pilot-install-software.sh script that is run by bot/build.sh script simply iterates over all existing easystack files, regardless of whether or not they have been changed, and thus triggers a whole bunch of useless EasyBuild sessions that hammer the GitHub API.

These changes use the <PR>.diff files that is left behind in the working directory that is prepared by the bot to only process the easystack files that are actually changed in a PR, which significantly limits the EasyBuild sessions being started.

I think it would be better if the bot collects the info on which files have been changed/added/removed by a PR, but for now this "dirty" approach would already help a lot, and can be replaced with a cleaner approach later if the bot feeds in a list of changed/removed/added files via the cfg/job.cfg file it prepares for each build job.

tested via boegel#28, works as intended

@eessi-bot
Copy link
Copy Markdown

eessi-bot Bot commented Oct 3, 2023

Instance eessi-bot-citc-aws is configured to build:

  • arch x86_64/generic for repo eessi-2021.12
  • arch x86_64/generic for repo eessi.org-2023.06-compat
  • arch x86_64/generic for repo eessi.io-2023.06-compat
  • arch x86_64/generic for repo eessi-2023.06-software
  • arch x86_64/intel/haswell for repo eessi-2021.12
  • arch x86_64/intel/haswell for repo eessi.org-2023.06-compat
  • arch x86_64/intel/haswell for repo eessi.io-2023.06-compat
  • arch x86_64/intel/haswell for repo eessi-2023.06-software
  • arch x86_64/intel/skylake_avx512 for repo eessi-2021.12
  • arch x86_64/intel/skylake_avx512 for repo eessi.org-2023.06-compat
  • arch x86_64/intel/skylake_avx512 for repo eessi.io-2023.06-compat
  • arch x86_64/intel/skylake_avx512 for repo eessi-2023.06-software
  • arch x86_64/amd/zen2 for repo eessi-2021.12
  • arch x86_64/amd/zen2 for repo eessi.org-2023.06-compat
  • arch x86_64/amd/zen2 for repo eessi.io-2023.06-compat
  • arch x86_64/amd/zen2 for repo eessi-2023.06-software
  • arch x86_64/amd/zen3 for repo eessi-2021.12
  • arch x86_64/amd/zen3 for repo eessi.org-2023.06-compat
  • arch x86_64/amd/zen3 for repo eessi.io-2023.06-compat
  • arch x86_64/amd/zen3 for repo eessi-2023.06-software
  • arch aarch64/generic for repo eessi-2021.12
  • arch aarch64/generic for repo eessi.org-2023.06-compat
  • arch aarch64/generic for repo eessi.io-2023.06-compat
  • arch aarch64/generic for repo eessi-2023.06-software
  • arch aarch64/neoverse_n1 for repo eessi-2021.12
  • arch aarch64/neoverse_n1 for repo eessi.org-2023.06-compat
  • arch aarch64/neoverse_n1 for repo eessi.io-2023.06-compat
  • arch aarch64/neoverse_n1 for repo eessi-2023.06-software
  • arch aarch64/neoverse_v1 for repo eessi-2021.12
  • arch aarch64/neoverse_v1 for repo eessi.org-2023.06-compat
  • arch aarch64/neoverse_v1 for repo eessi.io-2023.06-compat
  • arch aarch64/neoverse_v1 for repo eessi-2023.06-software

trz42
trz42 previously approved these changes Oct 3, 2023
Copy link
Copy Markdown
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.

Looks good. Two minor suggestions. Feel free to ignore them.

Comment thread EESSI-pilot-install-software.sh Outdated
Comment on lines +190 to +193
pr_diff=$(ls [0-9]*.diff | head -1)

# use PR patch file to determine in which easystack files stuff was added
for easystack_file in $(cat ${pr_diff} | grep '^+++' | cut -f2 -d' ' | sed 's@^[a-z]/@@g' | grep '^eessi.*yml$'); do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's ok. However, if we can assume that git is available we could also just do

for easystack_file in $(git diff --name-only | grep '^eessi.*yml$'); do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't git diff also list files that were removed?
With +++ I'm making sure we only get files where stuff was added (so the file will exist).

May not matter much in practice, as long as we don't remove easystack files (I think we may do that when rebuilding for eessi.io, but we could do a cleanup PR first to collapse easystack files to latest EasyBuild release, and then follow up with "build" PRs for eessi.io).

So I'll switch to this, it's a lot cleaner (and hopefully it's only temporary, since I would really prefer getting this info from the bot instead).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in 4657384

echo -e "Processing easystack file ${easystack_file}...\n\n"

# determine version of EasyBuild module to load based on EasyBuild version included in name of easystack file
eb_version=$(echo ${easystack_file} | sed 's/.*eb-\([0-9.]*\).*/\1/g')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Alternative way might be

eb_version=$(echo ${easystack_file} | cut -f 4 -d '-')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a bit less strict, at least with eb- we have some guarantee that which follows is an EasyBuild version...

So I prefer sticking to the sed.

Eventually we probably want to specify the EasyBuild version to use in the easystack file itself (but then EasyBuild needs to allow for that, I think).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ack.

Copy link
Copy Markdown
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.

Looks good.

@trz42 trz42 merged commit 8514c36 into EESSI:2023.06 Oct 4, 2023
trz42 pushed a commit to trz42/software-layer that referenced this pull request May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants