Switch to defining _XOPEN_SOURCE=600 for htscodecs tests

[daviesrob]

Required for gettimeofday() on FreeBSD. This needs to be set on the command line as htscodecs tests currently don't include config.h.

Fixes #1606

This is a minimal fix. A more complete one will require some changes to htscodecs.

[jkbonfield]

The lines with -D_POSIX_C_SOURCE in appeared in 878c71bf0, but there is no indication in the commit message as to why it's necessary. Certainly on FreeBSD just deleting the definition is sufficient to make it compile without warnings.

While _XOPEN_SOURCE=600 doesn't harm, I'm curious why either of these are required. We ought to get it documented somewhere so we know what system we need to tiptoe around.

[daviesrob]

It's needed to get getopt() if building in strict-c99 mode (i.e. -std=c99 -pedantic). Example:

gcc -Wall -g -O3 -std=c99 -pedantic -Wformat=2 -fvisibility=hidden -Werror  -I. -I/root/libdeflate -Ihtscodecs -c -o htscodecs/tests/rANS_static_test.o htscodecs/tests/rANS_static_test.c
htscodecs/tests/rANS_static_test.c: In function 'main':
htscodecs/tests/rANS_static_test.c:110:19: error: implicit declaration of function 'getopt' [-Werror=implicit-function-declaration]
  110 |     while ((opt = getopt(argc, argv, "o:dtr")) != -1) {
      |                   ^~~~~~
cc1: all warnings being treated as errors

[jkbonfield]

Arguably you could say if the user is restricting the C specification to a specific hard-line version with extra pedantry, then they also need to adjust C flags. Doing one without the other is just a problem of their own making, although to be fair we don't document the specific standards we require.

Also, why ios that only needed for htscodecs? We use getopt in other places too. Eg:

$ gcc -Wall -g -O2 -std=c99 -pedantic -fvisibility=hidden  -I.  -I/nfs/users/nfs_j/jkb/ftp/compression/libdeflate -I/nfs/users/nfs_j/jkb/scratch/src/zstd/lib -c -o bgzip.o bgzip.c
bgzip.c: In function 'main':
bgzip.c:230:37: warning: implicit declaration of function 'fileno'; did you mean 'fopen'? [-Wimplicit-function-declaration]
         else if (!pstdout && isatty(fileno((FILE *)stdout)) )
                                     ^~~~~~
                                     fopen
bgzip.c:437:24: warning: implicit declaration of function 'strdup'; did you mean 'strcmp'? [-Wimplicit-function-declaration]
                 name = strdup(argv[optind]);
                        ^~~~~~
                        strcmp
bgzip.c:437:22: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
                 name = strdup(argv[optind]);

Adding -D_XOPEN_SOURCE=600 to the flags cures that too. So what's true for one bit of code should also apply to the other, no? This was also my logic behind this comment: https://github.com/samtools/htslib/blob/develop/Makefile#L37

If we wish to accept a build where the user is being overly restrictive demanding C99 and only C99, without permitting XOPEN functions, and we wish to build without warnings, then we would need to amend the Makefile so it's globally defined. This would be preferable to adding it only for htscodecs files. What am I missing here?

[daviesrob]

I think the difference is that the htscodecs test sources don't include config.h, while bgzip.c does. So if you do ./configure CFLAGS='-std=c99 -pedantic' _XOPEN_SOURCE gets set by the PTHREAD_MUTEX_RECURSIVE test (which is a bit obscure, but it does the trick) which fixes up htslib, but the necessary value doesn't get to the htscodecs test code.

There's no similar test on the htscodecs side so you don't see the problem there.

It is arguable that the htscodecs test .c files should be updated to include config.h, and the configure tests for both packages should be improved to ensure the correct feature test macros are set in config.h. This was a quick fix for the issue, but it may be better to head over to htscodecs and sort it out properly...