Fix cextern/wcslib build to compile original WCSLIB source in C99 mode - #11376

Tentatively marked this for no changelog entry, as the modification seemed almost trivial, but wow, does -std=c99 break gcc 4.x or 32bit!

I think the problem with c99 is that it will not work with manylinux1 wheels. We could maybe switch to manylinux2010 though it requires a more recent pip (something we discussed a bit in #10873).

Maybe just leave the patch from #11260 as is? That is, close this PR without merging? I do not see any practical benefit from undoing the patch from #11260.

And really, this is the single loop in the entire WCSLIB that does not comply to ANSI C standard. It is strange (to me) that the advice given was to change compiler flags instead of fixing this one loop.

I could not find any build log in https://github.com/astropy/astropy/pull/11376/checks?check_run_id=2061368004 so it seems like the actual build finished, but from the test failures apparently the entire wcslib is broken when compiled with -std-c99. No idea really what is the difference to -std=gnu99 or how it works out in the manylinux wheels...

And really, this is the single loop in the entire WCSLIB that does not comply to ANSI C standard. It is strange (to me) that the advice given was to change compiler flags instead of fixing this one loop.

From Mark Calabretta's reply

Certain C99 constructs and library functions have been used in WCSLIB
for many years, but so many C compilers support them transparently
that no one noticed, or at least didn't comment. I guess the relaxed
placement of variable declarations was a step too far!

Back in 1995 I originally wrote WCSLIB in K&R C. I completed the
transition to ANSI C89 (ISO C90) in 2004-05. As it has been around for
over 20 years now, I considered that it ought to be safe to use at least
the simpler C99 elements, and have started transitioning to it. In the
release notes for 7.3.1 I reported that I had changed all in-code
commenting to C99 style, and since then have been using the more general
placement of variable declarations allowed by C99 in code that I modify.

Currently configure uses the AC_PROG_CC_STDC test macro, which operates
as follows:

If the C compiler cannot compile ISO Standard C (currently C99), try
to add an option to output variable CC to make it work. If the
compiler does not support C99, fall back to supporting ANSI C89
(ISO C90).

I find that gcc (9.3.0) supports C99 code without needing any specific
options. I hope that there aren't still ancient compilers in use out
there that baulk at C99 - we need to move on!

I took it that future releases are likely to use more C99 constructs, so this seemed the cleaner and more future-proof solution. But right now for 7.4 it should not make much difference indeed.

I think the problem with c99 is that it will not work with manylinux1 wheels. We could maybe switch to manylinux2010 though it requires a more recent pip (something we discussed a bit in #10873).

Fwiw, we can move to manylinux2010 now we dropped support for Python 3.6 in 4.2 (according to the now accepted APE 18).

I think the problem with c99 is that it will not work with manylinux1 wheels. We could maybe switch to manylinux2010 though it requires a more recent pip (something we discussed a bit in #10873).

Fwiw, we can move to manylinux2010 now we dropped support for Python 3.6 in 4.2 (according to the now accepted APE 18).

So is manylinux1 still using gcc 4.2 (which appears to be the default compiler on CentOS 5)? As we don't seem to have any CI testing that far back, that should be another motivation to move onwards.

we can move to manylinux2010

Does this mean someone gonna open a PR to patch the Azure pipeline?

If possible to get c99 to work, I would prefer to use that option instead of gnu99. By being able to compile C code without enabling GNU extensions, I think the code will be able to compile on a wider range of platforms/compilers (i.e., using VS on Windows).

I don't really have an idea how to debug it without a build log from the CI, and the actual compilation apparently completing without critical errors.
I managed to build with OS X gcc 4.8.5 on the oldest Mac I could get hold of right now; that also built with -std=c99 and passed the wcs tests, so maybe it's a 32bit only issue. Those failures à la

E                   astropy.wcs.wcs.FITSFixedWarning: 'obsfix' made the change 'Observatory coordinates inconsistent by 1502543570342217992772683310777721624316675426054939682258832658863342747322483892439289327303502810188873728.0 metres'.

certainly look like there may be a mixup of float32 and float64 somehow!

That value is breathtaking!

Yes... <Quantity 4.86941208e+82 Gpc>

If possible to get c99 to work, I would prefer to use that option instead of gnu99. By being able to compile C code without enabling GNU extensions, I think the code will be able to compile on a wider range of platforms/compilers (i.e., using VS on Windows).

I was also more worried initially about the MSVC or mingw32 builds, but at least the CI one seems to accept it.
The original configure afaics is trying, in this order
'' -std=gnu99 -std=c99 -c99 -AC99 -D_STDC_C99= -qlanglvl=extc99
i.e. first without extra args, but to follow that approach setup_package.py would need some equivalent to ac_fn_c_try_compile. But I guess we could always make it conditional e.g. on unix compilers, or even

    if determine_64_bit_int() != 'int' and get_compiler() == 'unix':
        cfg['extra_compile_args'].extend(['-std=gnu99'])

else set -std=c99, or nothing at all, if all the win32 compilers already handle C99.