Skip to content

When not using configure, define _XOPEN_SOURCE#1246

Merged
daviesrob merged 1 commit intosamtools:developfrom
jmarshall:noconfigure-xopen
Mar 3, 2021
Merged

When not using configure, define _XOPEN_SOURCE#1246
daviesrob merged 1 commit intosamtools:developfrom
jmarshall:noconfigure-xopen

Conversation

@jmarshall
Copy link
Copy Markdown
Member

@jmarshall jmarshall commented Mar 3, 2021

Non-configure -std=c99 builds (as opposed to -std=gnu99 builds), e.g., make CC='gcc -std=c99', fail as glibc (and maybe others) suppresses non-Standard-C functions etc in standard headers in this mode:

$ make distclean
$ make CC='gcc -std=c99'  # On Linux i.e. glibc
…
gcc -std=c99 -g -Wall -O2 -fvisibility=hidden -I.  -c -o kfunc.o kfunc.c
kfunc.c: In function ‘kf_erfc’:
kfunc.c:76:16: error: ‘M_SQRT2’ undeclared (first use in this function)
  z = fabs(x) * M_SQRT2;
                ^~~~~~~

…This is the first error of many — e.g., missing strdup() declarations, etc

In particular: Rhtslib, Rsamtools, and other R-based builds do not use configure and don't supply their own config.h, and may specify -std=c99. See for example rwdavies/STITCH#45, where HTSlib is built without running configure as part of an R build process — and that R build process supplies -std=c99.

This is part of something I've been working on to tidy up the setting of these _POSIX/_XOPEN…SOURCE defines in configure.ac, but this is not the time to be proposing that! This part by itself will prevent some build failures for the R people and others though.

@jmarshall
Copy link
Copy Markdown
Member Author

jmarshall commented Mar 3, 2021 •
edited
Loading

[macosx + clang USE_CONFIG:no CI build fails]
Well, well… so that's why we need 600! At first glance, this appears to be a bug in macOS's headers…

@jmarshall
When not using configure, define _XOPEN_SOURCE
1ef943c 
Non-configure -std=c99 builds (as opposed to -std=gnu99 builds), e.g.,
`make CC='gcc -std=c99'`, previously failed as glibc (and maybe others)
suppresses non-Standard-C functions in standard headers in this mode.
This reactivates them.

In particular: Rhtslib, Rsamtools, and other R-based builds do not use
configure and don't supply their own config.h, and may specify -std=c99.

(500 suffices for glibc, but macOS's headers require 600 to have them
provide declarations for strdup() and snprintf(). This appears to be a
bug related to their expected _C99_SOURCE define, which isn't defined.)
@jmarshall jmarshall force-pushed the noconfigure-xopen branch from 3423a9a to 1ef943c Compare March 3, 2021 09:59
@jkbonfield
Copy link
Copy Markdown
Contributor

jkbonfield commented Mar 3, 2021

[macosx + clang USE_CONFIG:no CI build fails]
Well, well… so that's why we need 600! At first glance, this appears to be a bug in macOS's headers…

Interesting. I see it failed on vsnprintf, which the linux man page claims needs _XOPEN_SOURCE >= 500 || _ISOC99_SOURCE. So it ought to pass on both fronts (although it doesn't appear the compile used -stdc=c99 and I don't know what level of compliance it defaults to).

From what I can tell from searching, macos man pages for vsnprintf mentioned c99 but are silent regarding XOpen.

@jmarshall
Copy link
Copy Markdown
Member Author

jmarshall commented Mar 3, 2021 •
edited
Loading

Mojave's <stdio.h> conditions [v]snprintf on __DARWIN_C_LEVEL, _C99_SOURCE, and/or __cplusplus, and you'd think _C99_SOURCE was supposed to be true in C99 mode. I dunno what macOS clang's default language level is [edited to add: turns out it's C11], but even with clang -std=c99 that one doesn't fire. Nothing in the headers defines _C99_SOURCE and clang doesn't seem to predefine it. So it seems like this vestigial _C99_SOURCE macro is left over from when it got removed from sys/cdefs.h (their equivalent of glibc's features.h) or something.

So setting 600 means it gets activated via __DARWIN_C_LEVEL instead.

I've spent a while looking through #423 and the rest of your old PTHREAD_MUTEX_RECURSIVE configure stuff trying to figure out why we settled on 600 instead of 500, but maybe this is the answer to that one. 😄

@jkbonfield
Copy link
Copy Markdown
Contributor

jkbonfield commented Mar 3, 2021

I can't recall why I chose 600, but I expect it was pragmatically what worked.

@daviesrob
Copy link
Copy Markdown
Member

daviesrob commented Mar 3, 2021

According to the feature_test_macros man page, _XOPEN_SOURCE = 600 is the one that gets all of the C99 stuff.

It certainly helps to be more resistant to random compiler options, thanks.

@daviesrob daviesrob merged commit 0380c7b into samtools:develop Mar 3, 2021
@jmarshall jmarshall deleted the noconfigure-xopen branch March 4, 2021 10:00
@jmarshall
Copy link
Copy Markdown
Member Author

jmarshall commented Mar 4, 2021

That is an interesting point that 600 implies the C99 functions, and I think we've explained the pragmaticness of 600, and thanks for merging.

Still… other things, notably -std=c99, imply the C99 functions too. It'd be an obtuse C implementation that interpreted cc -D_XOPEN_SOURCE=500 -std=c99 as “give me C99 language constructs but take the C99 library functions away“ rather than as a statement about what level of POSIX+SUS the code wants to have available in addition to the C99 basics. And in fact on Mojave _XOPEN_SOURCE=500 does not remove other C99 additions like labs(), lldiv(), the C99 maths functions, or isblank(). So I'll still quixotically file a bug with Apple…

@jmarshall jmarshall mentioned this pull request Apr 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmarshall @jkbonfield @daviesrob