added option --dump-tweaked-easyconfig to dump the easyconfig tweaked… #2636
Conversation
… by the --try-* options to the current working directory
|
Tagging @ocaisa |
| @@ -409,6 +410,8 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None): | |||
| # don't try and tweak anything if easyconfigs were generated, since building a full dep graph will fail | |||
| # if easyconfig files for the dependencies are not available | |||
| if try_to_generate and build_specs and not generated_ecs: | |||
| if options.dump_tweaked_easyconfig: | |||
| tweaked_ecs_paths = [os.getcwd(), os.getcwd()] | |||
There was a problem hiding this comment.
Please add a comment to explain why you need to pass the same path twice, looks a bit weird.
Also, please only do a single os.getcwd call.
And if you're up for it: add a wrapper for os.getcwd in filetools that does it in a try/except OSError, so we have proper error reporting for this in case this goes horribly wrong...
There was a problem hiding this comment.
I was about to consider a PR related to this, I have a few problems with this approach. It's greedy, it will copy over all the tweaked easyconfigs and place them indiscriminately in the current directory. This is not what happens behind the scenes though, and that is why we use two paths. The first is prepended to the robot path, the second appended, by setting them to be equal you are giving precedence to the tweaked easyconfigs and this is not what you want.
It's also pretty complicated for a user to reproduce that search hierarchy (you must explicitly set the robot path and the config value needs to be sandwiched in the middle), you'd need to advise them how to do that.
What I was considering was doing this copy once the dependencies have been resolved. At that point you can check for tweaked_easyconfigs and tweaked_dep_easyconfigs in the paths of the final ec's and only dump those. This would remove the need to worry about setting the robot path.
There was a problem hiding this comment.
I trust you on that. To be honest, this has been opened for so long, I don't even know if the problem I was trying to address is still there. I was just trying to kick it up by merging develop, to see if tests would pass.
There was a problem hiding this comment.
I totally see the use case, you want easy access to the tweaked easyconfigs, there are a couple of quirks to that though.
There was a problem hiding this comment.
I think I have something that works in #3332
There was a problem hiding this comment.
You can just close this current PR if you want.
|
@mboisson A test for this new CLI option should be added too.... :) |
|
Mmm, I'm not sure what these options do ? Do they apply after hooks/try-... are done ? Is there something that stops the build when those options are called ? |
|
@mboisson Status of this one? |
|
Status is I don't remember what was discussed a year ago :) |
… by the --try-* options to the current working directory