Replace internal dependencies by FetchContent

[DerNils-git]

Replace internal dependencies management by CMake FetchContent commands.

Advantage:

• Updating the dependencies is simply done by updating the version/hash used within FetchContent_Declare which makes it easier to stay at the project HEAD of dependencies for project maintainers
• Less CMake options that users have to take care about while keeping the same flexibility

Required changes:

• Increase CMake minimum required version

Dependencies still have to be updated:

• TOML11 requires CMAKE_CXX_STANDARD to be set if the latest hash shall be used
• Catch2 requires to be moved from v2 to v3 which requires additional source changes.

If the pull request is accepted I would open issues to update the dependencies mentioned above and assign them to me.

(Sorry about the slightly chaotic git history. The change to FetchContent is done from 0ba14dc to 0084e9b. The previous commits resulted from a previous try to move to FetchContent which failed and the commits are reverted)

[franzpoeschel]

We should be aware that with this, the build folder will modify the source tree.
What does the workflow look like switching between different openPMD versions that specify different dependency versions. Will FetchContent notice and update the dependencies' versions upon configuring?

[DerNils-git]

That is true and might be a problem. I don't have experience with this.

Moving from lower to higher versions of OpenPMD should not be a problem. These directories have been deleted using git rm and should therefore be deleted when moving forward. FetchContent can then download into these directories w/o problem.

A problem might arise if one moves from a higher to a lower version. If the directories have been populated by FetchContent and git tries to add the old libraries again in the same directories I can not predict the behavior.

It would be possible to use a different directory with FetchContent until we are sure that it works as expected.

[franzpoeschel]

Since @ax3l uses FetchContent in WarpX, let's wait until he is back to see what their approach looks like (relevant bits in their CMake config: https://github.com/ECP-WarpX/WarpX/blob/11e2a1722aac8929db0ed677f634673d235c1396/cmake/dependencies/openPMD.cmake#L29)

[franzpoeschel]

You mean the warning below? Do I understand correctly that this has been fixed on toml11 dev and we are waiting for the next release?

CMake Deprecation Warning at share/openPMD/thirdParty/toml11/CMakeLists.txt:1 (cmake_minimum_required):                                                                                                                    
  Compatibility with CMake < 3.5 will be removed from a future version of                                                                                                                                                  
  CMake.                                                                                                                                                                                                                   
                                                                                                                                                                                                                           
  Update the VERSION argument <min> value or use a ...<max> suffix to tell                                                                                                                                                 
  CMake that the project does not need compatibility with older versions. 

[DerNils-git]

Yes.
But also moving to the last commit is not trivial with toml11 since that requires CMAKE_CXX_STANDARD to be set which I could not add to openPMD-api w/o breaking the CI.

[franzpoeschel]

since that requires CMAKE_CXX_STANDARD to be set which I could not add to openPMD-api w/o breaking the CI.

That's probably an issue unrelated to this PR which we should better solve separately?

[DerNils-git]

Yes, after finishing this PR I would try to Update catch2 and toml11 with separate issues as described above

[DerNils-git]

Is it to be expected that one of the CI stages fails? The error in this step does not seem to be related to the changes of this pull request.

[franzpoeschel]

Is it to be expected that one of the CI stages fails? The error in this step does not seem to be related to the changes of this pull request.

The failure comes from the increased minimum CMake version:

CMake Error at CMakeLists.txt:3 (cmake_minimum_required):
  CMake 3.24.0 or higher is required.  You are running version 3.22.1

The other "red text" seems to be unrelated (I see it on other PRs also), but does not cause a failure

Is version 3.22 also fine as a minimum? Otherwise, I'll push a commit that tries bumping the minimum CMake version of that CI run.

[DerNils-git]

Yes, sorry I was confused by the first red section in that action.

CMake 3.24 was the lowest version which still works with the implementation that I provided here. Using a lower CMake Version requires to remove the OVERIDE_FIND_PACKAGE argument in FetchContent.

[franzpoeschel]

It seems like we need to consider saying good-bye to the Win32 CI runner. The last updates in https://repo.anaconda.com/pkgs/main/win-32/ were in 2022, the latest included CMake version is 3.22.1 from Dec 7, 2021, hence the failure.

[franzpoeschel]

Using a lower CMake Version requires to remove the OVERIDE_FIND_PACKAGE argument in FetchContent.

After an offline discussion, this might be the preferred way to go. Even if Win32 is end of life, the runners are still useful as they are a good way to check our handling of datatypes, also Win32 is still being used in lab settings.
Given that this is a program library. we should be careful about raising the required CMake version to something released only very recently, if it can be avoided.

[DerNils-git]

Is it possible to trigger the failed CI again. In the last run it seems that there was an internal error since the directory in which openPMD should have been placed already existed.

[franzpoeschel]

Is it possible to trigger the failed CI again. In the last run it seems that there was an internal error since the directory in which openPMD should have been placed already existed.

I think the Appveyor part of our CI has no way to trigger a selective restart
I force-pushed to give it a second chance

[DerNils-git]

@ax3l is there any chance to get direct access to the system on which the CI is failing? That would be easier than debugging this error by additional commits to this pull request.
Unfortunately I could not reproduce this error locally.

[franzpoeschel]

I can reproduce the same error an a Linux system using CMake 3.22.1
This might be easier for investigating, I'll have a look

[franzpoeschel]

The problem is that CMake 3.22 does not correctly interpret the SOURCE_DIR param on FetchContent_Declare, leaving empty directories. This leads ot a rather cryptic failure in FetchContent_MakeAvailable.
Seems fixed with CMake 3.23. I'd say, we can either use the SOURCE_DIR param only in CMake 3.23 or later, alternatively leave it out altogether.

[DerNils-git]

We could also just depricate the SOURCE_DIR keyword. I hoped that FetchContent skips the download if the directory is already populated. Unfortunately this does not work. Therefore adding the SOURCE_DIR does not give any advantage and we could also just use the CMake default directories for FetchContent.