Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Aug 10, 2017

Merging this will make Travis enforce having SHA256 checksums for sources & patches in all easyconfigs touched by future PRs, including for extensions in Python, R, Perl easyconfigs...

Before we merge this, we should probably make it a bit less painful by:

(proof-of-concept in #5001 boegel#40)

edit: now requires easybuilders/easybuild-framework#2551

@boegel boegel modified the milestones: 3.x, 3.6.0 Mar 20, 2018
@boegel
Copy link
Member Author

boegel commented Mar 20, 2018

We should get this merged ASAP... Only blocker is letting --check-style complain about not having a SHA256 checksum?

@migueldiascosta Are you up for helping out with this (both this PR & enhancing --check-style)?

@migueldiascosta
Copy link
Member

@boegel enhancing --check-style would mean adding a _eb_check_sha256_checksums method to framework/easyconfig/style.py doing basically what this test_sha256_checksums method does?

@boegel
Copy link
Member Author

boegel commented Mar 21, 2018

@migueldiascosta Yes, basically exactly that. ;)

@migueldiascosta
Copy link
Member

@boegel wouldn't it be better to define a per-easyconfig function that both test_sha256_checksums and _eb_check_sha256_checksums could use?

@boegel
Copy link
Member Author

boegel commented Mar 21, 2018

@migueldiascosta Not sure what you mean... Like a method in the EasyConfig class?
That's probably not a good idea, parsing an easyconfig (which happens when you create an instance of EasyConfig) is relatively expensive.
For the tests that doesn't matter too much, since we already make sure we parse each easyconfig only once, but for --check-style this may slow it down significantly.

I do understand that it's a lot easier to implement through EasyConfig though, so we can check and see if it actually does make a major difference in terms of speed for --check-style or not.

@boegel boegel modified the milestones: 3.6.0, next release Apr 24, 2018
@boegel boegel modified the milestones: 3.6.1, next release May 23, 2018
@boegel boegel modified the milestones: 3.6.2, next release Jul 6, 2018
@boegel boegel changed the title add test to enforce SHA256 checksums in touched files in pull requests to develop add test to enforce SHA256 checksums in touched files in pull requests to develop (WIP) Aug 16, 2018
@boegel boegel changed the title add test to enforce SHA256 checksums in touched files in pull requests to develop (WIP) add test to enforce SHA256 checksums in touched files in pull requests to develop (REVIEW) Aug 16, 2018
@boegel boegel changed the title add test to enforce SHA256 checksums in touched files in pull requests to develop (REVIEW) add test to enforce SHA256 checksums in touched files in pull requests to develop (WIP) Aug 17, 2018
@easybuilders easybuilders deleted a comment from boegelbot Sep 4, 2018
@boegel boegel changed the title add test to enforce SHA256 checksums in touched files in pull requests to develop (WIP) add test to enforce SHA256 checksums in touched files in pull requests to develop (REVIEW) Sep 4, 2018
@easybuilders easybuilders deleted a comment from boegelbot Sep 4, 2018
@boegel
Copy link
Member Author

boegel commented Sep 4, 2018

This should be good to go now...

Proof of concept test in boegel#40, which results in failing tests because of purposefully tweaking easyconfig to trigger errors (see boegel@505d512):

FAIL: Specific checks only done for the (easyconfig) files that were changed in a pull request.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/boegel/easybuild-easyconfigs/test/easyconfigs/easyconfigs.py", line 367, in test_changed_files_pull_request
    self.check_sha256_checksums(changed_ecs)
  File "/home/travis/build/boegel/easybuild-easyconfigs/test/easyconfigs/easyconfigs.py", line 333, in check_sha256_checksums
    self.assertTrue(len(checksum_issues) == 0, "No checksum issues:\n%s" % '\n'.join(checksum_issues))
AssertionError: No checksum issues:
Non-SHA256 checksum found for bzip2-1.0.6.tar.gz: 00b516f4704d4a7cb50a1d97e6e8e15b
Checksums missing for one or more sources/patches in bzip2-1.0.6-GCCcore-8.1.0.eb: found 1 sources + 0 patches vs 0 checksums
Checksums missing for one or more sources/patches of extension numpy in Python-2.7.15-intel-2018b.eb: found 1 sources + 1 patches vs 0 checksums
Checksums missing for one or more sources/patches of extension ipaddress in Python-2.7.15-intel-2018b.eb: found 1 sources + 0 patches vs 0 checksums
Checksums missing for one or more sources/patches of component libXi in X11-20180604-GCCcore-7.3.0.eb: found 1 sources + 0 patches vs 0 checksums
Checksums missing for one or more sources/patches of component xkeyboard-config in X11-20180604-GCCcore-7.3.0.eb: found 1 sources + 0 patches vs 0 checksums

(see also https://travis-ci.org/boegel/easybuild-easyconfigs/jobs/424251294)

So everything seems to be working as expected...

@migueldiascosta, @vanzod: please review? It's time we get this merged, it should hopefully safe some time reviewing PRs since it should empower contributors more to get PRs good to go based on automated feedback...

Copy link
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm (tested it locally, setting the travis environment variables, worked as advertised)

@vanzod vanzod merged commit 1256b46 into easybuilders:develop Sep 5, 2018
@boegel
Copy link
Member Author

boegel commented Sep 5, 2018

🎉

@boegel boegel deleted the enforce_sha256_checksums branch September 5, 2018 16:24
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.

3 participants