Skip to content
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

fix support for specifying multiple PRs to --from-pr #3707

Merged
merged 2 commits into from
May 27, 2021
Merged

fix support for specifying multiple PRs to --from-pr #3707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61801b4
fetch easyconfigs to unique dir per PR
migueldiascosta May 27, 2021
894a56f
fix support for specifying multiple PRs to --from-pr
boegel May 27, 2021
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
3 changes: 2 additions & 1 deletion easybuild/framework/easyconfig/tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,9 @@ def det_easyconfig_paths(orig_paths):

if from_pr_list is not None:
pr_files = []
tmpdir = tempfile.mkdtemp()
for pr in from_pr_list:
pr_files.extend(fetch_easyconfigs_from_pr(pr))
pr_files.extend(fetch_easyconfigs_from_pr(pr, path=os.path.join(tmpdir, 'files_pr%s' % pr)))
Copy link
Member

Choose a reason for hiding this comment

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

Just changing this here is not enough, and even incorrect (that's why you found yourself having to add the OpenBLAS easyconfig to the tests to make things work).

There's a hidden shared path between alt_easyconfig_paths (which was tweaked in #3605 to take into account multiple PR #s, so you get a path like /.../files_pr123_456 when using --from-pr 123,456), which then gets set as the value for the pr_path build option. This is then used in fetch_files_from_pr to determine the default value for the easyconfigs being pulled from a PR (which you're overriding here).

So if we spread easyconfigs from PRs over multiple paths, we have to take that into account in alt_easyconfig_paths too, or we ensure that all those paths get injected into the robot search path correctly (which is done via the result produced by det_robot_path in set_up_configuration)...


if ec_files:
# replace paths for specified easyconfigs that are touched in PR
Expand Down
41 changes: 41 additions & 0 deletions test/framework/easyconfigs/test_ecs/o/OpenBLAS/OpenBLAS-0.3.1-GCC-7.3.0-2.30.eb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name = 'OpenBLAS'
version = '0.3.1'

homepage = 'http://xianyi.github.com/OpenBLAS/'
description = "OpenBLAS is an optimized BLAS library based on GotoBLAS2 1.13 BSD version."

toolchain = {'name': 'GCC', 'version': '7.3.0-2.30'}
# need to build with -fno-tree-vectorize due to asm constraint bugs in OpenBLAS<0.3.6
# cfr. https://github.com/easybuilders/easybuild-easyconfigs/issues/7180
toolchainopts = {'vectorize': False}

source_urls = [
# order matters, trying to download the large.tgz/timing.tgz LAPACK tarballs from GitHub causes trouble
'http://www.netlib.org/lapack/timing/',
'https://github.com/xianyi/OpenBLAS/archive/',
]
sources = ['v%(version)s.tar.gz']
patches = [
('large.tgz', '.'),
('timing.tgz', '.'),
# this patch together with buildopts = 'USE_SIMPLE_THREADED_LEVEL3=1'
# is a workaround for the matrix multiplication issues
# https://lists.ugent.be/wws/arc/easybuild/2019-05/msg00006.html
# https://github.com/eylenth/Openblas_matrix_issue
'OpenBLAS-%(version)s_disable-special-handling-of-OpenMP-thread-memory-allocation.patch',
]
checksums = [
'1f5e956f35f3acdd3c74516e955d797a320c2e0135e31d838cbdb3ea94d0eb33', # v0.3.1.tar.gz
'f328d88b7fa97722f271d7d0cfea1c220e0f8e5ed5ff01d8ef1eb51d6f4243a1', # large.tgz
'999c65f8ea8bd4eac7f1c7f3463d4946917afd20a997807300fe35d70122f3af', # timing.tgz
# OpenBLAS-0.3.1_disable-special-handling-of-OpenMP-thread-memory-allocation.patch
'c85de436d6fff5d9cec1e24127ea9921551cdee373319dbade922d5cd3facd6a',
]

# added as workaround for the matrices multiplication issue. see the patches above
buildopts = 'USE_SIMPLE_THREADED_LEVEL3=1'

# extensive testing can be enabled by uncommenting the line below
# runtest = 'PATH=.:$PATH lapack-timing'

moduleclass = 'numlib'