only perform is_short_modname_for sanity check in det_short_module_name if modaltsoftname is available#2138
Conversation
…set. Without calling self.check_ec_type to obtain a full parsed easyconfig file 'modaltsoftname' is not set and the sanity check may fail.
|
I wonder if |
Travis checks found that even if check_ec_type(ec) called the 'modaltsoftname' key is not always set.
| self.log.debug("Obtained valid short module name %s" % mod_name) | ||
|
|
||
| # sanity check: obtained module name should pass the 'is_short_modname_for' check | ||
| ec = self.check_ec_type(ec) |
There was a problem hiding this comment.
@bartoldeman You should include this at the top of det_short_module_name, before you pass down ec, since otherwise you may be tranforming ec into a full EasyConfig instance twice in the same det_short_module_name call
There was a problem hiding this comment.
After thinking this through, I'm not sure that this does what you think/want it does...
check_ec_type will basically go out and ask the module naming scheme that is being used whether the provided ec dictionary is sufficient to query the module naming scheme.
This is used to determine whether or not a full easyconfig file is required for each dependency, or whether the limited information that is provided by the dependency specification (i.e., name, version, versionsuffx & toolchain) is sufficient (it is for the default EasyBuildMNS, not for HierarchicalMNS which also requires moduleclass).
So, unless the module naming scheme you are using specifies that modaltsoftname must also be provided, this probably doesn't have the effect you want it to have.
Since this is only a sanity check, we could limit it to the case where the MNS is being queried with a full EasyConfig instance, basically by only raising the error if modaltsoftware is indeed available.
What made you make this change exactly? Maybe it helps to have a bit more context here...
|
@bartoldeman Also, it would be nice if we could include an enhanced test that fails without this fix, and passes with the fix... |
|
You can trigger the issue by setting |
|
@bartoldeman In your example, you have |
|
Still M4. This way EB finds M4-1.4.17.eb and gets the modulename from there. |
|
Also, the issue |
"Since this is only a sanity check, we could limit it to the case where the MNS is being queried with a full EasyConfig instance, basically by only raising the error if modaltsoftware is indeed available."
| ]) | ||
| write_file(ec2, ec2_txt) | ||
|
|
||
| self.test_toy_build(ec_file=self.test_prefix, verify=False, |
There was a problem hiding this comment.
@bartoldeman you also need to enable raise_error here to ensure that the test will actually fail with is_short_modname_for('yot/0.0-one', 'toy') for active module naming scheme returns False, see bartoldeman#3
enhance test_toy_modaltsoftname
|
Good to go, thanks @bartoldeman! |
Fix sanity check for det_short_module_name() where modaltsoftname is set.
Without calling self.check_ec_type to obtain a full parsed easyconfig file
'modaltsoftname' is not set and the sanity check may fail.