add support for postinstallcmds for extensions#3663
add support for postinstallcmds for extensions#3663smoors wants to merge 1 commit intoeasybuilders:developfrom
Conversation
|
I tested locally that this works, using both postinstallcmds globally and for extensions in a single easyconfig. |
| # We can't inherit the 'start_dir' value from the parent (which will be set, and will most likely be wrong). | ||
| # It should be specified for the extension specifically, or be empty (so it is auto-derived). | ||
| self.cfg['start_dir'] = self.ext.get('options', {}).get('start_dir', None) | ||
| self.cfg['postinstallcmds'] = self.ext.get('options', {}).get('postinstallcmds', None) |
There was a problem hiding this comment.
Hmm, I'm a bit confused that this is needed...
Why isn't postinstallcmds picked up automatically by default?
There was a problem hiding this comment.
for the same reason that it's needed for start_dir I guess.
without this line the global postinstallcmds is used instead, but maybe you know a better way?
There was a problem hiding this comment.
start_dir is a bit special, since that's determined "on the fly" during the installation
There may be easyconfigs that rely on postinstallcmds being inherited from the parent (for start_dir that wouldn't really make sense I think).
That puts us between a rock and a hard place, I guess...
What happens currently if you set a specific postinstallcmds value for an extension (X), and also have a top-level postinstallcmds (Y)? X should override Y already. If not, that's a bug that we should definitely figure out and fix...
There was a problem hiding this comment.
What happens currently if you set a specific postinstallcmds value for an extension (X), and also have a top-level postinstallcmds (Y)?
to summarize: I have an easyconfig with several extensions. one of them has a postinstallcmds, the others don't. there is also a global postinstallcmds.
- current situation (without this PR and the easyblock PR)
only the globalpostinstallcmdsis executed, and only once at the end during the postprocessing step - using the easyblock PR only
the globalpostinstallcmdsis executed at the end and for each extension that does not havepostinstallcmds.
thepostinstallcmdsof the extension is executed for it's own extension only - using this PR ad the easyblock PR
everything works as intended
There may be easyconfigs that rely on postinstallcmds being inherited from the parent
as is shown in the summary, that is not possible for Python extensions. also, there is currently no other easyblock that uses postinstallcmds, so that should not be an issue?
X should override Y already. If not, that's a bug that we should definitely figure out and fix...
that's exactly what this PR does, although I admit I don't fully understand how all this complex stuff works
conclusion:
to me it makes a lot of sense that postinstallcmds be specified for the extension specifically, or be empty, just like for start_dir. otherwise if you don't specify it, it will be inherited from the parent, which is exactly what you dont want.
postinstallcmds is special too because in bundles you don't have a global buildcmd, buildopts, ...
There was a problem hiding this comment.
Should perhaps add a comment for why postinstallcmds is handled this way.
Without checking the rest of the code for extensions, this does make sense to me.
@boegel any other comments?
There was a problem hiding this comment.
This is not required, postinstallcmds is already correctly picked up when it's specified for a specific extension.
The problem is that postinstallcmds for extensions are just ignored (because post_install_step is not run for each extension).
That's fixed in easybuilders/easybuild-easyblocks#2404, but only for extensions that are installed for PythonPackage, we can/should fix that in framework instead.
What's more is that the current approach in easybuilders/easybuild-easyblocks#2404 results in running the postinstallcmds twice if an easyconfig uses PythonPackage as easyblock (so a standalone installation, not an extension).
We can avoid that in framework by only picking up postinstallcmds for an extension via Extension.
3 participants
(created using
eb --new-pr)cfr. easybuilders/easybuild-easyblocks#2381