dolfinx icon indicating copy to clipboard operation
dolfinx copied to clipboard

Bump minimum CMake to 3.19 and introduce version ranges

Open jorgensd opened this issue 3 years ago • 4 comments

Cmake 3.19 added version ranges: https://cmake.org/cmake/help/latest/release/3.19.html back in november 2020. This would make it easier for DOLFINx extensions to specify requirements on DOLFINx.

For instance: v0.5.1 was just released with some bug fixes, and no changes in the API. I need to rewrite my CMakeLists.txt for dolfinx_mpc to support this change. With version ranges, I would be able to specify

find_package(DOLFINX 0.5...0.6 REQUIRED)

which would result in:

-- The following REQUIRED packages have been found:
 * DOLFINX (required version 0.5...0.6), New generation Dynamic Object-oriented Library for - FINite element computation, <https://github.com/FEniCS/dolfinx>

when using v0.5.1. For more rules regarding ranges see: https://cmake.org/cmake/help/latest/command/find_package.html#basic-signature

jorgensd avatar Sep 09 '22 10:09 jorgensd

@drew-parsons I think this will help when bumping different parts of the FEniCS eco-system, making it easier to provide minimal bounds (If we introduce this in Basix as well).

jorgensd avatar Sep 09 '22 10:09 jorgensd

This is probably safe. Would want to check conda support but almost certainly their cmake is up to date.

Ubuntu 20.04 focal has cmake 3.16. But we're already not able to (simply) build dolfinx 0.5 because of the use of C++20 features (std::span), not supported by gcc-9. So for that reason ubuntu 20.04 support is not an impediment (it will have to be left on dolfinx 0.4).

Might want to consider if we're happy with that ubuntu situation, or whether people building dolfinx manually on ubuntu 20.04 should still be supported. The policy is on a best-effort basis, so can instead just ask such users to upgrade to a more recent ubuntu release.

@francesco-ballarin might have an opinion on dropping old ubuntu support, because of the Google colab fiasco.

drew-parsons avatar Sep 09 '22 12:09 drew-parsons

@jorgensd @drew-parsons I am fine with dropping support for ubuntu 20.04, my FEM on Colab pipeline is very mildly dependent on upstream ubuntu packages: due (as you call it) to that fiasco I basically have to rebuild and package any dependency anyway in my own workflow.

I've just checked:

  • the cmake version in my workflow is 3.24.1 (i.e., the latest from kitware apt), so well above 3.19. Therefore, requiring 3.19 will not hinder my workflow.
  • the cmake version of an actual Colab instance is 3.22.6 (as of today, might change once in a while), so again above 3.19. Can you please remind me if JIT compilation actually uses cmake? I seem to remember that it was the case in dijitso, but if I remember correctly now JIT uses pkgconfig (and cppimport?) and never cmake. If that was the case, the end user would never have to use cmake on an actual Colab instance, unless they are doing something crazy (like deciding to compile the C++ demos on Colab, but what would be the point? ;) )

francesco-ballarin avatar Sep 09 '22 12:09 francesco-ballarin

It would also be good if DOLFINx explicitly specified it's dependency on UFCx through ranges rather than implicitly through its own version number.

PR now coupled with: https://github.com/FEniCS/ffcx/pull/535

I guess we want something similar in BASIX (we currently have no version requirement of basix, which I guess is wrong)? @garth-wells @mscroggs

jorgensd avatar Sep 14 '22 14:09 jorgensd

Summary of cmake: Assume that we have DOLFINx 0.5.2.0 installed with

write_basic_package_version_file(${CMAKE_BINARY_DIR}/dolfinx/DOLFINXConfigVersion.cmake
  VERSION ${DOLFINX_VERSION}
  COMPATIBILITY AnyNewerVersion)

in /cpp/dolfinx/CMakeLists.txt. Then:

  1. find_package(DOLFINX 0.4.0 REQUIRED) works, as we accept any newer version
  2. find_package(DOLFINX 0.5.3 REQUIRED) leads to an error, as we can only find version 0.5.2.0
  3. find_package(DOLFINX 0.6.0 REQUIRED) leads to the same error
  4. find_package(DOLFINX 0.5...0.6 REQUIRED) works
  5. find_package(DOLFINX 0.5.0...<0.6.0 REQUIRED) works
  6. find_package(DOLFINX 0.4.0...<0.5.0 REQUIRED) throws an error
  7. find_package(DOLFINX 0.4.0...0.5.0 REQUIRED) throws an error

Point 1. can be thought of as a lower bound on the version you can use, similar to how one usually set version requirements in Python. i.e. using find_package(0.4.0) is equal to version >= 0.4.0 if you want an upper bound, say within the same minor version, you would need to explicitly hard-code it (which at least from a pythonic viewpoint is sensible).

jorgensd avatar Nov 01 '22 09:11 jorgensd

I've just hit this problem in practice while trying to package @jorgensd 's dolfinx-mpi for debian. ExactVersion in cpp/dolfinx/CMakeLists.txt means that dolfinx 0.5.2 is not compatible with the dolfinx 0.5.1 requested by dolfinx-mpc, even when patched with a version range 0.5.1...<0.6 (note the < should be there, or else 0.6.0 itself would be considered compatible)

I think the patch to cpp/dolfinx/CMakeLists.txt should declare COMPATIBILITY SameMinorVersion not COMPATIBILITY AnyNewerVersion, though. I am assuming here that dolfinx 0.6 is ABI incompatible with 0.5.

The changes to the .yml files would need to be reverted before merging this PR.

drew-parsons avatar Dec 14 '22 19:12 drew-parsons

SameMinorVersion

@drew-parsons If we run with SameMinorVersion, one would force the user to use:

find_package(DOLFINX 0.6.0...<0.7.0 REQUIRED)

I.e. something like:

find_package(DOLFINX 0.6.0...0.7.0 REQUIRED)

is not accepted and throws:

CMake Error at CMakeLists.txt:93 (find_package):
  Could not find a configuration file for package "DOLFINX" that is
  compatible with requested version range "0.6.0...0.7.0".

  The following configuration files were considered but not accepted:

    /usr/local/dolfinx-real/lib/cmake/dolfinx/DOLFINXConfig.cmake, version: 0.6.0.0

I find it very strange that we force people to bump the version of their code whenever we release a version of DOLFINx. I would say it is up to the developer of the package depending on DOLFINx to make sure that their code is compatible with the latest release. There might be cases where DOLFINx has API changes, but they are not affecting the external package. @jhale @garth-wells @chrisrichardson your thoughts on this?

jorgensd avatar Dec 19 '22 11:12 jorgensd

I find it very strange that we force people to bump the version of their code whenever we release a version of DOLFINx.

You're absolutely right, this is the key point. Will the API be substantially changing in every 0.x release, requiring users to modify their existing code? Across 0.4 to 0.5, and 0.6, that has been the case, and I was proposing we'd want the 0.6...<0.7 variant (with SameMinorVersion), on the assumption that 0.7 would also introduce API changes.

At some point the API changes would slow down, at which point it would be time to make a 1.0 release. At that point the policy could change to SameMajorVersion assuming semantic versioning is employed. But I'd still argue then for SameMajorVersion rather than AnyNewerVersion.

If no major API changes are expected, but new major versions are wanted anyway (not semantic versioning), or if we want to just leave it to users to manage version compatibility themselves, then AnyNewerVersion is fine.

drew-parsons avatar Dec 19 '22 12:12 drew-parsons

You're absolutely right, this is the key point. Will the API be substantially changing in every 0.x release, requiring users to modify their existing code? Across 0.4 to 0.5, and 0.6, that has been the case, and I was proposing we'd want the 0..6...<0.7 variant (with SameMinorVersion), on the assumption that 0.7 would also introduce API changes.

Im not sure we should enforce this on the user. Some of the API changes every minor release, but it doesn’t necessarily mean that it will break every package based on DOLFINx. I guess we need to decide something. What do you think @jhale ?

jorgensd avatar Dec 19 '22 20:12 jorgensd

My understanding of the discussions so far is that we aim to use Semantic Versioning 2.0. That means that public API changes will automatically lead to a major version bump once we go past v1.0.0. At the moment, pre-v1.0.0 even minor version bumps signify a public API change.

Whether an API change actually breaks a downstream package is not important (e.g. they only use a limited subset of the API). It is for downstream users and developers to test and make that determination.

https://semver.org

jhale avatar Dec 20 '22 08:12 jhale

Whether an API change actually breaks a downstream package is not important (e.g. they only use a limited subset of the API). It is for downstream users and developers to test and make that determination.

If we want to go that route, I would suggest we use AnyNewerVersion, as that makes it up to the developer of the corresponding C++ library to set the correct version range.

jorgensd avatar Dec 20 '22 08:12 jorgensd

P.S. I don't see why downstream developers should be in anyway tied into to DOLFINx's version numbering.

If I want to have my downstream package at version 0.1 and it's compatible with DOLFINx 0.5...<0.7 then I should be able to do it?

jhale avatar Dec 20 '22 08:12 jhale