Add package for libbsd, add variant to expat for libbsd#4945
Merged
adamjstewart merged 3 commits intospack:developfrom Aug 3, 2017
Merged
Add package for libbsd, add variant to expat for libbsd#4945adamjstewart merged 3 commits intospack:developfrom
adamjstewart merged 3 commits intospack:developfrom
Conversation
The recent expat release requires a high quality source of randomness. CentOS 7 does not seem to have one, but one is available in libbsd. This commit adds a package for libbsd and adds a variant to expat to use it (defaults to False).
Member
|
Hmm, CentOS 7 isn't exactly an old kernel, and many HPC centers will be using it in the future. I think it should be enabled by default. In general, variants that add dependencies should default to False, but in this case, the default doesn't build. The only thing that needs to change is that this flag (and dependency) don't apply to older releases. We could add a |
Make the libbsd variant default to true. Conflict if you're asking for libbsd and an older version of expat. This means that in order to install an older version of expat you'll need to specify `~libbsd`.
Contributor
Author
|
Here's an example of handling it with a conflict. What I'd really like to be able to do is change the variant's default value based on the version. Is that possible? |
adamjstewart
reviewed
Aug 1, 2017
| homepage = "http://expat.sourceforge.net/" | ||
| url = "https://sourceforge.net/projects/expat/files/expat/2.2.2/expat-2.2.2.tar.bz2" | ||
|
|
||
| # Version 2.2.2 introduced a requirement for a high quality |
Member
There was a problem hiding this comment.
It looks like it was actually 2.2.1 that introduced this flag.
Contributor
Author
There was a problem hiding this comment.
I think that there were some get_random knobs in 2.2.1 (here, if I've navigated the release right) but the code that makes the build fail came in for 2.2.2 (here, ditto) and here(changelog).
Member
Not at the moment. |
This was referenced Aug 3, 2017
Contributor
Author
|
@adamjstewart -- is this good to go? |
Member
|
It still worries me that |
Get rid of the conflicts and use better constraints/tests in the `depends_on` and the `configure_args` bits.
Contributor
Author
|
Halllmark needs to market a line of greeting cards with the theme "Thanks for making me a better contributor!" I'm failing to think what the graphics would look like, but.... I think that the most recent commit does what you're suggesting: |
adamjstewart
approved these changes
Aug 3, 2017
Contributor
|
I just ran into this issue too. Looking forward to the changes! Thanks! Jim G |
Contributor
|
@hartzell @adamjstewart I'm seeing a failure in the libbsd build. expat builds ok now. |
Member
|
@jgalarowicz All that tells me is that |
Member
|
@hartzell I'm also seeing build errors on macOS: |
Contributor
|
SPACK-BUILD.out: CONFIG.LOG [jeg@localhost libbsd-0.8.6]$ |
Member
|
@jgalarowicz Run |
Contributor
|
@adamjstewart @hartzell |
Contributor
Author
|
Nuts. @jgalarowicz -- Can you build But, Mac OS looks like it's not simple. See here, and here and here. |
Member
|
I'm able to build |
Contributor
Author
|
How about if we make libbsd conflict with mac os and further conditionalize the expat dependency on the platform (assuming there are sufficient hooks to make that happen)? |
Member
|
That works for me but not for @jgalarowicz who is on Fedora 19. |
Contributor
Author
|
@jgalarowicz -- What compiler are you using? The build is dying at the last line of this bit of libbsd's #ifndef __has_include
#define __has_include 1
#endif
#ifndef __has_include_next
#define __has_include_next 1
#endif
#ifdef LIBBSD_OVERLAY
/*
* Some libc implementations do not have a <sys/cdefs.h>, in particular
* musl, try to handle this gracefully.
*/
#if __has_include_next(<sys/cdefs.h>)
A bit of poking around with |
Contributor
Author
|
Fedora 19 seems to be shipped with [email protected]. |
Contributor
|
@hartzell Yes, 4.8.3. I'm planning on upgrading soon to Fedora 26. |
Contributor
|
@hartzell @adamjstewart I can build expat 2.2.0 to get around this temporarily, if this is an older compiler issue. |
Contributor
Author
|
I didn't think carefully about my |
Contributor
Author
|
[edited to fix the fenced code block] It turns out that I can not build it on CentOS 7 with the system's Seems to have the same problem: |
Contributor
Author
|
Sadly, the libbsd Bugzilla has |
Contributor
Author
TL;DRThis seems to let it build for me: --- include/bsd/sys/cdefs.h.orig 2017-08-04 16:34:56.404995800 -0700
+++ include/bsd/sys/cdefs.h 2017-08-04 16:35:19.345043883 -0700
@@ -25,10 +25,10 @@
*/
#ifndef __has_include
-#define __has_include 1
+#define __has_include(X) 1
#endif
#ifndef __has_include_next
-#define __has_include_next 1
+#define __has_include_next(X) 1
#endif
#ifdef LIBBSD_OVERLAYI'll add something to the package, see if it builds for me, and PR it for discussion. DetailsIn the spirit of showing my work (aka showing where I went off the rails...), here's how I ended up there, starting by isolating the Given a #include "blah.h"and a #ifndef __has_include
#define __has_include 1
#endif
#ifndef __has_include_next
#define __has_include_next 1
#endif
#ifdef LIBBSD_OVERLAY
/*
* Some libc implementations do not have a <sys/cdefs.h>, in particular
* musl, try to handle this gracefully.
*/
#if __has_include_next(<sys/cdefs.h>)
#include_next <sys/cdefs.h>
#endif
#else
#if __has_include(<sys/cdefs.h>)
#include <sys/cdefs.h>
#endif
#endifThe running this command (I started with the failing make command and trimmed until I could trim no more) with CentOS's If I change #ifndef __has_include
#define __has_include 1
#endif
#ifndef __has_include_next
#define __has_include_next 1
#endif
__has_include_nextI get this output: which suggests that the compiler is tripping over a construct that looks like #if 1(<sys/cdefs.h>)
a=1;
#endifrecapitulates the error: |
hartzell
pushed a commit
to hartzell/spack
that referenced
this pull request
Aug 4, 2017
See the discussion in spack#4945 (after the merge) for additional background. libbsd builds with [email protected] on CentOS 7, but not with the system's [email protected]. Others have reported problems with [email protected] on Fedora 19. The problem boils down to the lack of support for the clang extension `__has_include_next`. The immediate symptom seems to be the pre-processor using defining macro like this ``` ``` then then tripping over an expansion of it like this: ``` blah.h:13:23: error: missing binary operator before token "(" ``` This patch changes the macro definition to: ``` ``` which swallows the arguments with which the macro is invoked. The end result is that libbsd builds for me on CentOS 7 using the system compiler.
adamjstewart
pushed a commit
that referenced
this pull request
Aug 5, 2017
* Fix cdefs macro to be compatible with gcc 4.8.x See the discussion in #4945 (after the merge) for additional background. libbsd builds with [email protected] on CentOS 7, but not with the system's [email protected]. Others have reported problems with [email protected] on Fedora 19. The problem boils down to the lack of support for the clang extension `__has_include_next`. The immediate symptom seems to be the pre-processor using defining macro like this ``` ``` then then tripping over an expansion of it like this: ``` blah.h:13:23: error: missing binary operator before token "(" ``` This patch changes the macro definition to: ``` ``` which swallows the arguments with which the macro is invoked. The end result is that libbsd builds for me on CentOS 7 using the system compiler. * Apply this patch for any compiler version before 5 This includes subversions of 4, like 4.8.5.
citibeth
pushed a commit
to citibeth/spack
that referenced
this pull request
May 15, 2018
…rsion of libexpat did. Also update for Expat's move to GitHub.
citibeth
pushed a commit
to citibeth/spack
that referenced
this pull request
May 15, 2018
…rsion of libexpat did. Also update for Expat's move to GitHub.
adamjstewart
pushed a commit
that referenced
this pull request
May 18, 2018
* PR #4945 did not make this work on SuSE 11. Adding the latest version of libexpat did. Also update for Expat's move to GitHub. * Update package.py * Update package.py Move to url_for_version()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The recent expat release requires a high quality source of randomness.
CentOS 7 does not seem to have one, but one is available in libbsd.
This commit adds a package for libbsd and adds a variant to expat to
use it (defaults to False).
I'd appreciate feedback on the use of a variant for this (as opposed to just making it the default, perhaps) and whether the variant should default to True or False.
Fixes #4943