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

[boost] Optionally use ICU #1274

Merged
merged 7 commits into from
Apr 23, 2020
Merged
Changes from 6 commits
Commits
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
12 changes: 11 additions & 1 deletion recipes/boost/all/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class BoostConan(ConanFile):
"segmented_stacks": [True, False],
"debug_level": [i for i in range(1, 14)],
"pch": [True, False],
"extra_b2_flags": "ANY" # custom b2 flags
"extra_b2_flags": "ANY", # custom b2 flags
"icu": [True, False],
Copy link
Contributor

Choose a reason for hiding this comment

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

are ICU and iconv mutually exclusive options?
if yes, maybe it's better to design option like locale_backend: ["ICU", "iconv", None]
this might be helpful: https://github.com/bincrafters/conan-boost_base/blob/testing/2.0.0/conanfile.py
/cc @grafikrobot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that iconv is more like a fallback, you can choose to use icu, but if it is not available, the package will use iconv. It also follows the same option pattern as zlib, zstd,... if you activate it then more requires will be added.

Copy link
Contributor

Choose a reason for hiding this comment

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

they are different. zlib, zstd, bzip2 backends can be used together, while iconv and ICU seems to be mutually exclusive.

}
options.update({"without_%s" % libname: [True, False] for libname in lib_list})

Expand All @@ -81,6 +82,7 @@ class BoostConan(ConanFile):
"debug_level": 2,
'pch': True,
'extra_b2_flags': 'None',
"icu": False,
}

for libname in lib_list:
Expand Down Expand Up @@ -133,6 +135,8 @@ def requirements(self):
self.requires("xz_utils/5.2.4")
if self.options.zstd:
self.requires("zstd/1.4.3")
if self.options.icu:
Copy link
Contributor

Choose a reason for hiding this comment

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

probably we have to check for without_locale option to avoid unneeded dependency in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ICU applies to regex and locale, not sure if you can apply ICU to one without the other. I really don't know how the transitivity and dependencies among Boost libraries work

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, seems to be check should be like:
if not without_locale or not without_regex:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a check. 'locale' doesn't compile without a i18n backend, but 'regex' does 🤷

self.requires("icu/64.2")

def package_id(self):
if self.options.header_only:
Expand Down Expand Up @@ -539,6 +543,12 @@ def _build_flags(self):
flags.append("-sNO_LZMA=%s" % ("0" if self.options.lzma else "1"))
flags.append("-sNO_ZSTD=%s" % ("0" if self.options.zstd else "1"))

if self.options.icu:
flags.append("-sICU_PATH={}".format(self.deps_cpp_info["icu"].rootpath))
flags.append("boost.locale.iconv=off boost.locale.icu=on")
else:
flags.append("--disable-icu")

def add_defines(option, library):
if option:
for define in self.deps_cpp_info[library].defines:
Expand Down