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.

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

ensure separate_build_dir is set to False for ITK and elastix components in SimpleElastix easyconfig #10300

Merged

Conversation

boegel
Copy link
Member

@boegel boegel commented Apr 1, 2020

(created using eb --new-pr)

fix for easyconfigs added in #10216, required because of changes in easybuilders/easybuild-easyblocks#1933

@boegel boegel force-pushed the 20200401141553_new_pr_SimpleElastix110 branch from 6633142 to 0c6a332 Compare April 1, 2020 12:18
@boegel boegel changed the title ensure separate_build_dir is set to False for ITK and elastix components i nSimpleElastix easyconfig ensure separate_build_dir is set to False for ITK and elastix components in SimpleElastix easyconfig Apr 1, 2020
@boegel boegel added the bug fix label Apr 1, 2020
@boegel boegel added this to the next release (4.2.0) milestone Apr 1, 2020
@boegel
Copy link
Member Author

boegel commented Apr 1, 2020

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node3114.skitty.os - Linux centos linux 7.7.1908, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/c3eeac60da345edcd6ceaeb1748fa53a for a full test report.

@boegel
Copy link
Member Author

boegel commented Apr 1, 2020

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node2711.swalot.os - Linux centos linux 7.7.1908, Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/749d2dc4cd3434efa1785e873b8ad648 for a full test report.

@@ -74,6 +74,9 @@ components = [
'source_urls': ['https://github.com/InsightSoftwareConsortium/ITK/archive/'],
'checksums': ['c6b3c33ecc73104c906e0e1a1bfaa41a09af24bf53a4ec5e5c265d7e82bdf69f'],
'srcdir': '%%(builddir)s/ITK-%s' % local_itk_ver,
# we need to take matters into our own hands here to ensure we have a unique build directory per component;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if it wouldn't be better if this is done in the CMakeMake EB. Currently the build folders are %(start_dir)s/easybuild_obj for these components, aren't they? How about instead if using %(builddir)s/easybuild_obj we use %(builddir)s/easybuild_obj_%(name)s or so?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, that's also an option I guess, yeah...

But let's make that a separate PR, I don't want to block easybuilders/easybuild-easyblocks#1933 over this.

@boegel
Copy link
Member Author

boegel commented Apr 1, 2020

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
generoso-3 - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2660 v4 @ 2.00GHz, Python 3.6.8
See https://gist.github.com/ddc9772adc7378ca6821c8fb7c20fa71 for a full test report.

@verdurin
Copy link
Member

verdurin commented Apr 1, 2020

Test report by @verdurin
SUCCESS
Build succeeded for 5 out of 5 (1 easyconfigs in this PR)
easybuild.novalocal - Linux centos linux 7.7.1908, Intel Xeon Processor (Skylake, IBRS), Python 2.7.5
See https://gist.github.com/d94e5e530408da0104a4329db5731016 for a full test report.

Copy link
Member

@verdurin verdurin left a comment

Choose a reason for hiding this comment

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

Looks fine.

@verdurin
Copy link
Member

verdurin commented Apr 1, 2020

Going in, thanks @boegel!

@verdurin verdurin merged commit 2094f9a into easybuilders:develop Apr 1, 2020
@boegel boegel deleted the 20200401141553_new_pr_SimpleElastix110 branch April 1, 2020 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants