cmake: Introduce `Libmultiprocess_INSTALL_PKGCONFIG_FILE` build option #112

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:240923-skip-pc changing 1 files +7 −3

hebasto commented at 1:07 PM on September 23, 2024: member

This PR makes the installation of the pkgconfig/libmultiprocess.pc file optional.

in CMakeLists.txt:95 in 39aa7ea85e outdated

  89 | @@ -88,9 +90,11 @@ install(FILES "include/mpgen.mk"
  90 |    DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} COMPONENT bin)
  91 |  
  92 |  # pkg-config module to build against libmultiprocess library, for downstream autoconf projects
  93 | -configure_file(pkgconfig/libmultiprocess.pc.in "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc" @ONLY)
  94 | -install(FILES "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc"
  95 | -  DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig COMPONENT lib)
  96 | +if(Libmultiprocess_INSTALL_PKGCONFIG_FILE)
  97 | +  configure_file(pkgconfig/libmultiprocess.pc.in "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc" @ONLY)
  98 | +  install(FILES "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc"

hebasto commented at 1:11 PM on September 23, 2024:

Also, the pkgconfig subdirectory seems redundant here, no?

fanquake commented at 1:16 PM on September 23, 2024: member

Is there a particular reason to not install it by default? Note that pkg-config is just a generic tool, and it's usage is not specific to Autotools. Meson and other build systems also use it.

cmake: Introduce `Libmultiprocess_INSTALL_PKGCONFIG_FILE` build option

This change makes the installation of the `pkgconfig/libmultiprocess.pc`
file optional.

f8eea25e3a

hebasto force-pushed on Sep 23, 2024
hebasto commented at 1:34 PM on September 23, 2024: member

Is there a particular reason to not install it by default? Note that pkg-config is just a generic tool, and it's usage is not specific to Autotools. Meson and other build systems also use it.

The default value of the new option has been changed to ON.
ryanofsky commented at 4:28 PM on September 23, 2024: collaborator

What's the motivation for this change? It seems like an unusual thing for a package to have a configuration option to not fully install itself. And if installing a .pc file is optional, should installing .cmake package files be optional also? I think I'd prefer the cmake code here to be as vanilla as possible and not have a non-standard option like this, unless there is a real reason why it's useful.

Also it seems would seem better to just use cmake components to control what is installed instead of using both cmake components and cmake options to control what is installed. Maybe installed metadata files could be moved into different components to provide finer-grained control over what to install if this is necessary.

hebasto commented at 10:40 AM on September 24, 2024: member

What's the motivation for this change?

Currently, the package built in Bitcoin Core depends looks like that:

$ tree
.
├── include
│   └── mp
│       ├── proxy.capnp.h
│       ├── proxy.h
│       ├── proxy-io.h
│       ├── proxy-types.h
│       └── util.h
└── lib
    ├── cmake
    │   └── Libmultiprocess
    │       ├── LibmultiprocessConfig.cmake
    │       ├── LibTargets.cmake
    │       └── LibTargets-noconfig.cmake
    ├── libmultiprocess.a
    └── pkgconfig
        └── libmultiprocess.pc

7 directories, 10 files

The pkgconfig/libmultiprocess.pc file is not needed. Alternatively, it can be deleted during postprocessing, as we do with other packages.

sedited commented at 8:24 PM on January 5, 2025: collaborator

Alternatively, it can be deleted during postprocessing, as we do with other packages.

I think I would prefer that.
hebasto closed this on Jan 5, 2025
bitcoin-core locked this on Jan 5, 2026

Contributors