-
Notifications
You must be signed in to change notification settings - Fork 52
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
[WIP] update licenses #675
base: 2023.06-software.eessi.io
Are you sure you want to change the base?
[WIP] update licenses #675
Conversation
Instance
|
Instance
|
WIP adding a new licenses update script PR 457; |
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.
Just had an initial look, perhaps you can add the current output of the script in a gist and link to this issue?
licenses/check_licenses.yml
Outdated
uses: actions/checkout@v3 | ||
|
||
- name: Set up Python | ||
uses: actions/setup-python@v4 |
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.
Should use specific version here, see other workflows
licenses/check_licenses.yml
Outdated
fi | ||
|
||
- name: Create a PR (if changes detected) | ||
uses: peter-evans/create-pull-request@v5 |
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.
Also needs specific commit
licenses/check_licenses.yml
Outdated
commit-message: "Auto PR: Update licenses" | ||
title: "Auto PR: Update licenses" | ||
body: ${{ steps.check_licenses.outputs.patch }} | ||
branch: main #fork branch |
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.
This is not the correct branch for EESSI
- name: Create a PR (if changes detected) | ||
id: create_pull_request | ||
uses: peter-evans/create-pull-request@5e914681df9dc83aa4e4905692ca88beb2f9e91f # v7.0.5 | ||
if: steps.check_licenses.outputs.patch != '' | ||
with: | ||
commit-message: "Auto PR: Update licenses" | ||
title: "Auto PR: Update licenses" | ||
body: ${{ steps.check_licenses.outputs.patch }} | ||
branch: update-licenses-${{ github.run_number }} | ||
base: [ "*-software.eessi.io" ] |
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.
Doesn't this mean that if a PR adds software, then it will trigger another PR to add the licence of the software to the yaml file? Instead, wouldn't you want to make a suggestion that they add the licence as part of the original PR?
pull_request: | ||
branches: [ "*-software.eessi.io" ] | ||
permissions: | ||
contents: write # set permissions for writing |
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.
This action is being given a lot of power at a global level, do we really need it to have write
permissions? Isn't it enough that we get a notification if it fails?
- name: Apply patch (if no PR created) | ||
if: steps.create_pull_request.outputs.number == '' && steps.check_licenses.outputs.patch != '' | ||
run: | | ||
if [ -f license_update.patch ] && [ -s license_update.patch ]; then | ||
git apply license_update.patch | ||
else | ||
echo "No changes to apply" | ||
fi | ||
git add licenses.json | ||
git diff --cached --exit-code || git commit -m "Update licenses.json" | ||
git push | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
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.
Rather than auto-apply a patch, can a code suggestion be made (or a PR to the source repo using a token without write permission to EESSI
)?
@@ -0,0 +1,59 @@ | |||
name: Check and update licenses |
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.
Duplicate file?
python update_licenses.py --source=pypi TensorFlow | ||
python update_licenses.py --source=github:easybuilders/easybuild EasyBuild |
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 are going to need to extract the software name from the easystack file, that is complicated. Perhaps instead you should add easystack
(and easyconfig
) support to update_licenses.py
?
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.
This could also help you decide where to look for licenses (since you have access to source_urls
)
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.
The other option is to gather software (and extension) names via Lmod
and then check them against the yaml file.
branch: update-licenses-${{ github.run_number }} | ||
base: [ "*-software.eessi.io" ] | ||
|
||
- name: Apply patch (if no PR created) |
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.
Why wouldn't the PR be created? Wouldn't that mean the previous step will have failed?
spare thoughts right now:
|
}, | ||
"ATK/2.38.0-GCCcore-13.2.0": { | ||
"License": [ | ||
"Other" |
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.
How did this end up with Other
? I see it is GPL v2 at https://gitlab.gnome.org/Archive/atk
}, | ||
"Archive-Zip/1.68-GCCcore-12.2.0": { | ||
"License": [ | ||
"Other" |
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.
Hmm, some of these are going to need some discussion. The license of this is at https://github.com/redhotpenguin/perl-Archive-Zip?tab=License-1-ov-file#readme and it is GPL1+ or an artistic licence. If we are going to put Other
we'll need a link to the licence if possible and possibly another field permission_to_redistribute
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.
Agree, this is the plain response from the ecosyste.ms API: https://repos.ecosyste.ms/api/v1/repositories/lookup?url=https%3A%2F%2Fgithub.com%2Fredhotpenguin%2Fperl-Archive-Zip
Will work today on a "smarter" way to do this and to scrape other sources because there are much more not found licenses than found or other
}, | ||
"Arrow/16.1.0-gfbf-2023b": { | ||
"License": [ | ||
"Other" |
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.
License for this is https://github.com/apache/arrow/blob/main/LICENSE.txt ("Apache-2.0")
"Retrieved From": "not found" | ||
}, | ||
"Bazel/6.3.1-GCCcore-12.3.0": { | ||
"License": "not found", |
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.
Apache 2.0 from https://github.com/bazelbuild/bazel/blob/master/LICENSE
}, | ||
"BeautifulSoup/4.12.2-GCCcore-12.3.0": { | ||
"License": [ | ||
"Other" |
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.
"Retrieved From": "https://www.fftw.org" | ||
}, | ||
"FFmpeg/6.0-GCCcore-13.2.0": { | ||
"License": "not found", |
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.
Updates:
|
Open the new one to fix the origin branch