Skip to content

cmake: Introduce Libmultiprocess_INSTALL_PKGCONFIG_FILE build option#112

Closed
hebasto wants to merge 1 commit intobitcoin-core:masterfrom
hebasto:240923-skip-pc
Closed

cmake: Introduce Libmultiprocess_INSTALL_PKGCONFIG_FILE build option#112
hebasto wants to merge 1 commit intobitcoin-core:masterfrom
hebasto:240923-skip-pc

Conversation

@hebasto
Copy link
Copy Markdown
Member

@hebasto hebasto commented Sep 23, 2024

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

Comment thread CMakeLists.txt
Comment on lines +94 to +95
configure_file(pkgconfig/libmultiprocess.pc.in "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc" @ONLY)
install(FILES "${CMAKE_CURRENT_BINARY_DIR}/pkgconfig/libmultiprocess.pc"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the pkgconfig subdirectory seems redundant here, no?

@fanquake
Copy link
Copy Markdown
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.

This change makes the installation of the `pkgconfig/libmultiprocess.pc`
file optional.
@hebasto
Copy link
Copy Markdown
Member Author

hebasto commented Sep 23, 2024

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
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Member Author

hebasto commented Sep 24, 2024

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
Copy link
Copy Markdown
Collaborator

sedited commented Jan 5, 2025

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

I think I would prefer that.

@hebasto hebasto closed this Jan 5, 2025
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jan 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants