Fixed license checking and CPATH definition #837
Conversation
|
Automatic reply from Jenkins: Can I test this? |
|
Jenkins: ok to test |
|
@damianam: needs a sync with |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1707/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. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1708/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. |
| self.install_subdir = os.path.join('linux86-64', self.version) | ||
|
|
||
| def configure_step(self): | ||
| """ | ||
| Dummy configure method, just a license check | ||
| Handle license file. It mimics intelbase.py behaviour. It should be merged into the framekwork | ||
| at some point |
There was a problem hiding this comment.
How much tweaking did you need to do here?
I'm not a big fan of including it like this, it's just a copy-paste, and then someone may start to fix issues here, but not in intelbase.py, etc. etc.
Now is the time to get this into framework...
There was a problem hiding this comment.
I haven't tweak it much. Just removed the intel bits. You are absolutely right, it is basically a copy&paste. The reason for it is to make easier the transition from this to the new state with this functionality present in the framework. Meaning that all this should become a couple of lines in the future. I guess the options are:
- Stick to a buggy easyblock until the new piece of the framework gets done and fix it then.
- Accept a copy&paste to fix the easyblock ASAP and have a trivial transition once the new piece of the framework is done.
1 is more elegant but keeps things unsolved for an undefined amount of time. 2 is a quick and ugly hack, but that works well until we have what we need in the framework.
I'd suggest to go for 1 if the changes in the framework are done before the next release, or go for 2 otherwise.
There was a problem hiding this comment.
@damianam: here you go easybuilders/easybuild-framework#1633 :)
|
Easyblocks unit test suite FAILed. See https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1709/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. |
| # $CPATH should not be defined in module for PGI, it causes problems | ||
| # cfr. https://github.com/hpcugent/easybuild-easyblocks/issues/830 | ||
| if 'CPATH' in dirs: | ||
| self.log.info("Removing $CPATH entry: %s", dirs['CPATH'] |
There was a problem hiding this comment.
missing ) at the end
Jenkins knoooooows if you don't retest ;)
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1713/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. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1746/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. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1751/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. |
|
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1757/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. |
|
Rereviewed, tested and approved, so going in. Thanks @damianam! |
Fixed license checking and CPATH definition
|
Thanks, this all looks great to me! The CPATH issue was an oversight, not deliberate. |
This PR does 2 things:
-Eliminates the
CPATHdefinition (see #830)-Improves the license checking to be compatible with license servers. It does it in the same was as
IntelBase.py. In the future, some of the functionality in these 2 places should be moved to the framework (see https://lists.ugent.be/wws/arc/easybuild/2016-02/msg00028.html)