Skip to content

added option --dump-tweaked-easyconfig to dump the easyconfig tweaked… #2636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mboisson wants to merge 3 commits into from
Closed

added option --dump-tweaked-easyconfig to dump the easyconfig tweaked… #2636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2169118
added option --dump-tweaked-easyconfig to dump the easyconfig tweaked…
mboisson Oct 23, 2018
794471e
taming hound
mboisson Oct 23, 2018
d93ce0f
merged upstream into this branch
mboisson May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion easybuild/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
:author: Toon Willems (Ghent University)
:author: Ward Poelmans (Ghent University)
:author: Fotis Georgatos (Uni.Lu, NTUA)
:author: Maxime Boissonneault (Universite Laval, Calcul Quebec, Compute Canada)
"""
import copy
import os
Expand Down Expand Up @@ -382,6 +383,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()]
Copy link
Member

@boegel boegel Oct 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Member

@ocaisa ocaisa May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@mboisson mboisson May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@ocaisa ocaisa May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally see the use case, you want easy access to the tweaked easyconfigs, there are a couple of quirks to that though.

Copy link
Member

@ocaisa ocaisa May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have something that works in #3332

Copy link
Contributor Author

@mboisson mboisson May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it compare to @boegel's #2665 ?

Copy link
Member

@ocaisa ocaisa May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he better answer that...

Copy link
Contributor Author

@mboisson mboisson May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just close this current PR if you want.

easyconfigs = tweak(easyconfigs, build_specs, modtool, targetdirs=tweaked_ecs_paths)

if options.containerize:
Expand Down Expand Up @@ -464,7 +467,8 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
inject_checksums(ordered_ecs, options.inject_checksums)

# cleanup and exit after dry run, searching easyconfigs or submitting regression test
stop_options = [options.check_conflicts, dry_run_mode, options.dump_env_script, options.inject_checksums]
stop_options = [options.check_conflicts, dry_run_mode, options.dump_env_script, options.inject_checksums,
options.dump_tweaked_easyconfig]
if any(no_ec_opts) or any(stop_options):
clean_exit(logfile, eb_tmpdir, testing)

Expand Down
2 changes: 2 additions & 0 deletions easybuild/tools/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
:author: Toon Willems (Ghent University)
:author: Ward Poelmans (Ghent University)
:author: Damian Alvarez (Forschungszentrum Juelich GmbH)
:author: Maxime Boissonneault (Universite Laval, Calcul Quebec, Compute Canada)
:author: Andy Georges (Ghent University)
"""
import copy
Expand Down Expand Up @@ -226,6 +227,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
'debug',
'debug_lmod',
'dump_autopep8',
'dump_tweaked_easyconfig',
'enforce_checksums',
'extended_dry_run',
'experimental',
Expand Down
3 changes: 3 additions & 0 deletions easybuild/tools/options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
:author: Toon Willems (Ghent University)
:author: Ward Poelmans (Ghent University)
:author: Damian Alvarez (Forschungszentrum Juelich GmbH)
:author: Maxime Boissonneault (Universite Laval, Calcul Quebec, Compute Canada)
"""
import copy
import glob
Expand Down Expand Up @@ -566,6 +567,8 @@ def informative_options(self):
'dep-graph': ("Create dependency graph", None, 'store', None, {'metavar': 'depgraph.<ext>'}),
'dump-env-script': ("Dump source script to set up build environment based on toolchain/dependencies",
None, 'store_true', False),
'dump-tweaked-easyconfig': ("Dump the easyconfig tweaked by the --try-* options to current directory",
None, 'store_true', False),
'last-log': ("Print location to EasyBuild log file of last (failed) session", None, 'store_true', False),
'list-easyblocks': ("Show list of available easyblocks",
'choice', 'store_or_None', 'simple', ['simple', 'detailed']),
Expand Down