[boost] Optionally use ICU #1274

jgsogo · 2020-04-04T12:54:46Z

Specify library name and version: boost/all

Add option icu to use ICU library (regex and locale)

Closes #1270

This PR requires #1273 before, so ICU 66.1 is available.

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

conan-center-bot · 2020-04-04T20:46:09Z

All green in build 1 (f460ff55fdd78443ec2aabf9be95eb850c5d39e4)! 😊

boost/1.69.0: Generated 131 packages. All logs here
boost/1.70.0: Generated 131 packages. All logs here
boost/1.71.0: Generated 131 packages. All logs here
boost/1.72.0: Generated 131 packages. All logs here

SSE4 · 2020-04-06T07:18:14Z

recipes/boost/all/conanfile.py

@@ -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],


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

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.

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

SSE4 · 2020-04-06T07:19:05Z

recipes/boost/all/conanfile.py

@@ -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.icu:


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

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

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

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

jgsogo · 2020-04-06T07:51:59Z

Anyway, I really dislike one change in this PR:

    def configure(self):
        if self.options.icu:
            tools.check_min_cppstd(self, "11")

This doesn't belong to the Boost recipe but to the ICU one. Nevertheless we know we cannot add it to ICU because C3i won't generate any package... we need to prioritize this kind of thing.

recipes/boost/all/conanfile.py

SSE4

move the cppstd check into ICU

jgsogo

I've removed the c++11 check that belongs to the ICU library.

conan-center-bot · 2020-04-18T23:09:36Z

All green in build 2 (2b15d9a0990f0905b6f26fe5c4cc6088545183d6)! 😊

boost/1.69.0: Generated 131 packages. All logs here
boost/1.70.0: Generated 131 packages. All logs here
boost/1.71.0: Generated 131 packages. All logs here
boost/1.72.0: Generated 131 packages. All logs here

conan-center-bot · 2020-04-21T15:52:41Z

An unexpected error happened and has been reported. Help is on its way! 🏇

conan-center-bot · 2020-04-21T19:01:18Z

All green in build 4 (67086e295dd046d9352f625ea894024862bdd4e5)! 😊

boost/1.69.0: Generated 131 packages. All logs here
boost/1.70.0: Generated 131 packages. All logs here
boost/1.71.0: Generated 131 packages. All logs here
boost/1.72.0: Generated 131 packages. All logs here

SSE4 · 2020-04-22T07:59:12Z

recipes/boost/all/conanfile.py

@@ -81,6 +82,7 @@ class BoostConan(ConanFile):
        "debug_level": 2,
        'pch': True,
        'extra_b2_flags': 'None',
+        "i18n_backend": 'iconv',


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

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).

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?

Now I'm adding this check:

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)

jgsogo · 2020-04-22T13:42:07Z

@SSE4 but... will you approve this PR? 😄

datalogics-kam · 2020-07-20T20:28:05Z

recipes/boost/all/conanfile.py

+            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")


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

yes, seems so, feel free to submit PR with fix

pepsiman · 2020-08-18T13:44:20Z

All green in build 4 (67086e295dd046d9352f625ea894024862bdd4e5)!

boost/1.69.0: Generated 131 packages. All logs here

boost/1.70.0: Generated 131 packages. All logs here

boost/1.71.0: Generated 131 packages. All logs here

boost/1.72.0: Generated 131 packages. All logs here

None of these packages have ICU enabled, so they tell me nothing.

On Windows, boost's build system expects ICU to have both release and debug libraries.
On 64bit Windows, boost's build system expects ICU to have bin64 and lib64 directories.

I don't believe that this package of boost will use the package of ICU on Windows.

danimtb · 2020-08-25T11:50:28Z

@pepsiman you will have to set the option i18n_backend to icu in order to use the dependency. However, you will have to build from sources.

pepsiman · 2020-08-25T16:19:43Z

@danimtb Yes, but it doesn't build from sources on Windows.

danimtb · 2020-08-26T07:29:44Z

please open an issue with the build log so we are able to check, thank you

jgsogo added 5 commits April 4, 2020 12:59

boost with icu

16983b2

see if installed

350e87b

forbidden

9b40af4

use options.icu

fc9c7dd

remove comments

f460ff5

jgsogo mentioned this pull request Apr 4, 2020

Freeling installation on MacOS TALP-UPC/FreeLing#90

Open

SSE4 approved these changes Apr 5, 2020

View reviewed changes

SSE4 requested a review from uilianries April 5, 2020 06:12

SSE4 reviewed Apr 6, 2020

View reviewed changes

uilianries reviewed Apr 6, 2020

View reviewed changes

recipes/boost/all/conanfile.py Outdated Show resolved Hide resolved

SSE4 suggested changes Apr 6, 2020

View reviewed changes

make it mergeable

2b15d9a

jgsogo commented Apr 18, 2020

View reviewed changes

jgsogo requested review from SSE4 and uilianries April 18, 2020 16:34

use choicce option for i18n backend

67086e2

SSE4 reviewed Apr 22, 2020

View reviewed changes

SSE4 approved these changes Apr 22, 2020

View reviewed changes

uilianries approved these changes Apr 22, 2020

View reviewed changes

jgsogo assigned danimtb Apr 23, 2020

danimtb approved these changes Apr 23, 2020

View reviewed changes

danimtb merged commit b013b05 into conan-io:master Apr 23, 2020

jgsogo deleted the feat/boost-icu branch April 23, 2020 09:51

datalogics-kam reviewed Jul 20, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[boost] Optionally use ICU #1274

[boost] Optionally use ICU #1274

jgsogo commented Apr 4, 2020 •

edited

Loading

conan-center-bot commented Apr 4, 2020

SSE4 Apr 6, 2020

jgsogo Apr 18, 2020

SSE4 Apr 21, 2020

SSE4 Apr 6, 2020

jgsogo Apr 6, 2020

SSE4 Apr 21, 2020

jgsogo Apr 21, 2020

jgsogo commented Apr 6, 2020

SSE4 left a comment

jgsogo left a comment

conan-center-bot commented Apr 18, 2020

conan-center-bot commented Apr 21, 2020

conan-center-bot commented Apr 21, 2020

SSE4 Apr 22, 2020

jgsogo Apr 22, 2020

SSE4 Apr 22, 2020

jgsogo Apr 22, 2020

jgsogo commented Apr 22, 2020

datalogics-kam Jul 20, 2020

SSE4 Jul 21, 2020

pepsiman commented Aug 18, 2020

danimtb commented Aug 25, 2020

pepsiman commented Aug 25, 2020

danimtb commented Aug 26, 2020

[boost] Optionally use ICU #1274

[boost] Optionally use ICU #1274

Conversation

jgsogo commented Apr 4, 2020 • edited Loading

conan-center-bot commented Apr 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgsogo commented Apr 6, 2020

SSE4 left a comment

Choose a reason for hiding this comment

jgsogo left a comment

Choose a reason for hiding this comment

conan-center-bot commented Apr 18, 2020

conan-center-bot commented Apr 21, 2020

conan-center-bot commented Apr 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgsogo commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pepsiman commented Aug 18, 2020

danimtb commented Aug 25, 2020

pepsiman commented Aug 25, 2020

danimtb commented Aug 26, 2020

jgsogo commented Apr 4, 2020 •

edited

Loading