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

Jump to bottom

make zlib a real dependency rather than a build dep in recent binutils easyconfigs (>= 2.28) #8340

Merged
merged 5 commits into from
May 17, 2019
Merged

make zlib a real dependency rather than a build dep in recent binutils easyconfigs (>= 2.28) #8340

merged 5 commits into from
May 17, 2019

Conversation

geimer
Copy link
Contributor

@geimer geimer commented May 16, 2019

Also add checksums where missing and rearrange parameters to make easyconfigs more consistent.

Addresses easybuilders/easybuild-easyblocks#1350, PR for modified easyblock will follow soon. However, it should be safe to already merge this PR even without the easyblock.

@geimer
Make zlib a real dependency rather than a build dep
65684b3 
Also add checksums where missing and rearrange parameters to make
easyconfigs more consistent
@boegel
Copy link
Member

boegel commented May 16, 2019

@geimer Promoting zlib to a runtime dep of binutils makes sense as discussed (cfr. https://github.com/easybuilders/easybuild/wiki/Conference-call-notes-20190515#outlook-to-easybuild-v391), but for the older binutils versions it's pretty much going to be impossible to make this change without introducing version conflicts.

For the recent binutils versions/toolchains you shouldn't have any problems since we've learned the hard way to stick to a single version of a dependency per toolchain generation (and zlib hasn't had any new releases for a while, which helps too).

In addition, we'll probably need to stick to the static embedding for the binutils built with dummy (which is only used to build GCCcore & it's build dependencies), to avoid this situation:

$ eb --check-conflicts --from-pr 8340
...
Conflict found for dependencies of binutils-2.32-GCCcore-8.3.0: zlib-1.2.11 vs zlib-1.2.11-GCCcore-8.3.0
	zlib-1.2.11 as dep of: ..., binutils-2.32, binutils-2.32-GCCcore-8.3.0, ...
	zlib-1.2.11-GCCcore-8.3.0 as dep of: binutils-2.32-GCCcore-8.3.0

@boegel boegel added this to the next release (3.9.1) milestone May 16, 2019
@geimer
Copy link
Contributor Author

geimer commented May 16, 2019

In addition, we'll probably need to stick to the static embedding for the binutils built with dummy (which is only used to build GCCcore & it's build dependencies), to avoid this situation:

$ eb --check-conflicts --from-pr 8340
...
Conflict found for dependencies of binutils-2.32-GCCcore-8.3.0: zlib-1.2.11 vs zlib-1.2.11-GCCcore-8.3.0
	zlib-1.2.11 as dep of: ..., binutils-2.32, binutils-2.32-GCCcore-8.3.0, ...
	zlib-1.2.11-GCCcore-8.3.0 as dep of: binutils-2.32-GCCcore-8.3.0

So why is this working then? I have been able to successfully build both...

@geimer
Copy link
Contributor Author

geimer commented May 16, 2019

Wait... There is actually no conflict. binutils-2.32 built with dummy is a builddependency of things on the GCCcore level, and thus never loaded when a GCCcore package is active. That is, when checking for conflicts, one has to distinguish between dependencies and builddependencies. Does that make sense?

@boegel
Copy link
Member

boegel commented May 16, 2019

OK, that's my mistake, I wasn't using --check-conflicts correctly here, sorry for the confusion...
It can also be used to check whether two (or more) easyconfigs have any conflicts between them.
And when using --from-pr, it by default pulls in all easyconfigs touched in the PR (unless you list one of more easyconfigs in the command), so using eb --check-conflicts --from-pr 8340 doesn't make any sense.

--check-conflicts does correctly deal with build dependencies as you mention, that's not the issue here.

Your changes are causing problems related to some old binutils versions, because of idiotic/weird stuff we did in older easyconfigs, and the bootstrapping we do for binutils.

For example (with your branch used for this PR checked out):

$ eb --check-conflicts SAMtools-1.3.1-intel-2016b.eb
== temporary log file in case of crash /tmp/eb-ZxZtkl/easybuild-aw5IWo.log
Conflict found for dependencies of SAMtools-1.3.1-intel-2016b: zlib-1.2.8-GCCcore-5.4.0 vs zlib-1.2.8-intel-2016b
	zlib-1.2.8-GCCcore-5.4.0 as dep of: SAMtools-1.3.1-intel-2016b, binutils-2.26-GCCcore-5.4.0, icc-2016.3.210-GCC-5.4.0-2.26, iccifort-2016.3.210-GCC-5.4.0-2.26, ifort-2016.3.210-GCC-5.4.0-2.26, iimpi-2016b, imkl-11.3.3.210-iimpi-2016b, impi-5.1.3.181-iccifort-2016.3.210-GCC-5.4.0-2.26, intel-2016b, ncurses-6.0-intel-2016b, zlib-1.2.8-intel-2016b
	zlib-1.2.8-intel-2016b as dep of: SAMtools-1.3.1-intel-2016b
Conflict found for dependencies of binutils-2.26-GCCcore-5.4.0: zlib-1.2.8 vs zlib-1.2.8-GCCcore-5.4.0
	zlib-1.2.8 as dep of: Bison-3.0.4-GCCcore-5.4.0, GCCcore-5.4.0, M4-1.4.17-GCCcore-5.4.0, binutils-2.26, binutils-2.26-GCCcore-5.4.0, flex-2.6.0-GCCcore-5.4.0, zlib-1.2.8-GCCcore-5.4.0
	zlib-1.2.8-GCCcore-5.4.0 as dep of: SAMtools-1.3.1-intel-2016b, binutils-2.26-GCCcore-5.4.0, icc-2016.3.210-GCC-5.4.0-2.26, iccifort-2016.3.210-GCC-5.4.0-2.26, ifort-2016.3.210-GCC-5.4.0-2.26, iimpi-2016b, imkl-11.3.3.210-iimpi-2016b, impi-5.1.3.181-iccifort-2016.3.210-GCC-5.4.0-2.26, intel-2016b, ncurses-6.0-intel-2016b, zlib-1.2.8-intel-2016b
Conflict between (dependencies of) easyconfigs: zlib-1.2.8-GCCcore-5.4.0 vs zlib-1.2.8-intel-2016b
	zlib-1.2.8-GCCcore-5.4.0 as dep of: SAMtools-1.3.1-intel-2016b, binutils-2.26-GCCcore-5.4.0, icc-2016.3.210-GCC-5.4.0-2.26, iccifort-2016.3.210-GCC-5.4.0-2.26, ifort-2016.3.210-GCC-5.4.0-2.26, iimpi-2016b, imkl-11.3.3.210-iimpi-2016b, impi-5.1.3.181-iccifort-2016.3.210-GCC-5.4.0-2.26, intel-2016b, ncurses-6.0-intel-2016b, zlib-1.2.8-intel-2016b
	zlib-1.2.8-intel-2016b as dep of: SAMtools-1.3.1-intel-2016b
ERROR: One or more conflicts detected!

For SAMtools, which has a direct dependency on zlib, two zlib modules would have to be loaded at the same time (the other one is the one you added as a runtime dep for binutils).

There's also a problem for binutils-2.26-GCCcore-5.4.0.eb.
Since it requires Core/binutils/2.26 at installation time, which now depends on Core/zlib/1.2.8, as well as Compiler/GCCcore/5.4.0/zlib/1.2.8, two different zlib modules would need to be loaded at the same time.
By default Lmod allows this by simply swapping out the zlib/1.2.8 that corresponds with Core/zlib/1.2.8 when the one that corresponds with Compiler/GCCcore/5.4.0/zlib/1.2.8 is being loaded, but i) we don't allow conflicting versions in the central repo, and ii) some sites (incl. HPC-UGent) have intentionally Lmod configured in such a way that it doesn't do the auto-swap, it fails hard.

So, both problems stand imho...

@boegel boegel changed the title Make zlib a real dependency rather than a build dep make zlib a real dependency rather than a build dep in recent binutils easyconfigs (>= 2.28) May 16, 2019
@boegel boegel self-assigned this May 16, 2019
@boegel
Copy link
Member

boegel commented May 16, 2019

Looks good, but this should hold until the accompanying PR for the easyblock is there too (@geimer is working on that)...

@geimer geimer mentioned this pull request May 17, 2019
Merged
@boegel
Copy link
Member

boegel commented May 17, 2019

Test report by @boegel
FAILED
Build succeeded for 11 out of 12 (11 easyconfigs in this PR)
node2578.golett.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/38bc86696d49898abd61433b63689188 for a full test report.

@boegel
Copy link
Member

boegel commented May 17, 2019

Test report by @boegel
FAILED
Build succeeded for 10 out of 11 (11 easyconfigs in this PR)
node2065.delcatty.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/34d578704d4df5e7176e866d4eb27f52 for a full test report.

@geimer
Copy link
Contributor Author

geimer commented May 17, 2019

@boegel I believe the build error is unrelated to the zlib stuff. It looks like it is more related to rpath-ing or so, which hasn't been touched...

@boegel
Copy link
Member

boegel commented May 17, 2019

@geimer Indeed, binutils-2.29-GCCcore-system.eb fails on our systems even without the changes done here, I'll submit a new test report without that one included (but the change should be done anyway).
It's likely an issue with using the system GCC (4.8.5) building binutils 2.29...

@boegel
Copy link
Member

boegel commented May 17, 2019

Test report by @boegel
SUCCESS
Build succeeded for 16 out of 16 (16 easyconfigs in this PR)
node2578.golett.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/617c10ac5cb46fc03c0f376a4370ff36 for a full test report.

@boegel
Copy link
Member

boegel commented May 17, 2019

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

@boegel
Copy link
Member

boegel commented May 17, 2019

Test report by @boegel
SUCCESS
Build succeeded for 16 out of 16 (16 easyconfigs in this PR)
node2001.delcatty.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/daac3e169efc93226b912faa40f282d7 for a full test report.

@boegel boegel mentioned this pull request May 17, 2019
Closed
@boegel
Copy link
Member

boegel commented May 17, 2019

Going in, thanks @geimer!

@boegel boegel merged commit 652b95e into easybuilders:develop May 17, 2019
@geimer geimer deleted the binutils_without_embedded_zlib branch May 18, 2019 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boegel boegel boegel approved these changes

Assignees

@boegel boegel

Labels
bug fix change
Projects
None yet
Milestone
3.9.1
Development

Successfully merging this pull request may close these issues.
2 participants
@geimer @boegel