Skip to content
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

add support for checksums specified in external checksums.json file #3749

Merged
merged 35 commits into from
Nov 22, 2022

Conversation

mboisson
Copy link
Contributor

@mboisson mboisson commented Jun 18, 2021

Renamed the branch since we switch from yaml to json.
This replaces #3748
to address
#3746

  • no test implemented
  • not sure what to do with --inject-checksums (added --inject-checksums-to-json)

I have tested this PR with Python-3.8.2-GCCcore-9.3.0.eb by removing the checksums from the EasyConfig, and with --enforce-checksums enabled.

It allows to remove checksums both for the main file and for the exts_list.

@mboisson mboisson changed the title Checksums external (WIP) Support checksums specified in external file (checksums.json) Jun 18, 2021
@mboisson
Copy link
Contributor Author

mboisson commented Jul 8, 2021

So far, I have assumed that one had either checksums in the EasyConfig file or a checksums.json file. The checksums.json file mechanism only takes effect if there are no checksums in the EasyConfig file.

This is not really useful to reuse existing EasyConfigs with a bump --try-software-version, as the list provided in the EasyConfig will take precedence and the build will fail.

I am rewriting the code such that both the checksums from the EasyConfig and the checksum from the Json file are loaded, and then providing a build_option to guide which one should be prioritized to validate the checksum if there are both.

@mboisson mboisson changed the title (WIP) Support checksums specified in external file (checksums.json) Support checksums specified in external file (checksums.json) Aug 3, 2021
@mboisson
Copy link
Contributor Author

mboisson commented Aug 3, 2021

@boegel, do you think we need to address --new-pr and the --inject-checksums-to-json when using --try-software-version issue in this PR ?

@mboisson mboisson closed this Nov 17, 2022
@mboisson mboisson reopened this Nov 17, 2022
@easybuilders easybuilders deleted a comment from boegelbot Nov 17, 2022
@easybuilders easybuilders deleted a comment from boegelbot Nov 17, 2022
@easybuilders easybuilders deleted a comment from boegelbot Nov 17, 2022
@easybuilders easybuilders deleted a comment from boegelbot Nov 17, 2022
@easybuilders easybuilders deleted a comment from boegelbot Nov 17, 2022
@boegel
Copy link
Member

boegel commented Nov 18, 2022

@mboisson Some proposed cleanup for the test part of this in ComputeCanada#26

I intend to make a pass over the non-test part too to propose some minor improvements, but this looks really good overall, thanks for all the effort you've put into this already! 👍

tweaks + enhanced tests for checksums.json support
@boegel boegel changed the title Support checksums specified in external file (checksums.json) add support for checksums specified in external checksums.json file Nov 22, 2022
@boegel boegel modified the milestones: 4.x, next release (4.6.3?) Nov 22, 2022
boegel and others added 2 commits November 22, 2022 22:36
tweak test_inject_checksums_to_json to avoid relying on order of keys in checksums.json
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

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