Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix standard default installation target presence #3264

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

FrancoisCarouge
Copy link
Contributor

Presently, for usage, installation, and packaging of the fmt library as transient dependency from third party CMake project, the extraneous FMT_INSTALL option needs to be set, this option is unexpected, needs to be discovered by users.

FetchContent_Declare(
  fmt
  GIT_REPOSITORY "https://github.com/fmtlib/fmt"
  FIND_PACKAGE_ARGS NAMES fmt)

# Note the non-standard required configuration option setting for installing, packaging the dependency:
option(FMT_INSTALL "Enable installation for the {fmt} project." ON)

FetchContent_MakeAvailable(fmt)

The CMake installation target for the fmt library should be present by default to conform to CMake defaults. Standard installation support simplifies usage in projects. The presence of the target, by default, allows standard CMake transient usage, installation, and packaging of the fmt library in other projects.

The extraneous configuration option can now be removed from projects using the library. Transient usage, packaging, installation follows. It simplifies usage. Users need not discover this option anymore. The library works like others. It would not override pre-installed, or pinned fmt library versions by downstream projects from CMake's "first to record, wins" approach which is what allows hierarchical projects to have parent projects override content details of child projects. Now, regular usage just works:

FetchContent_Declare(
  fmt
  GIT_REPOSITORY "https://github.com/fmtlib/fmt"
  FIND_PACKAGE_ARGS NAMES fmt)
FetchContent_MakeAvailable(fmt)

Reference: https://cmake.org/cmake/help/latest/module/FetchContent.html

@vitaut vitaut merged commit 2c80ced into fmtlib:master Jan 11, 2023
@vitaut
Copy link
Contributor

vitaut commented Jan 11, 2023

Thanks for the PR. I think it's reasonable to be consistent with CMake defaults.

xkszltl added a commit to cyfitech/cris-core that referenced this pull request Jun 3, 2024
Matan-B added a commit to Matan-B/ceph that referenced this pull request Aug 14, 2024
See: fmtlib/fmt#3264

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
Matan-B added a commit to ceph/ceph-ci that referenced this pull request Aug 15, 2024
See: fmtlib/fmt#3264

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants