Add cmake build system

[timj]

Does not attempt to build the documentation.

• Includes some wcslib bug fixes from upstream.
• Fixed some compiler warnings with buffer overruns
• Replaces K&R prototypes with modern versions to prevent compiler warnings and allow C23.
• Includes some tests ported over from Fortran,

[timj]

@dsberry I've had a go (with Claude) to fix some of the compiler warnings. This fixes K&R problems and string buffers. Whilst it was fixing wcslib it suggested 3 cleanups that were in upstream wcslib that we should use.

The Cmake build was to make it easier for Claude to build and test the code standalone. I have not verified that the plugin code is working at all.

See what you think.

[embray]

Nice, works for me. I'm impressed that it rewrote messgen entirely in cmake, which I guess is a cleaner approach.

Still think it's too bad it doesn't run most of the tests -- for that my PR would still be needed, or at least the parts that provide my Starlink compatibility bits.

You've both mentioned wanting to rewrite the tests in C though which would be another way to handle that. I don't know the history well enough to know if or why it's important to still test the Fortran interface.

[embray]

The cmake build also at least seemed significantly faster to me on my machine -- maybe less overhead due to libtool, etc.

[dsberry]

Thanks Tim, I'll take a look but it will probably be the second half of next week before I can get round to it.

[timj]

Nice, works for me. I'm impressed that it rewrote messgen entirely in cmake, which I guess is a cleaner approach.

Yes. It was pretty wild seeing it work out from first principles how messgen worked. It managed to work out the formula without looking at messgen.c (although at some point I did point it at the source for validation).

Still think it's too bad it doesn't run most of the tests -- for that my PR would still be needed, or at least the parts that provide my Starlink compatibility bits.

I'm more than happy to merge your PR as well since it's more likely to be more compatible with make dist and build things in the same way as the Starlink build. The cmake has not been tested for tar ball making and installs yet but is really handy for LLMs.

You've both mentioned wanting to rewrite the tests in C though which would be another way to handle that. I don't know the history well enough to know if or why it's important to still test the Fortran interface.

Give me a minute. I had Claude planning on migrating the tests to C but it ran out of tokens last night. The Fortran is still important because most of the Starlink application code is still in Fortran so we have to make sure the interface is working.

[embray]

That's one of the main things I prefer about autotools -- it is better for declaring what should go into source distributions. CPack takes the wrong approach IMO, though it's more flexible for things like Windows installers etc.

If you decide to merge this you can do that first and I can rebase my branch no problem.

[timj]

For interest, here's the summary of the wcsLib upstream vs AST version comparison that Claude generated all on its own:

[timj]

This is the summary from Claude of what it has tried to do on this pull request so far:

Summary:

Primary Request and Intent:

The user asked to add a CMake build system to the AST (Astrometry/World Coordinate Systems) C library that doesn't require Starlink-specific build infrastructure. This evolved into a multi-session effort covering: CMake build system creation, README documentation, GitHub Actions CI, optional Fortran interface support, fixing compiler warnings (buffer overflows, prototype mismatches), cherry-picking bug fixes from upstream wcslib, modernizing K&R function definitions, and a major test coverage enhancement effort converting Fortran tests to C. The overarching goal is making AST buildable and testable without any Starlink dependencies.

Key Technical Concepts:

• CMake build system (minimum 3.24) alongside existing Starlink autotools
• Starlink MESSGEN error code generation formula: ((fac|0x800)<<16) + ((msgno|0x1000)<<3) + severity
• makeh Perl script generates ast.h public header via C preprocessor
• Link-time plugin architecture: libast uses -undefined dynamic_lookup on macOS; satellite libraries (ast_err, ast_grf_*) provide error/graphics implementations resolved at link time
• WHOLE_ARCHIVE linking required for test executables to pull in static satellite library symbols
• palwrap.c #includes all PAL/ERFA .c files with symbol renaming via pal2ast.h/erfa2ast.h
• Vendored wcslib (from v2.9) with AST-specific modifications (ast prefix, dynamic p/p2 arrays, TPN projection, K&R definitions)
• Fortran-to-C test conversion patterns: astToString/astFromString replaces channel callbacks, astMalloc/astFree replaces PSX, err_mark/err_rlse omitted, 1-based to 0-based index adjustments
• Fortran CHARACTER padding vs C null-terminated strings affects FitsTable column sizes
• Thread-safe vs static global variable declarations must match sizes (e.g., FormatKey_Buff, CnvType_Text)

Files and Code Sections:

• CMakeLists.txt (top-level)

  • Main CMake build file, minimum 3.24, project version 9.3.1
  • System checks replacing autoconf (CheckIncludeFile, CheckFunctionExists, CheckTypeSize, CheckCSourceCompiles)
  • Optional Fortran: searches PATH for gfortran then versioned gfortran-NN (8-20), determines TRAIL_TYPE/REAL_FUNCTION_TYPE/BLANK_COMMON_SYMBOL
  • Generates config.h, version.h, ast_err.h, object.h, f77.h, ast.h, AST_PAR
  • -undefined dynamic_lookup on macOS for shared libast

• add_subdirectory(ast_tester) inside BUILD_TESTING block

• cmake/gen_ast_err_h.cmake

  • Parses ast_err.msg, generates C header with same values as Starlink MESSGEN

• cmake/gen_ast_h.sh

  • Shell wrapper that creates staging directory for makeh Perl script in out-of-source builds

• cmake/gen_ast_par.cmake

  • Runs astbad utility to compute AST__BAD/AST__NAN/AST__NANF, substitutes into ast_par.source with E->D exponent conversion

• cmake/config.h.cmake.in, cmake/version.h.cmake.in, cmake/ast.pc.in, cmake/astConfig.cmake.in

  • Template files for configure_file

• cmake/sae_par.h

  • Stub: #define SAI__OK 0 and #define SAI__ERROR 1

• cmake/star/mers.h

  • Inline no-op stubs for errMark, errRlse, errStat, errAnnul, errRep

• .github/workflows/cmake.yaml

  • CI for ubuntu-latest and macos-latest, shared/static builds, optional Fortran builds
  • Installs libyaml and gcc on each platform

• README.md

  • Project overview, CMake build instructions (prerequisites, options, find_package/pkg-config usage), Starlink autotools instructions, 30-test suite mention

• PLAN.md

  • Living document tracking test coverage enhancement progress, must be kept updated

• ast_tester/CMakeLists.txt

  • Test target definitions with WHOLE_ARCHIVE linking for satellite libs
  • ast_add_test() helper function
  • Conditional tests for YAML and pthreads
  • Data file copying (2dspline_c.dat, splittest1.ast)

• Compiler warning fixes:

  • src/fitschan.c: formatkey_buff 10->40, FormatKey_Buff struct member 10->40, sprintf->snprintf throughout FormatKey function, CnvType_Text sprintf->snprintf, buf/buf2 overlap fix (tmpbuf[600] + strncpy), snprintf for cnvtype_text
  • src/fitschan.h: astPutFits_ const char [81] -> const char *, FormatKey_Buff[10]->[40], CnvType_Text[81]->[163]
  • src/fitstable.c: keyword[14]->[20]
  • src/keymap.c: buff[20]->[30]
  • src/memory.c: stemp[10]->[24]
  • src/xmlchan.c: orgatt/sysatt[20]->[30]
  • src/yamlchan.c: rowname[20]->[30]
  • src/cmpframe.c: sprintf->snprintf with sizeof(buf2)

• src/ellipse.h and src/ellipse.c: 5th param of astEllipse_/astEllipseId_/astInitEllipse_ from const double[2] to const double *

• src/moc.c: null guard text ? text : "" for format strings

• Vendored wcslib fixes:

  • wcslib/proj.c: ZPN w[0] initialization for k<2 (prj->w[0] = PI), AIT boundary u<0.5 instead of u<0.0, all 87+2 functions modernized from K&R
  • wcslib/wcstrig.c: cosd/sind rewritten with floor(angle/90+/-0.5)%4 switch, all 7 functions modernized
  • wcslib/tpn.c: all 3 functions modernized from K&R
  • src/dssmap.c: platepos/platepix modernized from K&R

• 19 Fortran-to-C test conversions (all in ast_tester/):

  • testzoommap.c, testnormmap.c, testmapping.c, testskyframe.c, testcmpframe.c, testlutmap.c, testratemap.c, testchannel.c (Batch 1)
  • testrate.c, testspecframe.c, testflux.c, testspecflux.c, testcmpmap.c, testpolymap.c (Batch 2)
  • testchebymap.c, testunitnormmap.c, testtrangrid.c, testmoc.c, testfitstable.c (Batch 3)

• Existing test fixes:

  • ast_tester/testobject.c: baseName() helper, LINE captures replacing hardcoded line numbers
  • ast_tester/testsplinemap_c.c: fscanf return value checks
  • ast_tester/testthreads.c: return NULL; in worker()

Errors and fixes:

• Satellite library segfaults: Tests crashed on null function pointers because static satellite libs (ast_err, ast_grf_*) weren't linked into test executables. Fix: WHOLE_ARCHIVE linking.
• testobject segfault: FILE includes path in out-of-source builds, line numbers shifted. Fix: baseName() helper, LINE captures.
• testyamlchan/testthreads segfault: Same satellite library issue as above, fixed by WHOLE_ARCHIVE.
• testratemap compile error: astPermMap(... 0.0 ...) - bare double instead of pointer. Fix: double zero = 0.0; astPermMap(..., &zero, ...).
• testunitnormmap segfault: Purely relative tolerance in differ() failed for near-zero values (3.69e-17 vs 0.0). User said "it can't be a precision issue because the Fortran code is very quickly calling into the same C interface." Fix: added absolute tolerance floor if(tol < 1e-14) tol = 1e-14.
• testmoc segfault: astGetCell uses 0-based indices in C, 1-based in Fortran. Fix: loop from 0 to MocLength-1.
• testmoc compile issues: astGetFitsS takes char**, not char[]; astAddPixelMaskF not R; AST__NANF not NANR; size_t not int64_t for GetMocString.
• testfitstable failures: AST__BADTYP not AST__NAXIN; byte array column-major ordering; string column sizes differ (C doesn't pad). Fix: used correct error code, reordered bytes, relaxed size checks with round-trip verification instead.
• testspecflux astEqual segfault: astEqual segfaults on SpecFluxFrame. Reverted to just checking round-trip succeeds.
• macOS CI gfortran not found: Homebrew installs versioned gfortran-NN. Initially hardcoded paths, user said to search PATH. Fix: find_program loop gfortran-20 down to gfortran-8.
• fitschan GCC warnings persisted: formatkey_buff enlarged but sprintf still warned because GCC can't prove remaining space. Fix: changed to snprintf with sizeof-len.
• K&R function warnings: wcslib and dssmap.c used old-style definitions. Fix: Python script + manual conversion to modern prototypes.

Problem Solving:

• Reverse-engineered Starlink MESSGEN error code formula from generated output and messgen source
• Designed staging directory approach for makeh Perl script in out-of-source CMake builds
• Analyzed vendored wcslib vs upstream v8.6 (24 years of changes) to identify cherry-pickable bug fixes vs impractical API changes
• Determined vectorized wcslib API not worth porting (scalar calls adequate, modest benefit vs high engineering cost)
• Developed systematic approach for Fortran-to-C test conversion: astToString/astFromString for checkdump, astMalloc for PSX, standard C for CHR
• Identified and documented C vs Fortran string padding difference affecting FitsTable tests

Pending Tasks:

• Continue converting remaining 13 Fortran tests to C (testframeset, testswitchmap, testtime, testkeymap, testxmlchan, testmocchan, teststcschan, testfitschan, teststc, testtable, testregions, testrebin, testrebinseq)
• Decide whether C API should pad strings for FITS BINTABLE conventions (testfitstable TODO)
• Investigate astEqual segfault on SpecFluxFrame (potential library bug)
• Future: runtime plugin system to replace link-time symbol resolution
• Keep PLAN.md updated as work progresses

Current Work:

The most recent work was small cleanup tasks as the user's token allocation was running low. I updated the README.md to mention the 30-test suite and reference PLAN.md. Before that, I attempted to strengthen testspecflux.c's checkdump with astEqual (segfaulted) and astOverlap (not applicable to Frame), then reverted and documented the limitation. The overall test conversion effort reached 30 passing tests (1 original + 10 existing C + 19 converted from Fortran).

Optional Next Step:

The next logical step per PLAN.md priority would be converting testframeset.f (391 lines, checkdump pattern) to C, as it's the highest-priority remaining unconverted test.

[timj]

Turning on sanitizers was really helpful because 3 tests immediately failed. The analysis reports:

[P1] PutColumnData reads float column buffers as double *, which causes the testfitstable ASan failure. In the AST__FLOATTYPE branch it does ((double *) pin)[iel] before calling astMapPut1F, so a 12-byte float[3] buffer is misaligned and overrun as if it were double[3]. See src/fitstable.c (line 2206).

[P1] PermMap::Equal can read past the end of a permutation array when comparing maps with different effective inversion states, which matches the testswitchmap trace. The loop runs to nin/nout, but PERMVAL indexes a non-NULL perms[i] unconditionally, even when that_inp or that_outp has been swapped to the shorter opposite-direction array. In the failing test case this is very plausibly an inverted PermMap( nin=2, nout=1 ) comparison reading outperm[1]. See src/permmap.c (line 338) and src/permmap.c (line 142).

[P1] MocChan::Sink1 copies one byte too many from a non-NUL-terminated buffer, which is the testmocchan ASan failure. The comment says buf is not null terminated, but it calls astStore( NULL, buf, nc + 1 ), causing an 81-byte read from an nc-byte buffer, and only then writes the terminator. See src/mocchan.c (line 769).

[P2] FitsChan has an additional UBSan issue exposed during testfitstable: reversing zero returned fields does pointer arithmetic on fields + nfret - 1 when nfret == 0, producing NULL - 1. That is separate from the actual ASan crash above, but still real UB. See src/fitschan.c (line 22585).

I will try to take a look at them.

[timj]

@dsberry for review purposes the most important thing to look at are the patches in the src/ directory for the core AST code that were found when I turned on address sanitization when running the tests and discovered some real buffer overruns.

You can likely ignore the cmake files if you want because the cmake build isn't a true replacement for autoconf yet: it hasn't got a usable dist target and the pkg-config code is not going to replace ast_link.

The test code is entirely up to you. It might be worth having a look at a random file to see what you think and tell me if you would like things reorganized.

[dsberry]

Thanks for the update. I was wondering where to start with all this. I've ear-marked some time on Wednesday to have a look at to the current PRs.

[timj]

You should look at the new README as well. There are instructions in there for how to run the cmake tests.

[dsberry]

Just starting to have a look at this. I've built it successfully using the classic starconf method, and the tests all seem to pass. I've also built it using cmake, following the instructions in README.md. Is there any way to speed things up by using multiple processes to compile multiple source files simultaneously, like the "make -j" option? Also, what's the best way to modify the compiler options (e.g. switching off optimisation) - do I just set CFLAGS before running the first cmake command?

After installation, I tried using pkg-config but got:

% pkg-config --cflags --libs ast
Package ast was not found in the pkg-config search path.
Perhaps you should add the directory containing `ast.pc'

I can't find ast.pc anywhere in the installation directory. I can see it in the source directory (build/ast.pc), but I expected it to be installed somewhere.

Regarding the changes to the core C files:

• In the places where the argument list for a function has been changed, the comment describing the argument list within the function's prologue (which are used for the documentation) seems not to have been updated.
• I'm not sure of the reason for some of the argument list changes. For instance, in ellipse.c, in function astInitEllipse_, argument "point2" has been changed from "const double point2[2]" to "const double *point2", but argument "point1", which seems identical in usage, has been left unchanged. Not sure why.
• In fitschan.c, at line 22587, the check on nfret is not really needed because if nfret were zero, the code in the following loop would never be executed. It's harmless though, although "nfret > 1" would make more sense, since the loop will not be executed if nfret is 1.
• A shame the changes don't have entries in the "History" section of the main prologue at the top of each file.

Otherwise, all looks good. Well done so far.

[timj]

Is there any way to speed things up by using multiple processes to compile multiple source files simultaneously, like the "make -j" option? Also, what's the best way to modify the compiler options (e.g. switching off optimisation) - do I just set CFLAGS before running the first cmake command?

The README has been updated to explain release modes and parallelism and how to use CFLAGS.

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[timj]

@dsberry I added code coverage reporting so if you look at the "coverage" github action and look for the section where it says it is uploading the HTML artifacts, you can view a coverage report for the library based on the C test code. fitschan.c currently at 50%. The readme also explains how to enable coverage.

[timj]

Interesting from the analysis of testrebin.f:

One note: the Fortran has a bug where TEST2/3/5/6 duplicate data checks instead of checking variance — the C faithfully reproduces this behavior. Not a compliance issue.

I guess I should fix the fortran and the C...

[timj]

For fun I got it to write a PLplot interface so that it would be easier for me to check the plotting situation without needing PGPLOT. I used Gemini Pro 3.1 for this and it did manage to sort itself out pretty well. This is the cobe example:

[dsberry]

Interesting from the analysis of testrebin.f:

One note: the Fortran has a bug where TEST2/3/5/6 duplicate data checks instead of checking variance — the C faithfully reproduces this behavior. Not a compliance issue.

I guess I should fix the fortran and the C...

@timj This reminds me - do future changes to the testers need to made independently in C and F77 source files, or can the new C files be changed automatically based on the changes made to the F77 files?

[dsberry]

@dsberry I added code coverage reporting so if you look at the "coverage" github action and look for the section where it says it is uploading the HTML artifacts, you can view a coverage report for the library based on the C test code. fitschan.c currently at 50%. The readme also explains how to enable coverage.

@timj - okay. It looks like there is a lot of work to do to get rid of all those red fields in the coverage report!

[timj]

do future changes to the testers need to made independently in C and F77 source files, or can the new C files be changed automatically based on the changes made to the F77 files?

They are completely independent. In theory you could continue to write Fortran test code and I could periodically ask an agent to sync up the C tests. Of course if you don't patch the C test you don't get an improved coverage report.

It looks like there is a lot of work to do to get rid of all those red fields in the coverage report!

Once we merge this giant pull request we should think about the FITS chan coverage. What I did for the rust experiment was have tests that read in every FITS header in ast_tester and compared it to a reference AST native serialization to make sure it came up with the right answer. That would prevent the FitsChan code from regressing and give us better coverage, at the expense of a few more simplified files that we have to trust are correct. Does this sound like a good start? Then there becomes a mechanism for seeing which special cases in FitsChan don't have any test header to demonstrate them.

PS Now that we have checks running, you really want to start getting into the habit of making a pull request rather than pushing direct to master.

[timj]

Okay. I've rebased. Waiting for checks to pass. Please merge using the default merge scheme. I see with #35 there is no merge commit. I think those commits are important so that you can tell which pull request made it. It's especially important for this PR which is gigantic.

[dsberry]

Okay. I've rebased. Waiting for checks to pass. Please merge using the default merge scheme. I see with #35 there is no merge commit. I think those commits are important so that you can tell which pull request made it. It's especially important for this PR which is gigantic.

okay. Done

[embray]

@timj By the way I am getting a couple test failures locally (on Linux) like:

"wcsconv_dss_fits-dss_ast" start time: Apr 22 16:48 CEST
Output:
----------------------------------------------------------
-- Output mismatch; showing unified diff:
--- dss.ast     2026-04-22 16:46:56.404175711 +0200
+++ dss_fits-dss_ast.out        2026-04-22 16:48:39.110171123 +0200
@@ -9,8 +9,8 @@
 #   Uni1 = "hh:mm:ss.s"        # Units for axis 1
 #   Uni2 = "ddd:mm:ss"         # Units for axis 2
 #   Dir1 = 0   # Plot axis 1 in reverse direction
-#   Bot2 = -1.5707963267948966         # Lowest legal axis value
-#   Top2 = 1.5707963267948966  # Highest legal axis value
+#   Bot2 = -1.57079632679489656        # Lowest legal axis value
+#   Top2 = 1.57079632679489656         # Highest legal axis value
  IsA Frame     # Coordinate system description
     Nframe = 2         # Number of Frames in FrameSet
     Base = 1   # Index of base Frame
@@ -46,8 +46,8 @@
 #         Uni1 = "hh:mm:ss.s"  # Units for axis 1
 #         Uni2 = "ddd:mm:ss"   # Units for axis 2
 #         Dir1 = 0     # Plot axis 1 in reverse direction
-#         Bot2 = -1.5707963267948966   # Lowest legal axis value
-#         Top2 = 1.5707963267948966    # Highest legal axis value
+#         Bot2 = -1.57079632679489656  # Lowest legal axis value
+#         Top2 = 1.57079632679489656   # Highest legal axis value
           Ax1 =        # Axis number 1
              Begin SkyAxis     # Celestial coordinate axis
              End SkyAxis
@@ -56,11 +56,11 @@
              End SkyAxis
        IsA Frame       # Coordinate system description
           Proj = "gnomonic polynomial"         # Description of sky projection
-#         SkyTol = 0.001       # Smallest significant separation [arc-sec]
+#         SkyTol = 0.00100000000000000002      # Smallest significant separation [arc-sec]
<cropped>

I am guessing likely due to the AST_DBL_DIG issue you mentioned earlier.

In my CMakeCache.txt I have:

AST_C_STANDARD:STRING=99

[timj]

Yes, the GitHub actions are forcing C11 but it's not the default. I need to read back from the weekend to see which options David suggested is the right fix. The AST fallback should probably be set to the C1q standard value.