remove executable flag from easyconfigs #18646
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Lot's of issues with existing ECs in test_pr_sanity_check_paths, test_pr_sha256_checksums & test_pr_python_packages How do you want to proceed? |
boegel
changed the title
Flamefire
force-pushed
the
executable-ecs
branch
2 times, most recently
from
3ae777d
to
c023781
Compare
|
@smoors Rebased after merge of #18647 Would be great to get this merged as there were 4 new files with wrong permissions (likely C&P) but getting all the tests to pass is more work than it's worth |
|
this should be a significant help here: |
Flamefire
force-pushed
the
executable-ecs
branch
from
c023781
to
32a6d96
Compare
|
@Flamefire: Tests failed in GitHub Actions, see https://github.com/easybuilders/easybuild-easyconfigs/actions/runs/6143357244 bleep, bloop, I'm just a bot (boegelbot v20200716.01) |
Flamefire
changed the title
|
@boegel How to proceed here? |
|
@Flamefire: Tests failed in GitHub Actions, see https://github.com/easybuilders/easybuild-easyconfigs/actions/runs/6546322045 bleep, bloop, I'm just a bot (boegelbot v20200716.01) |
|
@Flamefire It's probably better to re-do this exercises in the |
|
Agree to do that in addition but I'd expect users to still use the (next) 4.x release to copy ECs from and submit PRs so fleshing out those issues is IMO cheap enough to do here once in bulk |
OK, but the failing CI is making that a really painful merge... Do we have a test in place to verify that easyconfigs touched in a PR don't have exec permissions? |
Can't you ignore the failing CI and still merge that? Other PRs should succeed then (There already was one where the exec permissions led to failures unrelated to the changes in that PR) Hence I think it is less painful to do that straight-forward merge here once and ignore CI than to have contributors have to deal with the possible many times in the future.
Yes, since #18647 |
This permission flag seems to have been set accidentally.
Flamefire
force-pushed
the
executable-ecs
branch
from
bb20ac4
to
5363818
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found a bunch of ECs that were marked executable which they clearly should not. Might even be bad for security
I fixed that and added a test, that (changed) ECs are readable, owner-writable and non-executable, i.e. at least 0644 without any executable flag.
An example failure I forced to test the test:
AssertionError: CFITSIO-3.34-ictce-5.5.0.eb must be readable (owner, group, other), not executable, at least owner writable, is: 0o100511As some of the ECs (mostly/only? in
__archive__) didn't parse correctly anymore making the test fail I did the minimal changes to fix them.