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.

Sign up for GitHub

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

Jump to bottom

cli11: Compile by default, add header_only option #25741

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

cli11: Compile by default, add header_only option #25741

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa865d9
CLI11: Compile by default, add header_only option
vasama Oct 27, 2024
4453129
Fix package_type handling
AbrilRBS Oct 28, 2024
f3f4260
Better package_type handling
AbrilRBS Oct 28, 2024
23540de
Update recipes/cli11/all/conanfile.py
AbrilRBS Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions recipes/cli11/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from conan.tools.files import get, copy, rmdir
from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout
from conan.tools.build import check_min_cppstd
from conan.tools.scm import Version
import os

required_conan_version = ">=1.52.0"
Expand All @@ -12,20 +13,40 @@ class CLI11Conan(ConanFile):
license = "BSD-3-Clause"
url = "https://github.com/conan-io/conan-center-index"
homepage = "https://github.com/CLIUtils/CLI11"
topics = "cli-parser", "cpp11", "no-dependencies", "cli", "header-only"
topics = "cli-parser", "cpp11", "no-dependencies", "cli"
package_type = "header-library"
settings = "os", "arch", "compiler", "build_type"
no_copy_source = True

options = {
"header_only": [True, False]
}
default_options = {
"header_only": False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"header_only": False
"header_only": True

Keep header-only as True, so the Package ID will be the same as for previous versions.

Copy link
Author

Choose a reason for hiding this comment

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

That really sucks because header only builds are very undesirable. It ends up breaking unrelated code due to leaked implementation details like included system headers. That's what prompted me to create the issue and PR in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

The main issue here is that upstream also defaults to a single-header installation, and the compilation behaviour is opt-in, so with this we would be following the choices on upstream

Copy link
Author

Choose a reason for hiding this comment

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

It's a very user hostile default though, only motivated by ease of distribution, which is hardly a concern if one is using Conan.

Copy link
Member

Choose a reason for hiding this comment

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

@vasama You still can build using the option that you need in your configuration as well. In CCI we mostly follow upstream default behavior, as it will affect all users and will reflect the upstream build as well.

Copy link
Author

Choose a reason for hiding this comment

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

I'm aware of that. I just want to make it clear that header only is a bad and pointless default.

}

@property
def _min_cppstd(self):
return "11"

@property
def _supports_compilation(self):
return Version(self.version) >= 2.3
AbrilRBS marked this conversation as resolved.
Show resolved Hide resolved

def config_options(self):
if not self._supports_compilation:
del self.options.header_only

def configure(self):
if self._supports_compilation and not self.options.header_only:
self.package_type = "static-library"

def layout(self):
cmake_layout(self, src_folder="src")

def package_id(self):
self.info.clear()
if not self._supports_compilation or self.info.options.header_only:
self.info.clear()

def validate(self):
if self.settings.compiler.get_safe("cppstd"):
Expand All @@ -36,6 +57,8 @@ def source(self):

def generate(self):
tc = CMakeToolchain(self)
if self._supports_compilation:
tc.variables["CLI11_PRECOMPILED"] = not self.options.header_only
tc.variables["CLI11_BUILD_EXAMPLES"] = False
tc.variables["CLI11_BUILD_TESTS"] = False
tc.variables["CLI11_BUILD_DOCS"] = False
Expand All @@ -56,8 +79,12 @@ def package(self):
rmdir(self, os.path.join(self.package_folder, "share"))

def package_info(self):
self.cpp_info.bindirs = []
self.cpp_info.libdirs = []
if self.options.get_safe("header_only"):
self.cpp_info.bindirs = []
self.cpp_info.libdirs = []
else:
self.cpp_info.libs = ["CLI11"]
self.cpp_info.defines = ["CLI11_COMPILE"]

self.cpp_info.set_property("cmake_file_name", "CLI11")
self.cpp_info.set_property("cmake_target_name", "CLI11::CLI11")
Expand Down