Add CMake support

[ajtribick]

Possibly of relevance to #32 - I wanted to be able to build this on Windows more easily, and I'm generally using CMake for my C++ projects on Linux. I've translated most of the automake build to CMake (minus generation of the .la libtool file), including generating the required files to allow both find_package(erfa) and including the library sources in the project, either way you end up with an erfa::erfa target which can be provided to target_link_libraries.

[avalentino]

LGTM

[mhvk]

I don't know enough about compiling on Windows to judge the PR, but it does seem to me that if we support it, we better test it too! Let me ping @astrofrog since we may as well move away from travis at the same time (for which we'll run out of free minutes at some point).

[mhvk]

If it is autogenerated, does it have to be included?

[ajtribick]

I overlooked updating this comment when I translated the generated file to cmake format. Updated.

[mhvk]

OK, no problem!

[mhvk]

@ajtribick - on second thought, there is no strict need to couple the move from travis to github actions to this PR, so if you are able to set up travis to test on windows too, that would be good enough (.travis.yml in the main directory).

[ajtribick]

Ok, I've added the Windows build with CMake+VS2017. As I understand it, the Travis environment does not currently have VS2019 installed by default, and I didn't want to overcomplicate things if you're going to be migrating to GitHub Actions in the near future. It would also be possible to add Linux CMake builds, if you want me to add those let me know.

[mhvk]

Nice! Approving too. Though let's wait with merging a few days to give @astrofrog a chance to have a look as well.

[astrofrog]

looks good to me too!

[sergiopasra]

Currently we are building releases with make distcheck. But the makefile is not aware of all these cmake files, so they won't be distributed in erfa-xxxx.tar.gz. It's fine if you build with a git checkout, though

[sergiopasra]

The release instructions in RELEASE.rst should be updated if we want to keep the version of the package in sync between autotools and cmake

[sergiopasra]

Why do you need another .pc file?

[ajtribick]

Good point, looks like it's possible to use the existing one (unlike config.h.in, where there are some formatting differences). Updated.

[ajtribick]

I've updated RELEASE.rst to include the version changes to the CMake files. Do you also want to include the Windows build instructions as well?

[sergiopasra]

I've updated RELEASE.rst to include the version changes to the CMake files. Do you also want to include the Windows build instructions as well?

If you consider that can be useful, please do. Not in RELEASE.rst, in a different file

[FFFFFG-IGG]

Possibly of relevance to #32 - I wanted to be able to build this on Windows more easily, and I'm generally using CMake for my C++ projects on Linux. I've translated most of the automake build to CMake (minus generation of the .la libtool file), including generating the required files to allow both find_package(erfa) and including the library sources in the project, either way you end up with an erfa::erfa target which can be provided to target_link_libraries.

Hello！I‘m a new windows user of VS2022, and I'd like to add 'erfa.h' into my project and use it, what should I do ? Would mind give me some suggestions? Many thanks!

[ajtribick]

Sorry, forgot this was still open when I did a bit of spring cleaning of my repositories. In any case, this isn't something I need right now as the Python version is sufficient for the project I was using erfa in.

[avalentino]

@astrofrog, @sergiopasra, @mhvk I think that this would be a very nice addition to liberfa.
If I understand correctly the PR is in a good shape and already has 3 approvals.

What is missing to finalize and merge?

[mhvk]

Yes, it does seem useful, but there were some last comments from @sergiopasra about how to mention this in the release instructions. More generally, since I don't use or know CMake, I felt it should not be me merging it - but am very happy for someone else to do so!

[avalentino]

Thanks @mhvk
I kindly ask you to re-open this PR then.
I cannot help with windows stuff but I will be happy to fix any remaining issues if needed.

[mhvk]

OK, reopened

[mhvk]

What is the status of this PR? It looks like it was down to the last little bits, but it hasn't ended up being merged. #32 (comment) reminded me that it would be useful to have!

[mhvk]

This PR was found to still work well on windows, though with a small comment. It would be good to revive it!

@ajtribick - your branch actually seems to have disappeared, although I think we still have here everything we need.
Would you want to rebase & update? If not, do not worry. I think we can do it. Indeed, @laheller, maybe you can help since you also have a suggestion for making the dynamic libraries? @avalentino - you earlier volunteered to add the one missing thing, which I think was short installation instructions for on windows. Would you have time to help wrap this up?

[laheller]

This PR was found to still work well on windows, though with a small comment. It would be good to revive it!

@ajtribick - your branch actually seems to have disappeared, although I think we still have here everything we need. Would you want to rebase & update? If not, do not worry. I think we can do it. Indeed, @laheller, maybe you can help since you also have a suggestion for making the dynamic libraries? @avalentino - you earlier volunteered to add the one missing thing, which I think was short installation instructions for on windows. Would you have time to help wrap this up?

@mhvk
My suggestion would be shortly to introduce a new macro definition in the headers based on condition whether we are on WIN32. When yes, the macro, let's say "LIBERFA_EXPORT" has the value "__declspec(dllexport)", otherwise undefined. Finally prepend LIBERFA_EXPORT on all function signatures, like for example:
const char LIBERFA_EXPORT *eraVersion(void);

The macro definition - only valid of coure, when LIBERFA_SHARED is defined:

#if defined(_MSC_VER) || defined(__CYGWIN__) || defined(__MINGW32__) || defined( __BCPLUSPLUS__)  || defined( __MWERKS__)
#  if defined( LIBERFA_SHARED )
#    define LIBERFA_EXPORT   __declspec(dllexport)
#  else
#    define LIBERFA_EXPORT
#  endif
#else
#  define LIBERFA_EXPORT
#endif

This is, how I implemented my Windows build of ERFA shared library.

BR,

Ladislav

[avalentino]

@avalentino - you earlier volunteered to add the one missing thing, which I think was short installation instructions for on windows. Would you have time to help wrap this up?

@mhvk, actually I do not have too much time n this period but, as I have already said, I'm very interested in having a new build system so I will try to help as much as I can.
Honestly I was no longer sure that there was still interest in having CMake as a build system after the merge of the PR introducing meson. Do we really want to have 3 different build systems or, maybe, it is better to focus on meson that is already in? (unfortunately I'm not as good with meson as with CMake but I could still try to help)

Regarding the windows question I have 2 comments:

1. unfortunately I do not have access to windows, so I cannot help to much on that front
2. it seems to me that, before implemented any change in the build system, we need a change in the erfa-fetch tool to introduce the new C macro and a re-import of the source code, and that we can go on with the job ob the build system. Probably it is better to have a distinct PR to update the sources.

Finally, if we decide to go on with this PR, please let me know if there is a branch on which we/I can work on, otherwise I will create one on my fork.

cheers

[ajtribick]

@mhvk - for my original use case of getting this to build on Windows, Meson is definitely capable of meeting that requirement and had it been in the project when I raised this I would have done this PR as a Meson update instead. My general feeling is that when it comes to CMake vs Meson, a project should pick one rather than trying to maintain both. Given this project has now picked Meson, this feels much less like a useful change to me.

[mhvk]

OK, @laheller - would meson work for you too? It does seem we should just have one system (and it is a bit sad that I even forgot we had changed to meson!).

[laheller]

OK, @laheller - would meson work for you too? It does seem we should just have one system (and it is a bit sad that I even forgot we had changed to meson!).

@mhvk
Sadly, I have zero experience with meson, but maybe it's time to learn new things...

[mhvk]

@laheller - now that I was reminded of meson, I did also remember we actually test this on windows as part of CI, so that should give a sense of what would be needed: all the run pieces from

erfa/.github/workflows/ci_workflows.yml

Lines 77 to 94 in dcef368

	Windows-meson:
	runs-on: windows-latest
	steps:
	- uses: actions/checkout@v2
	with:
	fetch-depth: 0
	- name: install dependencies
	run: pip install --pre meson ninja
	- uses: ilammy/msvc-dev-cmd@v1
	- name: configure
	run: meson setup builddir --fatal-meson-warnings -Ddefault_library=static
	- name: build
	run: ninja -C builddir
	- name: check
	run: meson test -C builddir
	- name: distcheck
	run: meson dist -C builddir

[avalentino]

@mhvk since we now have meson, probably we can close this PR
Do you agree?

[mhvk]

Yes, indeed, meson should be good enough...