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

Remove duplicate perl modules and add CI check #12575

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Apr 13, 2021

@Flamefire Flamefire force-pushed the removeDuplicatePerlModules branch 2 times, most recently from 3b15181 to a14aff5 Compare April 13, 2021 10:37
@boegelbot
Copy link
Collaborator

@Flamefire: Tests failed in GitHub Actions, see https://github.com/easybuilders/easybuild-easyconfigs/actions/runs/744421481
Output from first failing test suite run:

FAIL: test_changed_files_pull_request (test.easyconfigs.easyconfigs.EasyConfigTest)
Specific checks only done for the (easyconfig) files that were changed in a pull request.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/easyconfigs/easyconfigs.py", line 974, in test_changed_files_pull_request
    self.check_Perl_modules(changed_ecs)
  File "test/easyconfigs/easyconfigs.py", line 806, in check_Perl_modules
    self.fail('\n'.join(failing_checks))
AssertionError: The following sources have the same checksum and are hence assumed to be duplicates: Carp, Carp::Heavy
Please use the name of the repeated modules as a plain string in `exts_list` of Perl-5.30.0-GCCcore-8.3.0.eb to avoid reinstalling the same module.

----------------------------------------------------------------------
Ran 11809 tests in 358.859s

FAILED (failures=1)
ERROR: Not all tests were successful.

bleep, bloop, I'm just a bot (boegelbot v20200716.01)
Please talk to my owner @boegel if you notice you me acting stupid),
or submit a pull request to https://github.com/boegel/boegelbot fix the problem.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/5ae26948ea013f25acaae65b6bcdd4cd for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/7f5c0a02118cc8f3343b2cc841b3b580 for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/ae7ce5695492e49f5a900709118df75a for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/05d7048b9bfb963973100e3e158912de for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/d9ab29f4376e674480e5536986c921a9 for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/a131819ac7df2b835a36bbc418587e9b for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2386
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
taurusa4 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/f8a1a039c0b7027628328a40d9fb318c for a full test report.

@Micket
Copy link
Contributor

Micket commented Apr 15, 2021

Just to that I understand what's happening here: Do we need the alternative names for some stuff so thats why they are these redefined packages?
(I'm unfamiliar with Perls module system)

@Flamefire
Copy link
Contributor Author

Flamefire commented Apr 15, 2021

Oh, sorry for the noise, test reports with --job are not a good idea it seems ;)

In R ECs we have packages only by name. Those are supposed to be contained in the R base installation. Similar in Perl there seem to be packages which are part of other packages. The only reason why those are added again appears to be to make sure they exist by checking for them in the sanity check. So the change here is to NOT install those (which leads to installing the same package exactly the same way again), but only use their name so that is sanity checked.
I'm not familiar with Perl myself, so that is what I understood.

TLDR: Allow adding sanity checks for packages which are sub-packages of already installed packages (basically)

@Micket
Copy link
Contributor

Micket commented Apr 15, 2021

Right. In some cases here, we don't actually have "sub" packages, but they are on the same level, like IO::Handle and friends under https://metacpan.org/release/IO
Perhaps it would be better to just have "IO" as the collection name instead of trying to list a specific submodule?
And, if so, just flat out don't allow duplicates be happy with that?

I don't think many are intentionally added IO::File to be sure.. it's almost certainly just accidental because someone (like me) didn't know there wasn't a 1-to-1 mapping of module to source in Perl.

Some of these also seem to be added despite them being part of Perl itself
Like all the IO stuff also show up here https://metacpan.org/release/perl so we shouldn't use any of those.

@Flamefire
Copy link
Contributor Author

Perhaps it would be better to just have "IO" as the collection name instead of trying to list a specific submodule?

Similar to Python ECs we use the name to do some kind of import <pkg> as a sanity check if that module exists. So collections don't seem possible.

If some of them are part of Perl we could still use this PR and in a follow-up "downgrade" them to the module name only, so they are checked for but not installed, similar to the R ECs. But that got to come from someone with more experience with Perl.

This PR is good as a check for the EasyBlock change and future removals/changes can be based on this :)

@Flamefire
Copy link
Contributor Author

@boegel @Micket There wasn't any further reply here. So what is the state?

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.

4 participants