libint2: moved sanity check and update CPATH#300
libint2: moved sanity check and update CPATH#300boegel merged 3 commits intoeasybuilders:developfrom
Conversation
|
Automatic reply from Jenkins: Can I test this? |
There was a problem hiding this comment.
Why did you remove this?
This acts as a (proper) default sanity check that is specific to Libint2 (i.e., not simply bin, lib).
You can always override sanity_check_paths in the easyconfig file, but having a proper default should remain so imho.
There was a problem hiding this comment.
I moved it to the easyconfig. I thought it is nicer to put this kind of thing in the config instead of the block?
There was a problem hiding this comment.
There should be a good default in the easyblock, if there is an easyblock.
If sanity_check_paths is also defined in the easyconfig, then that overrides whatever is in the easyblock.
Don't remove this, it should be there, and you're also breaking backward compatibility this way.
There was a problem hiding this comment.
How am I breaking backwards compatibility here? It doesn't get checked but that shouldn't break anything? All the easyconfigs for libint2 have changed to include a sanity_check_paths.
It's no problem to move it back but I don't entirely see the point? Why put this in the easyblock? It seems more sensible to put this in the easyconfig as files and paths can change in the future?
There was a problem hiding this comment.
There might be libint2 copies of easyconfig files out there that do not specify sanity_check_paths, and those will be broken (unless libint2 provides a non-empty bin and lib dir, which is what EB checks by default).
If there is an easyblock, having a good default for sanity_check_paths there allows to remove at least 4 lines from the easyconfig, thus cleans things up.
Even if files differ across versions, I would still prefer adding a version check in the easyblock rather than copying all the sanity check paths to the easyconfigs... Those should be as clean as possible, imho.
There was a problem hiding this comment.
OK, I have re-added the check (The bin dir will be empty).
But I still don't agree with doing this in the block. I think only install logic should be done here but this is just a list of files that can change from version to version. I would rather keep the block clean and move as much as possible to the general framework (with sensible defaults) and the easyconfigs.
There was a problem hiding this comment.
+1 for wpoely86's last comment;
my stance is, that if we don't push more of the easyblocks' code towards framework (and generic easyblocks), and of course easyconfigs, we have not really understood the generalization of build procedures well enough.
|
Jenkins: ok to test |
|
retested with easybuilders/easybuild-easyconfigs#506, works fine, so merging in |
libint2: moved sanity check and update CPATH
No description provided.