add support for --new, --copy, --edit and --show #2665

boegel · 2018-11-18T20:14:29Z

--new: create new easyconfig file based on specified information
--copy: copy easyconfig file to specified location (or current directory)
--edit: open easyconfig file with editor command specified via --editor-command-template
--show: print contents of easyconfig file

This PR is mostly about adding the support for --new. The idea here is that you can basically just throw stuff at it that should be included in a new easyconfig file. The implementation uses a bunch of heuristics to figure out what the provided information likely represents. As such, it certainly won't be perfect in all possible situations, but I'm confident it will be very useful in practice.

The other options (--copy, --edit and --show) are intended to be used together with either --new or --search, and have a pretty straightforward implementation.

Example usage (docs update coming up later):

$ eb --new example 1.2.3 foss/2018a ConfigureMake files:bin/example "This an example description" homepage=https://example.com https://files/example.com/example-1.2.3.tar.gz moduleclass=tools --show
== temporary log file in case of crash /tmp/eb-rHjhuK/easybuild-uKEQ4P.log
== Contents of easyconfig file ./example-1.2.3-foss-2018a.eb:

easyblock = 'ConfigureMake'

name = 'example'
version = '1.2.3'

homepage = {'https': '//example.com'}
description = "This an example description"

toolchain = {'name': 'foss', 'version': '2018a'}

source_urls = ['https://files/%(name)s.com']
sources = [SOURCE_TAR_GZ]


sanity_check_paths = {
    'files': ['bin/%(name)s'],
    'dirs': [],
}

moduleclass = 'tools'

== easyconfig file ./example-1.2.3-foss-2018a.eb created!

…from scratch

…han print them

…tect whether changes were made

…ling of argument passed to --copy

…e (where they belong)

…ile + leverage build_option in main.py to clean up code a bit

…allowed to perform search action

…r values, also handle sanity_check_paths

…ct values

easybuild/tools/robot.py

damianam

Haven't reviewed the tests yet. My biggest concern/comment is that this PR is very flexible, which complicates the code and can leave lots of situations potentially mishandled if the heuristics don't get it right. Shouldn't it make more sense being more strict in how the parameters are passed in the command line? Making unnecessary assumptions got me in troubles in the past :-P

damianam · 2018-11-19T17:57:07Z

easybuild/framework/easyconfig/tools.py

+    easyblock_names = [e['class'] for e in avail_easyblocks().values()]
+
+    # regular expression to recognise a version
+    version_regex = re.compile('^[0-9][0-9.-]')


Why this hyperrestrictive pattern?

The assumption here is that a version always starts with a digit, and is followed by either another digit or a . or -.

Is that too restrictive? I can loosen it up, but I think that would fit most versions out there?

You can always use eb --new-pr example version=1.2.3, that bypasses this heuristic entirely...

Well, maybe "hyperrestrictive" is a stretch. But we have a couple of cases where version is a single digit, or a digit followed by a dash (-). I guess we can consider those corner cases not considered in the heuristic?

damianam · 2018-11-20T16:15:04Z

easybuild/framework/easyconfig/tools.py

+    # regular expression to recognise a version
+    version_regex = re.compile('^[0-9][0-9.-]')
+
+    # first check whether easyconfig parameter name is specfied as '<param_name>=<value>'


damianam · 2018-11-20T16:27:51Z

easybuild/framework/easyconfig/tools.py

@@ -699,3 +701,187 @@ def avail_easyblocks():
                                       easyblock_mod_name, easyblocks[easyblock_mod_name]['loc'], path)

    return easyblocks
+
+
+def parse_param_value(string):


There is 0 logging in the method. Maybe add some? It is important to know what the heuristics are doing

Very good point, thanks, I'll look into that.

damianam · 2018-11-20T16:35:09Z

easybuild/framework/easyconfig/tools.py

+                    value[item_key] = []
+                    if item_val:
+                        value[item_key].append(item_val)
+                elif isinstance(item_val, tuple):


Can't item_val be a list as well?

No, because ; is used as a separator between different <key>:<value> entries in a dict value. And , is the separator for tuples (see below).

; is also the separator for lists, but since anything that starts with [a-z_]+: is considered a dict value, it'll get split on ; to separate different <key>=<value> entries, and hence you can never have a list-type value anymore... :)

Example: files:bin/example,lib/libexample.a;dirs:include gets parsed as:

{ 'files': ['bin/example', 'lib/libexample.a'], 'dirs': ['include'], }

; can not be used as a separator between bin/example and lib/libexample.a

damianam · 2018-11-20T16:51:00Z

easybuild/main.py

@@ -173,6 +173,73 @@ def run_contrib_style_checks(ecs, check_contrib, check_style):
    return check_contrib or check_style


+def handle_cat_copy_edit(filepaths, target=None, copy=None):


I don't understand why this returns an empty list in all cases except if --copy is used. Wouldn't it make sense to have fp appended in --edit as well?

The result of handle_cat_copy_edit is only used to print a message mentioning which files have been copied.

If you only use --edit, it'll open the file for editing in place, without copying it.

So you can go eb --search foo=1.2.3 --edit (--edit & co work together with both the added --new and the existing --search).

boegel · 2018-11-21T20:51:51Z

@damianam W.r.t. your concern about flexibility: you can always bypass the heuristics by always passing <param>=<value> style arguments, which get preference over anything else.

For anything that doesn't match this pattern, there are a number of very reasonable heuristics.

One thing I should also clarify is that eb --new only generates an easyconfig file, it does not start the actual build afterwards. So, you're supposed to check the generated easyconfig file before using it, not use it blindly...

boegel · 2018-11-22T11:28:25Z

@damianam Please re-review?

damianam · 2018-11-23T16:29:49Z

easybuild/framework/easyconfig/tools.py

+    easyblock_names = [e['class'] for e in avail_easyblocks().values()]
+
+    # regular expression to recognise a version
+    version_regex = re.compile('^[0-9][0-9.-]')


Well, maybe "hyperrestrictive" is a stretch. But we have a couple of cases where version is a single digit, or a digit followed by a dash (-). I guess we can consider those corner cases not considered in the heuristic?

damianam · 2018-11-23T16:38:52Z

test/framework/easyconfig.py

+        self.assertEqual(parse_param_value(''), (None, ''))
+        self.assertEqual(parse_param_value('test123'), (None, 'test123'))
+
+        self.assertEqual(parse_param_value("this is a description"), ('description', "this is a description"))


Have a test where "description" is not part of the string?

damianam · 2018-11-23T16:39:39Z

test/framework/easyconfig.py

+
+        self.assertEqual(parse_param_value("this is a description"), ('description', "this is a description"))
+
+        self.assertEqual(parse_param_value('1.2.3'), ('version', '1.2.3'))


Test different versions? 1.0, 22.0, 1.2a....

damianam · 2018-11-23T16:41:59Z

test/framework/easyconfig.py

+
+        # names of source tarballs are recognized
+        for ext in ['tar.gz', 'gz', 'tar.bz2', 'bz2', 'zip']:
+            self.assertEqual(parse_param_value('toy-0.0.' + ext), ('sources', 'toy-0.0.' + ext))


What if the file is toy_0.0.zip or toy0.0.tar?

boegel added 16 commits November 16, 2018 21:13

add support for 'eb --new' to simplify creating new easyconfig files …

d85c6ba

…from scratch

make -E the short option for --experimental

7193739

add edit_file function + dedicated test for it

ed2e394

add support for --cat, --copy, --edit that collaborate with --new/-n

712303c

add support to search_easyconfigs function to return results rather t…

84c513d

…han print them

fix default value for --copy to '.'

ecd61f0

replace --editor with --editor-command-template, and enhance it to de…

b832b74

…tect whether changes were made

make --search aware of --cat, --copy and --edit

6be0151

add test for --new combined with --cat, --copy, --edit + smarter hand…

7801f47

…ling of argument passed to --copy

rename --cat to --show, move tests for --new to options.py test modul…

398732b

…e (where they belong)

ensure that --copy doesn't blindly overwrite an existing easyconfig f…

7509291

…ile + leverage build_option in main.py to clean up code a bit

add --search-action-limit option to control how many search hits are …

c3a3da2

…allowed to perform search action

test interaction between --search and --copy/--edit/--show

755adda

flesh out separate function for parsing values as easyconfig paramete…

e9be0bf

…r values, also handle sanity_check_paths

take into account empty values in parse_param_value for tuple/list/di…

6b35735

…ct values

add tests for sanity_check_paths value in --new

498e6c1

boegel added the enhancement label Nov 18, 2018

boegel added this to the 3.8.0 milestone Nov 18, 2018

houndci-bot reviewed Nov 18, 2018

View reviewed changes

easybuild/tools/robot.py Show resolved Hide resolved

boegel mentioned this pull request Nov 19, 2018

support for injecting additional toolchainopts (WIP) #2368

Open

damianam reviewed Nov 20, 2018

View reviewed changes

boegel mentioned this pull request Nov 20, 2018

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

Closed

boegel added 5 commits November 22, 2018 09:18

add logging to parse_param_value, add more tests + minor fixes/tweaks

a110f41

refactor to use variables for different separators in parse_param_value

9677ef1

add test for 'http' arguments to --new

4c17fe6

Merge branch 'develop' into eb_new

6a5b4a9

require --experimental for --new

142155d

boegel mentioned this pull request Nov 22, 2018

add EasyConfig.filename() method, and use it in easyblock.py ocaisa/easybuild-framework#39

Merged

damianam requested changes Nov 23, 2018

View reviewed changes

boegel modified the milestones: 3.8.0, 3.9.0 Dec 6, 2018

boegel modified the milestones: next release (3.9.1), 3.x May 14, 2019

boegel mentioned this pull request Dec 23, 2019

add support for --copy-ec #3142

Merged

boegel modified the milestones: 3.x, 4.x Feb 20, 2020

migueldiascosta mentioned this pull request Dec 15, 2020

Coding style should define variable order in the easyconfig easybuilders/easybuild-easyconfigs#216

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for --new, --copy, --edit and --show #2665

add support for --new, --copy, --edit and --show #2665

boegel commented Nov 18, 2018

damianam left a comment

damianam Nov 19, 2018

boegel Nov 21, 2018

damianam Nov 23, 2018

damianam Nov 20, 2018

damianam Nov 20, 2018

boegel Nov 21, 2018

damianam Nov 20, 2018

boegel Nov 21, 2018

damianam Nov 20, 2018

boegel Nov 21, 2018

boegel commented Nov 21, 2018

boegel commented Nov 22, 2018

damianam Nov 23, 2018

damianam Nov 23, 2018

damianam Nov 23, 2018

damianam Nov 23, 2018

		@@ -173,6 +173,73 @@ def run_contrib_style_checks(ecs, check_contrib, check_style):
		return check_contrib or check_style


		def handle_cat_copy_edit(filepaths, target=None, copy=None):


		self.assertEqual(parse_param_value("this is a description"), ('description', "this is a description"))

		self.assertEqual(parse_param_value('1.2.3'), ('version', '1.2.3'))

add support for --new, --copy, --edit and --show #2665

Are you sure you want to change the base?

add support for --new, --copy, --edit and --show #2665

Conversation

boegel commented Nov 18, 2018

damianam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boegel commented Nov 21, 2018

boegel commented Nov 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment