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 initial support for a toolchain based module naming scheme #1168

Merged
merged 12 commits into from Oct 14, 2015

Conversation

ocaisa
Copy link
Member

@ocaisa ocaisa commented Feb 11, 2015

where software is installed in a directory specific to each toolchain, and each toolchain module extends the module path variable.

NB This won't work unless toolchains properly inherit (iccifort -> iimpi ->intel) and/or dependencies are listed appropriately in toolchains.

The other required change in easyblock.py changed the order of statements written in the module file so that the 'module use' statement appears after loading the dependencies. This allows the module path to be updated in the correct order in this naming scheme.

software is installed in a directory specific to each toolchain, and
each toolchain module extends the path. This
won't work unless toolchains properly inherit (iccifort -> iimpi ->
intel) and dependencies are listed appropriately in toolchains.

The other required change in easyblock.py changed the order of
statements written in the module file so that the 'module use' statement
appears after loading the dependencies. This allows the module path to
be updated in the correct order in this naming scheme.
@hpcugentbot
Copy link
Contributor

Automatic reply from Jenkins: Can I test this?

@ocaisa
Copy link
Member Author

ocaisa commented Feb 11, 2015

Part of the reason for doing this is that, in a hierarchy, module spider returns pretty complicated ways to get to a particular module. Since we (choose to) expose users to our toolchains, this conflicts a little with how we explain access to installed software.

@boegel
Copy link
Member

boegel commented Feb 13, 2015

Jenkins: ok to test

This is useful when toolchains are not exposed to users.
"""
# return True
# In our case we have to load the toolchains because they are explicitly exposed when extending the module path
Copy link
Member

Choose a reason for hiding this comment

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

indent comment

@boegel
Copy link
Member

boegel commented Feb 13, 2015

we would also need to have this tested by the unit tests, to ensure it doesn't get broken by changes in the framework in the future

Fixed the names...added some humour
@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1337/
Test PASSed.

@ocaisa
Copy link
Member Author

ocaisa commented Feb 13, 2015

I'll take a look at the tests you mentioned in #1176 and see how this might be done. I'm a little surprised it passed the tests because without modifying intel or goolf to include a dependency on iimpi/gompi (and then iteratively), you almost certainly can't install software within this MNS. It will work for gpsolf and intel-para though because I've already defined them hierarchically.

I'll be able to resolve this more completely once we implement the method for subtoolchains, since I can enforce the dependency then (given this MNS is in use).

@boegel
Copy link
Member

boegel commented Feb 13, 2015

The tests pass because they're not using this new MNS.. :-)

@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1338/
Test PASSed.

@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1339/
Test PASSed.

@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1340/
Test PASSed.

@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1341/
Test PASSed.

@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1532/
Test FAILed.

@boegel
Copy link
Member

boegel commented Mar 19, 2015

@ocaisa: ignore the failing test, Jenkins must be drunk

@boegel
Copy link
Member

boegel commented Mar 19, 2015

Jenkins: test this please

@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1533/
Test PASSed.

We currently still need to expand the toolchain load because of the current lack of support for subtoolchains within the framework
@hpcugentbot
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1535/
Test PASSed.

@hpcugentbot
Copy link
Contributor

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2082/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Sep 22, 2015

@ocaisa: what's going on with your last commit? I'm not sure how useful an empty pull request is... ;)

@hpcugentbot
Copy link
Contributor

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2085/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Sep 22, 2015

@ocaisa: last tests have failed due to an issue on Jenkins, I'm looking into it

@boegel
Copy link
Member

boegel commented Sep 22, 2015

Jenkins: test this please

@ocaisa
Copy link
Member Author

ocaisa commented Sep 22, 2015

Argh, I was tidying up everything for the subtoolchain stuff and I deleted the MNS from the branch history to keep things clean...but I never forked the original devel branch so things got complicated.

@boegel
Copy link
Member

boegel commented Sep 22, 2015

@ocaisa: git revert is your friend

@hpcugentbot
Copy link
Contributor

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2090/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Contributor

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2185/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel boegel merged commit 2e96ad9 into easybuilders:develop Oct 14, 2015
@boegel
Copy link
Member

boegel commented May 9, 2016

@ocaisa any idea why we ended up not including toolchain_mns.py? It was removed in 8e18877 .

@bartoldeman was hitting a problem with easybuilders/easybuild-easyconfigs#2899, basically because ToolchainMNS as it was is unaware of PGI (I think...)

Do you have an updated version of ToolchainMNS somewhere?

@ocaisa
Copy link
Member Author

ocaisa commented May 11, 2016

It couldn't work properly because it relied on knowing which toolchain it was inheriting from (you need to have iimpi loaded to correctly load intel since things built with iimpi are only available by the path extension provided by the iimpi module). Because eb requires components of a toolchain to be expanded (you can switch this resolution off but the framework complains then), this meant you had to explicitly include the subtoolchain as a dependency for it to work.
With the features introduced by minimal-toolchains this should no longer be necessary except that the toolchain hierarchy is only available to the easyconfig class and not the mns class (at present).

@ocaisa
Copy link
Member Author

ocaisa commented May 11, 2016

Annoyingly I pulled directly from my develop branch so I need to open a new PR from a fresh branch...though the scheme is still effectively broken and will only work with some manual editing of toolchain easyconfigs to add the appropriate subtoolchain dependencies

@boegel
Copy link
Member

boegel commented May 11, 2016

@ocaisa I was mainly asking because I was figuring out the bug that got fixed with #1754; if you know it doesn't work (well), there's probably little point in contributing it back?

Although, I have to admit, this is a scheme we would consider using, so maybe it is useful to have a PR for it that makes it clear what the known problems are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants