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

[boost] Optionally use ICU #1274

Merged
merged 7 commits into from
Apr 23, 2020
Merged

[boost] Optionally use ICU #1274

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
16983b2
boost with icu
jgsogo Apr 4, 2020
350e87b
see if installed
jgsogo Apr 4, 2020
9b40af4
forbidden
jgsogo Apr 4, 2020
fc9c7dd
use options.icu
jgsogo Apr 4, 2020
f460ff5
remove comments
jgsogo Apr 4, 2020
2b15d9a
make it mergeable
jgsogo Apr 18, 2020
67086e2
use choicce option for i18n backend
jgsogo Apr 21, 2020
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
19 changes: 18 additions & 1 deletion recipes/boost/all/conanfile.py
View file
Open in desktop
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
"i18n_backend": ["iconv", "icu", None],
}
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',
"i18n_backend": 'iconv',
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, so recipe has no dependency on libiconv, but it still builds with iconv backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, in Mac/Linux probably it is quite common to have it installed, boost finds it and uses it. If not found, and this could be a problem, boost won't build locale but it will succeed (there is a message in the output, nothing else).

Copy link
Contributor

Choose a reason for hiding this comment

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

I have doubts... might be not very reliable or reproducible, then sometimes locale won't be built, but sometimes will be. maybe you can add an actual reference to libiconv recipe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I'm adding this check:

image

still we would need a libiconv requires, but probably it is better for another PR, this one keeps it as it was before (regarding iconv)

}

for libname in lib_list:
Expand Down Expand Up @@ -120,6 +122,10 @@ def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC

def configure(self):
if not self.options.i18n_backend and not self.options.without_locale:
raise ConanInvalidConfiguration("Boost 'locale' library requires a i18n_backend, either 'icu' or 'iconv'")

def build_requirements(self):
self.build_requires("b2/4.2.0")

Expand All @@ -133,6 +139,8 @@ def requirements(self):
self.requires("xz_utils/5.2.4")
if self.options.zstd:
self.requires("zstd/1.4.3")
if self.options.i18n_backend == 'icu':
self.requires("icu/64.2")

def package_id(self):
if self.options.header_only:
Expand Down Expand Up @@ -539,6 +547,15 @@ 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.i18n_backend == 'icu':
flags.append("-sICU_PATH={}".format(self.deps_cpp_info["icu"].rootpath))
flags.append("boost.locale.iconv=off boost.locale.icu=on")
elif self.options.i18n_backend == 'iconv':
flags.append("boost.locale.iconv=on boost.locale.icu=off")
else:
flags.append("boost.locale.iconv=off boost.locale.icu=off")
flags.append("--disable-icu --disable-iconvv")
Copy link
Contributor

Choose a reason for hiding this comment

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

I was just looking at this recipe, and noticed this. Is --disable-iconvv a typo? Should it be --disable-iconv?

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 so, feel free to submit PR with fix


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