Problem 2556

Summary:	Changes to FindEXPAT in CMake 3.27 break Geant4PackageCache.cmake file
Product:	Geant4	Reporter:	Ben Morgan <Ben.Morgan>
Component:	cmake	Assignee:	Ben Morgan <Ben.Morgan>
Status:	CLOSED FIXED
Severity:	normal	CC:	greenc
Priority:	P4
Version:	11.1
Hardware:	All
OS:	All
Attachments:	Patch to support package cache output with CMake >= 3.27

Description Ben Morgan 2023-07-28 16:02:05 CEST

Initially discovered through a post on the Forum: https://geant4-forum.web.cern.ch/t/the-install-of-geant4-v11-2-0-beta/10883 and subsequently confirmed locally. CMake 3.27 introduced a change to its `FindEXPAT.cmake` module that no longer stores the `EXPAT_LIBRARY` variable in the cache:

https://gitlab.kitware.com/cmake/cmake/-/commit/13079f382b480c8fe6a68d98afc2cec716a75e59

Configuring Geant4 with CMake 3.27 will result in empty values for `EXPAT_LIBRARY` in the package cache file and thus errors from `find_package(Geant4)` like:

```
CMake Error at ...Geant4PackageCache.cmake:19 (geant4_set_and_check_package_variable):
  geant4_set_and_check_package_variable Macro invoked with incorrect
  arguments for macro named: geant4_set_and_check_package_variable
```

Temporary workarounds are either to: 

- not install the package cache file by adding `-DGEANT4_INSTALL_PACKAGE_CACHE=OFF` to the Geant4 CMake arguments
- delete `Geant4PackageCache.cmake` from an existing install if and only if all of the paths in that file are to locations known to CMake
- add the location of the expat library to its entry

In the last case, the offending line will look like:

```
geant4_set_and_check_package_variable(EXPAT_LIBRARY ""  "")
```

and should be updated to:

```
geant4_set_and_check_package_variable(EXPAT_LIBRARY "/path/to/your/libexpat.so" PATH "path to expat lib")
```

Patches for Geant4 10.7 through to 11.2 will be posted here when tested.

Comment 1 Ben Morgan 2023-08-24 14:50:57 CEST

Created attachment 821 [details]
Patch to support package cache output with CMake >= 3.27

The attached patch can be applied cleanly to Geant4 10.7 through to 11.2-beta. It has been tested on each version with various combinations of build/use time CMake versions albeit not exhaustively.

It is merged to the development branch and so will be in the 11.2 release, with application to preceeding versions in the next patches to these releases.

Comment 2 Ben Morgan 2023-08-24 14:51:42 CEST

Will leave assigned until patches are fully deployed.

Comment 3 greenc 2023-11-28 16:22:27 CET

After reading the CMake code here, I'm pretty sure that saving `EXPAT_LIBRARY` (or any other variable produced by `find_package(dep...)` is unnecessary because of the use of `find_dependency()` in the Config file itself.

Comment 4 Ben Morgan 2023-11-28 16:37:30 CET

It's necessary(*) for the use case where the needed packages are in locations that CMake may not know about after package install. For example, `CMAKE_PREFIX_PATH` is set at install time with a load of non-standard locations, but not at application build time. A packaging system will almost certainly handle this, but custom/personal installs may not.


(*) They are strong arguments on both sides to delete/retain the package cache functionality, hence the default to install it (to help novice/inexperienced users), but switch that off if required.

Comment 5 Ben Morgan 2023-12-20 21:29:28 CET

Closing as resolved as deployed in 11.2 and 11.1 patches. Can refer back to this in future for older releases if needed.