-
Notifications
You must be signed in to change notification settings - Fork 64
Draft: Licensing #1149
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
base: main
Are you sure you want to change the base?
Draft: Licensing #1149
Conversation
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: eessi/github-action-eessi@v3 |
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.
You could consider something like
https://github.com/EESSI/software-layer-scripts/blob/99c82b562e243cc6f566e72fbcbbe1a23d568d0c/.github/workflows/test-eb-hooks.yml#L24-L27
here. That would mean you can just
module unload EESSI
export EESSI_SOFTWARE_SUBDIR_OVERRIDE=${{matrix.EESSI_SOFTWARE_SUBDIR_OVERRIDE}}
module load EESSI/${{matrix.EESSI_VERSION}}
Note that you can currently not creating any matrix (so your action is not quite correct yet). I wonder if you need to do an architecture override at all, it is probably enough to just do this on x86 and Arm since they sometimes have slightly different dependencies. An example of an OS matrix that does exactly that is https://github.com/EESSI/github-action-eessi/blob/main/.github/workflows/minimal-usage.yml
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.
Didn't consider that different architectures would mean different dependencies, do we have any know example for that?
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.
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.
i've checked in all easyconfigs and LAMMPS is the only one that adds dependencies depending on the arch, the rest just change some compilation options which is not relevant here, anyway i fixed the matrix (with only x86 and a64fx) and will add what you mentioned to add the EESSI version
echo "Modules to check" | ||
cat missing_modules.txt | ||
|
||
# Check if module exists as key in JSON |
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.
To understand this, you are using the full module name as a key? Isn't that too restrictive? Someone would have to add each new version to the file. I would have thought that you only check for the name.
How will this cover extensions? You may want to consider a different approach to this check. Take a look at what I have done in https://github.com/EESSI/software-layer/blob/main/.github/workflows/test_compare_stacks.yml, you could reuse https://github.com/EESSI/software-layer/blob/main/.github/workflows/scripts/compare_stacks.py#L44 to gather all modules and extensions and check for them in your json file. I think you still need what you have, as you do want to fail the CI check if anything is missing (this will force maintainers to rerun the check before they merge, which will retrigger the license check).
That seems like a decent design to me though then, we fail the CI if anything is missing. You can do a preliminary licence check based on eb_missing_out
to give a first indication of what may need to be added. When CI is run and things are not missing that means they have been ingested and we can then do the full license check (which includes extensions).
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.
That's something I discussed with Pedro, if we use the full name it would mean that for every update of the software we are re-checking the licensing, it's very unlikely that a software will change its license but it could happen.
About extensions, it would go on a separate way and file (extension_licenses.json). We thought of maybe in the preliminary stages of the check, once we gathered the EasyConfig file, parse if there is an exts_list
parameter and from there start a new CI for that software in particular as the logic is a bit different from a "normal" piece of software.
Co-authored-by: ocaisa <[email protected]>
New PR as per EESSI/software-layer-scripts#55
Needs further testing and refinement, as soon as scripts are tested I will move them to software-layer-scripts