Switch from Autotools to CMake for fitsio-sys

[zec]

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.

[HaoxuanGuo]

Is it possiable to support MSVC native compile on Windows platform? Compared with autotools, CMake is cross platfrom supported includes native MSVC.

[zec]

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

[simonrw]

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¹. 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.

Footnotes

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. ↩

[simonrw]

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?

[zec]

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 does1. 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?

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 cmake for 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.

[HaoxuanGuo]

@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?

[simonrw]

@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:

1. building cfitsio from source with autotools
2. building cfitsio from source with cmake
   They are not additive, which I am ok with but we should probably proceed down one of two paths. Either
3. we make three features: fitsio-src, autotools and cmake and raise a build error if an invalid combination is passed (e.g. fitsio-src alone, or both autotools and cmake)
4. we use two features: cfitsio-src-autotools and cfitsio-src-cmake which 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:

1. move the autotools integration behind an autotools feature flag and make the dependency optional
2. put the changes you have made here behind a cmake feature flag
3. add validation in the build.rs if 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.

[zec]

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 (or cmake, 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.

[HaoxuanGuo]

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.