Allow config.guess code from configuremake.py to be used in rpackage.py #1938
Conversation
Don't need this patch if we merge easybuilders/easybuild-easyblocks#1938
edmondac
changed the title
Don't need this patch if we merge easybuilders/easybuild-easyblocks#1938
Don't need this patch if we merge easybuilders/easybuild-easyblocks#1938
Don't need this patch if we merge easybuilders/easybuild-easyblocks#1938
Don't need this patch if we merge easybuilders/easybuild-easyblocks#1938
There was a problem hiding this comment.
This looks good but I'd suggest to consider doing that always: Always unpack the sources and check for a config.guess and update that if necessary (or simply always). This avoids waiting for a bug report rolling in and matches what ConfigureMake does
An example implementation is here: https://github.com/Flamefire/easybuild-easyblocks/tree/update_r_config_guess
| super(RPackage, self).run(unpack_src=True) | ||
| else: | ||
| super(RPackage, self).run() | ||
|
|
||
| if self.cfg['update_config_guess']: | ||
| confmake = ConfigureMake(self.cfg) |
There was a problem hiding this comment.
IMO the better option would be to make obtain_config_guess a static or better free function. It mostly is already anyway.
| for name in files: | ||
| if name == 'config.guess': | ||
| confmake.config_guess = os.path.join(root, name) | ||
| if not confmake.check_config_guess(): |
There was a problem hiding this comment.
The ConfigureMake EB does this unconditionally (probably based on the assumption that it is highly unlikely that the config.guess matches). So I'd do the same here.
As an alternative: Add a quiet option to the check_config_guess to avoid the warnings. IMO there is nothing to warn about here. For the ConfigureMake EB the updating of the config.guess is the natural thing to do. So the output generated by obtain_config_guess is enough
|
Question about the rpackage EasyBlock:
My approach would be to:
|
|
Deferring this to #1949 - so I'll close this PR. |
Needed for easybuilders/easybuild-easyconfigs#9639