Refactor EVE's exported CMake target and installation

[justend29]

Hello!

While using EVE for the first time via CMake, I noticed a few points of friction. Instead of creating a GitHub issue, I thought it would be most beneficial to arrive with solutions instead of problems. As this is my first PR into EVE, I've only made a draft PR.

The CMake issues were as follows

• EVE installs its CMake config-files into a non-default CMake path, directly in /usr/lib, preventing consuming libraries, including those installed by my system package manager, from finding it without explicitly specifying its path. CMake's search paths are specified in their documentation, here
• The exported target for eve, eve_lib, is different than that created in the build-tree, eve::eve.
• Furthermore, exported CMake targets are routinely exported with a namespace that ends in double colons (::). These are unacceptable in a path name, and will therefore limit CMake to finding CMake targets, with all of their interface requirements, instead of the compiled library or arbitrary library with the same name, eve_lib in this case.

Furthermore, I noticed a comment in EVE's installation CMake module that mentioned it was installing EVE to versioned directories. It wasn't actually doing this

Changes

EVE....

• installs its config-file into a path within CMake's search path (specifically, a versioned path)
• exports a target named eve::eve, to align with the internal target and to retain the ::.
• installs all files into versioned paths, allowing multiple simultaneously installed EVE versions
• has new usage documentation to reflect usage through CMake
  • the sample CMake configure+build commands were also updated to be build-system agnostic
  • documentation was built (a6c3634 contains all of the doxygen artifacts and can therefore be easily removed if there's an official procedure for this)
• 's integration test has been updated for the new target & install
• installs its LICENSE.md (+1)

Testing

I've tested installing EVE in both default and custom installation locations. In both of these, a separate project and the integration test within EVE were able to find, link, and use EVE with the CMake language commands added to the documentation.

I've also gone through the procedure of building and running the unit.meta.exe unit test in the CI docker container detailed in the internal documentation. This was mostly a smoke test to ensure I didn't completely bork EVE. Running the full CI suite will be a better indication of this.

[jfalcou]

Hi!

Thanks for this very thorough PR! We've been eye-balling our CMake install process for a while and I am very happy someone with proper skills took a look at it.

Some points to go forward:

• we usually don't PR the HTML output of the doc until the very end. We also require to use trunk Doxygen cause some of our C++ needs a really recent Doxygen version. The best is to leave it out and we'll take care of updating the docs ourselves.
• the interesting tests should be the ones in tests/integration as they "play along" faking an installation/fetch from GA+FetchContent etc...

I think that once you clean up the docs/ part, we can run that as an actual PR and see how it goes.
I'll add more comments directly in the PR code.

[DenisYaroshevskiy]

Awesome that you took the time, thanks. @jfalcou will do the review - I don't know any of this, just wanted to say thanks.

[justend29]

@jfalcou @DenisYaroshevskiy Thanks for the quick look over!

As for the Doxygen documentation, I have removed all of the generated Doxygen artifacts from this PR and let you handle generation in the most appropriate manner.

The CI failures for these seem to come from pulling the export-install branch, but I really haven't sufficiently looked. I'll get back to the FetchContent and CPM integration tests after work.

[jfalcou]

The CPM/FetchContent fails are probably due to the fact I try to fetch the from our repo. So whenever a PR from outside is made, it can't find the branch on jfalcou/eve. I probably need to grab the proper git ref for the PR and pull that.

[justend29]

I've made the PR official so we can review the code in the meantime, and then we can address the CI checks, dealing with pulling proper git refs, after.

[jfalcou]

So yeah the Fetch and CPM tests fail because we do stuff like

FetchContent_Declare(eve
  GIT_REPOSITORY https://github.com/jfalcou/eve.git
  GIT_TAG        ${EVE_SHA1}
  )

EVE_SHA1 beign passed by the Github Action as github.head_ref
So I guess we need to pass that + another macro to grab the actual repo the PR is coming from.
github.repositoryUrl maybe but I fear it'll resolve as jfalcou/eve anyway.

[justend29]

@jfalcou I've rebased this branch on #1336 to combine the integration tests updates, specifically for the CI integration tests for this forked PR. Although those are now working, all of the other GitHub Actions CI checks are now failing due to a weird permission error within the CI environment.

Whenever you happen to be free, could you please take a peek at their details to see if you recognize the issue?

[jfalcou]

Whenever you happen to be free, could you please take a peek at their details to see if you recognize the issue?

Oh something probably stopped our runner. I'll check if they need to be rebooted or something. That's unrelated to what you did.

[jfalcou]

Very last comment before merging

[jfalcou]

let's be pedantic for 0.5ns.
Can we remove that noisy commit? I guess this new line is accidental.

[justend29]

Being pedantic is good.

I actually placed that line there purposefully to change a Dockerfile, causing rebuilding of the docker image. I did this in an attempt to fix those mysterious CI errors. It didn't work.

I will clean up!

[jfalcou]

Should we remove the 3.12 remark ?

[justend29]

Since we are now saying 3.19 is the minimum, yes. Thanks!

[jfalcou]

Didn't we had a EVE_BUILD_INTEGRATION entry at some point or did I dreamt too deep ?

[justend29]

Yes, that option was added as part of #1336 to enable the integration tests that are now registered with CTest. Thanks for catching my not updating this documentation section to match! I'll do that.

[justend29]

I added these docs and made an aggregated integration target to run the integration tests. This target was really just to have a target to put in the table, though ;)

[jfalcou]

Thanks a lot for this amazing contribution!

[DenisYaroshevskiy]

Good work, @justend29
Thanks for taking the time

[justend29]

Happy to help, guys! Thanks for the amazing project!