From 37b0d8657bf78637fc5a8e3b5ebe0133f92c5f23 Mon Sep 17 00:00:00 2001 From: Jasper Grimm Date: Thu, 17 Mar 2022 15:32:12 +0000 Subject: [PATCH 1/5] ensure --review-pr can find dependencies included in PR --- easybuild/framework/easyconfig/tools.py | 7 ++++++- easybuild/tools/options.py | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index be6ff5fe01..3aaed01c6f 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -308,7 +308,7 @@ def get_paths_for(subdir=EASYCONFIGS_PKG_SUBDIR, robot_path=None): return paths -def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None): +def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None, review_pr=None): """Obtain alternative paths for easyconfig files.""" # paths where tweaked easyconfigs will be placed, easyconfigs listed on the command line take priority and will be @@ -322,6 +322,11 @@ def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None): # paths where files touched in PRs will be downloaded to, # which are picked up via 'pr_paths' build option in fetch_files_from_pr pr_paths = None + if from_prs and review_pr: + from_prs.append(review_pr) if review_pr not in from_prs + elif review_pr: + from_prs = [review_pr] + if from_prs: pr_paths = [os.path.join(tmpdir, 'files_pr%s' % pr) for pr in from_prs] diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 6d123508a2..1fd18ceedc 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1547,10 +1547,16 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): except ValueError: raise EasyBuildError("Argument to --from-pr must be a comma separated list of PR #s.") + try: + review_pr = int(eb_go.options.review_pr) + except ValueError: + raise EasyBuildError("Argument to --review-pr must be an integer PR #.") + # determine robot path # --try-X, --dep-graph, --search use robot path for searching, so enable it with path of installed easyconfigs tweaked_ecs = try_to_generate and build_specs - tweaked_ecs_paths, pr_paths = alt_easyconfig_paths(tmpdir, tweaked_ecs=tweaked_ecs, from_prs=from_prs) + tweaked_ecs_paths, pr_paths = alt_easyconfig_paths(tmpdir, tweaked_ecs=tweaked_ecs, from_prs=from_prs, + review_pr=review_pr) auto_robot = try_to_generate or options.check_conflicts or options.dep_graph or search_query robot_path = det_robot_path(options.robot_paths, tweaked_ecs_paths, pr_paths, auto_robot=auto_robot) log.debug("Full robot path: %s", robot_path) From b87c8a3761bb89eeeead706a6013ff806902cc5f Mon Sep 17 00:00:00 2001 From: Jasper <65227842+jfgrimm@users.noreply.github.com> Date: Thu, 17 Mar 2022 15:46:02 +0000 Subject: [PATCH 2/5] fix syntax --- easybuild/framework/easyconfig/tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 3aaed01c6f..66d181c1d9 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -323,7 +323,7 @@ def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None, review_pr=Non # which are picked up via 'pr_paths' build option in fetch_files_from_pr pr_paths = None if from_prs and review_pr: - from_prs.append(review_pr) if review_pr not in from_prs + from_prs.append(review_pr) if review_pr not in from_prs else from_prs elif review_pr: from_prs = [review_pr] From fa629682fe9dcef98d920c50c2b0873b1df0526a Mon Sep 17 00:00:00 2001 From: Jasper <65227842+jfgrimm@users.noreply.github.com> Date: Mon, 21 Mar 2022 11:06:32 +0000 Subject: [PATCH 3/5] guard review_pr assignment to avoid attempting to convert None to int --- easybuild/tools/options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 1fd18ceedc..126dd701cc 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1548,7 +1548,7 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False): raise EasyBuildError("Argument to --from-pr must be a comma separated list of PR #s.") try: - review_pr = int(eb_go.options.review_pr) + review_pr = (lambda x: int(x) if x else None)(eb_go.options.review_pr) except ValueError: raise EasyBuildError("Argument to --review-pr must be an integer PR #.") From 3bbf975051361eb4fd9a339c357f9b7ed6ac8eec Mon Sep 17 00:00:00 2001 From: Jasper Grimm Date: Tue, 29 Mar 2022 09:35:48 +0100 Subject: [PATCH 4/5] avoid appending directly to from_prs, tidy if/else logic --- easybuild/framework/easyconfig/tools.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 66d181c1d9..6796c483ec 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -321,14 +321,14 @@ def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None, review_pr=Non # paths where files touched in PRs will be downloaded to, # which are picked up via 'pr_paths' build option in fetch_files_from_pr - pr_paths = None - if from_prs and review_pr: - from_prs.append(review_pr) if review_pr not in from_prs else from_prs - elif review_pr: - from_prs = [review_pr] - + pr_paths = [] if from_prs: - pr_paths = [os.path.join(tmpdir, 'files_pr%s' % pr) for pr in from_prs] + pr_paths = from_prs + if review_pr and review_pr not in pr_paths: + pr_paths.append(review_pr) + + if pr_paths: + pr_paths = [os.path.join(tmpdir, 'files_pr%s' % pr) for pr in pr_paths] return tweaked_ecs_paths, pr_paths From 77af56f4d5ef143ae08b0275e8b68b54349ad7b3 Mon Sep 17 00:00:00 2001 From: Jasper <65227842+jfgrimm@users.noreply.github.com> Date: Tue, 29 Mar 2022 10:32:20 +0100 Subject: [PATCH 5/5] ensure pr_paths is a value copy --- easybuild/framework/easyconfig/tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 6796c483ec..4fb3de77ac 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -323,7 +323,7 @@ def alt_easyconfig_paths(tmpdir, tweaked_ecs=False, from_prs=None, review_pr=Non # which are picked up via 'pr_paths' build option in fetch_files_from_pr pr_paths = [] if from_prs: - pr_paths = from_prs + pr_paths = from_prs[:] if review_pr and review_pr not in pr_paths: pr_paths.append(review_pr)