simonrw
Switch from Autotools to CMake for fitsio-sys
The current Autotools-based build process for the CFITSIO library in fitsio-sys (when enabled with the fitsio-src feature) requires an in-tree build, i.e., the .o files are placed in the same directory as the respective .c files. This leads to build failures when using fitsio from crates.io when it is built for multiple platforms—at least not without the remediation of removing the .o files, Makefile, etc. from ~/.cargo/registry/src in between builds, which is a bad developer experience.
This pull request switches from the Autotools-based build to using the alternative CMake-based build for CFITSIO. At the expense of requiring CMake to be installed, this allows for out-of-tree builds of CFITSIO, making multiple cross-compilations possible without needing a non-obvious remediation step.
Is it possiable to support MSVC native compile on Windows platform? Compared with autotools, CMake is cross platfrom supported includes native MSVC.
That is supported by the cmake crate by setting the CMAKE_GENERATOR environment variable [1] for the Cargo invocation.
[1] https://docs.rs/cmake/0.1.54/cmake/struct.Config.html#method.generator
Thanks for posting this PR. I am not convinced replacing one build system with another is the answer. I feel it is less likely that a user has cmake than has the autotools suite (really just make since the configure script has been created already).
I have been doing some investigation. autootols (specifically the Rust crate) supports out-of-tree builds already, however we are using a version of cfitsio that is too old to support it.
In my git bisecting I find that the git tag cfitsio-4.4.0 in the upstream repo does not support out-of-tree builds, but cfitsio-4.4.0 does[^1]. I think if we switch to using (at least) cfitsio 4.5.0 and call insource(true) in the build script then that should solve your out-of-tree build problem. How do you feel about this?
If you continue to feel strongly we can provide an option to support cmake for the build process, in which case this PR will come in handy.
[^1]: during my git bisect I found the actual commit that first supports out-of-tree builds for the cfitsio repo is https://github.com/HEASARC/cfitsio/commit/d575c988095ac644f3877d2afc2b45d322e57522 which is in between 4.4.1 and 4.5.0.
Is it possiable to support MSVC native compile on Windows platform? Compared with autotools, CMake is cross platfrom supported includes native MSVC.
@HaoxuanGuo rust-fitsio does not support MSVC at the moment, at least based on my testing. I do not remember why at this stage, but perhaps it was just that I could not compile cfitsio well enough. If you give @zec's suggestion (using CMAKE_GENERATOR) a try, can you let me know if you are successful?
In my git bisecting I find that the git tag
cfitsio-4.4.0in the upstream repo does not support out-of-tree builds, butcfitsio-4.4.0does1. I think if we switch to using (at least)cfitsio4.5.0 and callinsource(true)in the build script then that should solve your out-of-tree build problem. How do you feel about this?
I'll try that out, hopefully in the next couple of days, and get back to you... though I believe you'd actually want .insource(false).
If you continue to feel strongly we can provide an option to support
cmakefor the build process, in which case this PR will come in handy.
For my group's purposes, I am not wedded to the use of CMake, provided CFITSIO can be built out-of-tree by other means in a sufficiently-flexible manner (our cross-compilation setup's a little weird, and it took some effort to support it in the autotools-based build at all). I can definitely see how a CMake-based build might be nice for others as an option, though, and I can re-jigger the build script accordingly.
@simonrw Sorry for the late reply. I tested compile cfitsio on Windows, it successed but with a dependency hell. cftisio depends on pthreads and zlib; zlib depends on pwsh. Which means rust-cfitsio is hard to build on a pure Windows without tuning. On another hand, it is not hard to install cfitsio from source for a user who has installed pthreads, zlib and pwsh. I whink it is not fluent on Windows with --features fitsio-src unless we would deal with these dependences. But, is it elegant to deal with these dependences on fitsio-src?
@HaoxuanGuo thanks for posting this information, but I'd like to split up integrating cmake and getting MSVC support working into two separate issues. I already have an open issue for MSVC support however it's not very well fleshed out! Would you mind adding your thoughts over there?
Regarding cmake support @zec I think I am ok with including optional cmake support since it supports your use case. I don't think it should be default though, but I am unclear on the UX.
Cargo features are meant to be additive, however we have two opposing features:
- building
cfitsiofrom source withautotools - building
cfitsiofrom source withcmakeThey are not additive, which I am ok with but we should probably proceed down one of two paths. Either - we make three features:
fitsio-src,autotoolsandcmakeand raise a build error if an invalid combination is passed (e.g.fitsio-srcalone, or bothautotoolsandcmake) - we use two features:
cfitsio-src-autotoolsandcfitsio-src-cmakewhich uniquely select one of the two distinct combinations, but they are pretty ugly names.
I am siding with 1. here, so can we do the following:
- move the
autotoolsintegration behind anautotoolsfeature flag and make the dependency optional - put the changes you have made here behind a
cmakefeature flag - add validation in the
build.rsif an invalid combination is given, with a helpful error message
I am happy to help push this over the line but unfortunately I don't have much time to devote to this implementation myself, sorry.
Regarding crate features: for optionally using the CMake build, I was thinking of having features with the following semantics:
fitsio-src: "build and link CFITSIO from source"src-cmake(orcmake, or whatever name): "if and when we build C code, use CMake (instead of the implicit (and historical) default of Autotools)"
This would be backward compatible while still having sensible meanings for any combination of features [1].
(Apologies for not getting around to testing building with the new version of CFITSIO yet; I've gotten pulled over to other things at work for the time being.)
[1] -fitsio-src, +src-cmake would have the same effect as -fitsio-src, -src-cmake, but that's OK.
I'm agree with @zec . Compatible is important for users and we should let it transparent.
MSVC support is another tough work, only cmake discuss here, nor MSVC would deal with later in another issue.
Hi, sorry for the lack of responses. I am happy to include this feature with the fetaure flags you suggest, however I am unable to build this branch myself in its current state.
I get the following error:
Updating crates.io index
Downloaded pkg-config v0.3.31
Downloaded cc v1.2.6
Downloaded 2 crates (118.0KiB) in 0.76s
Compiling cc v1.2.6
Compiling pkg-config v0.3.31
Compiling cmake v0.1.54
Compiling fitsio-sys v0.5.5 (/Users/simon/dev/rust-fitsio/fitsio-sys)
error: failed to run custom build command for `fitsio-sys v0.5.5 (/Users/simon/dev/rust-fitsio/fitsio-sys)`
Caused by:
process didn't exit successfully: `/Users/simon/.cargo-target/debug/build/fitsio-sys-bc2e23a7a5220328/build-script-build` (exit status: 101)
--- stdout
CMAKE_TOOLCHAIN_FILE_aarch64-apple-darwin = None
CMAKE_TOOLCHAIN_FILE_aarch64_apple_darwin = None
HOST_CMAKE_TOOLCHAIN_FILE = None
CMAKE_TOOLCHAIN_FILE = None
CMAKE_GENERATOR_aarch64-apple-darwin = None
CMAKE_GENERATOR_aarch64_apple_darwin = None
HOST_CMAKE_GENERATOR = None
CMAKE_GENERATOR = None
CMAKE_PREFIX_PATH_aarch64-apple-darwin = None
CMAKE_PREFIX_PATH_aarch64_apple_darwin = None
HOST_CMAKE_PREFIX_PATH = None
CMAKE_PREFIX_PATH = None
CMAKE_aarch64-apple-darwin = None
CMAKE_aarch64_apple_darwin = None
HOST_CMAKE = None
CMAKE = None
running: cd "/Users/simon/.cargo-target/debug/build/fitsio-sys-28c4f9abbc9ceb30/out/build" && CMAKE_PREFIX_PATH="" LC_ALL="C" "cmake" "/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio" "-DCMAKE_OSX_ARCHITECTURES=arm64" "-DUseCurl=OFF" "-DBUILD_SHARED_LIBS=OFF" "-DUSE_PTHREADS=ON" "-DCMAKE_INSTALL_PREFIX=/Users/simon/.cargo-target/debug/build/fitsio-sys-28c4f9abbc9ceb30/out" "-DCMAKE_C_FLAGS= -O0 -fPIE -ffunction-sections -fdata-sections -fPIC --target=arm64-apple-macosx15.5" "-DCMAKE_C_COMPILER=/usr/bin/cc" "-DCMAKE_CXX_FLAGS= -ffunction-sections -fdata-sections -fPIC --target=arm64-apple-macosx15.5" "-DCMAKE_CXX_COMPILER=/usr/bin/c++" "-DCMAKE_ASM_FLAGS= -ffunction-sections -fdata-sections -fPIC --target=arm64-apple-macosx15.5" "-DCMAKE_ASM_COMPILER=/usr/bin/cc" "-DCMAKE_BUILD_TYPE=Debug"
-- The C compiler identification is AppleClang 17.0.0.17000013
-- The CXX compiler identification is AppleClang 17.0.0.17000013
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Pthreads: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/usr/lib/libpthread.tbd
-- Looking for gethostbyname
-- Looking for gethostbyname - found
-- Looking for connect
-- Looking for connect - found
-- Configuring done (2.0s)
-- Generating done (0.0s)
-- Build files have been written to: /Users/simon/.cargo-target/debug/build/fitsio-sys-28c4f9abbc9ceb30/out/build
running: cd "/Users/simon/.cargo-target/debug/build/fitsio-sys-28c4f9abbc9ceb30/out/build" && LC_ALL="C" "cmake" "--build" "/Users/simon/.cargo-target/debug/build/fitsio-sys-28c4f9abbc9ceb30/out/build" "--target" "install" "--config" "Debug" "--parallel" "8"
[ 1%] Building C object CMakeFiles/cfitsio.dir/buffers.c.o
[ 2%] Building C object CMakeFiles/cfitsio.dir/drvrnet.c.o
[ 4%] Building C object CMakeFiles/cfitsio.dir/checksum.c.o
[ 5%] Building C object CMakeFiles/cfitsio.dir/editcol.c.o
[ 6%] Building C object CMakeFiles/cfitsio.dir/drvrfile.c.o
[ 8%] Building C object CMakeFiles/cfitsio.dir/cfileio.c.o
[ 9%] Building C object CMakeFiles/cfitsio.dir/drvrmem.c.o
[ 10%] Building C object CMakeFiles/cfitsio.dir/edithdu.c.o
[ 12%] Building C object CMakeFiles/cfitsio.dir/eval_f.c.o
[ 13%] Building C object CMakeFiles/cfitsio.dir/eval_l.c.o
[ 14%] Building C object CMakeFiles/cfitsio.dir/eval_y.c.o
[ 16%] Building C object CMakeFiles/cfitsio.dir/f77_wrap2.c.o
[ 17%] Building C object CMakeFiles/cfitsio.dir/f77_wrap1.c.o
--- stderr
CMake Warning (dev) at CMakeLists.txt:6 (PROJECT):
cmake_minimum_required() should be called prior to this top-level project()
call. Please see the cmake-commands(7) manual for usage documentation of
both commands.
This warning is for project developers. Use -Wno-dev to suppress it.
CMake Deprecation Warning at CMakeLists.txt:7 (CMAKE_MINIMUM_REQUIRED):
Compatibility with CMake < 3.10 will be removed from a future version of
CMake.
Update the VERSION argument <min> value. Or, use the <min>...<max> syntax
to tell CMake that the project requires at least <min> but has been updated
to work with policies introduced by <max> or earlier.
CMake Warning:
Manually-specified variables were not used by the project:
CMAKE_ASM_COMPILER
CMAKE_ASM_FLAGS
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/editcol.c:129:46: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value]
129 | newsize = (newsize + pcount) * gcount * (abs(longbitpix) / 8);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:323:7: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
323 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:356:5: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
356 | alarm(net_timeout*10);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:374:5: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
374 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/editcol.c:129:46: note: use function 'labs' instead
129 | newsize = (newsize + pcount) * gcount * (abs(longbitpix) / 8);
| ^~~
| labs
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:390:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
390 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:456:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
456 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:615:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
615 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:823:5: error: call to undeclared function 'close'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
823 | close(sock);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1060:5: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1060 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1071:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1071 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1095:10: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
1094 | snprintf(errStr,MAXLEN,"Content-Length not a multiple of 2880 (https_open) %u",
| ~~
| %zu
1095 | inmem.size);
| ^~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/usr/include/secure/_stdio.h:60:62: note: expanded from macro 'snprintf'
60 | __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
| ^~~~~~~~~~~
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1138:6: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1138 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1148:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1148 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1182:6: warning: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
1181 | "Content-Length not a multiple of 2880 (https_file_open) %d",
| ~~
| %zu
1182 | inmem.size);
| ^~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/usr/include/secure/_stdio.h:60:62: note: expanded from macro 'snprintf'
60 | __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
| ^~~~~~~~~~~
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1347:5: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1347 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1358:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1358 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1419:13: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
1418 | snprintf(errStr,MAXLEN,"Content-Length not a multiple of 2880 (ftps_open) %u",
| ~~
| %zu
1419 | inmem.size);
| ^~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/usr/include/secure/_stdio.h:60:62: note: expanded from macro 'snprintf'
60 | __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
| ^~~~~~~~~~~
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1467:6: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1467 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1477:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1477 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1571:9: warning: format specifies type 'int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
1570 | "Content-Length not a multiple of 2880 (ftps_file_open) %d",
| ~~
| %zu
1571 | inmem.size);
| ^~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/usr/include/secure/_stdio.h:60:62: note: expanded from macro 'snprintf'
60 | __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
| ^~~~~~~~~~~
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1622:6: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1622 | alarm(0);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:1632:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1632 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:2116:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
2116 | alarm(net_timeout);
| ^
/Users/simon/dev/rust-fitsio/fitsio-sys/ext/cfitsio/drvrnet.c:2253:3: error: call to undeclared function 'alarm'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
2253 | alarm(net_timeout);
| ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
4 warnings and 20 errors generated.
gmake[2]: *** [CMakeFiles/cfitsio.dir/build.make:149: CMakeFiles/cfitsio.dir/drvrnet.c.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....
1 warning generated.
eval.y:5022:16: warning: expression result unused [-Wunused-value]
5022 | this->value.undef; /* Dummy - BITSTRs do not have undefs */
| ~~~~~~~~~~~ ^~~~~
eval.y:5116:16: warning: expression result unused [-Wunused-value]
5116 | this->value.undef; /* Dummy - BITSTRs do not have undefs */
| ~~~~~~~~~~~ ^~~~~
2 warnings generated.
gmake[1]: *** [CMakeFiles/Makefile2:87: CMakeFiles/cfitsio.dir/all] Error 2
gmake: *** [Makefile:136: all] Error 2
thread 'main' panicked at /Users/simon/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cmake-0.1.54/src/lib.rs:1119:5:
command did not execute successfully, got: exit status: 2
build script failed, must exit now
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
$
It might be a macOS aarch64 issue, however this is my primary development platform and therefore it is the highest priority for me. We regularly test Linux on CI anyway.
I don't know if you are using macOS yourself, however if you are not I'm happy to dive more into this failure myself. It just might take a while.
In the meantime you are able to build your feature branch for yourself, and include it into your projects. I hope that's enough to enable this workflow for you at least, and you are not blocked by this.
I have (finally) gotten around to updating this pull request... in large part because multiple teammates ran into the same compiler errors on macOS as @simonrw.
This rev of the pull request provides a choice between Autotools and CMake when using fitsio-src, as previously proposed. The CMake build now works on macOS and MSYS2.