conan-io / conan-center-index

Recipes for the ConanCenter repository
https://conan.io/center
MIT License
948 stars 1.73k forks source link

[package] boost/1.74.0: broken package_info when i18n_backend=icu #3895

Closed ohanar closed 3 years ago

ohanar commented 3 years ago

Everything builds just fine, but then the components fail validation:

$ conan install boost/1.74.0@ -o boost:i18n_backend=icu --build boost
...
ERROR:
        ConanException: boost/1.74.0 package_info(): Package require 'libiconv' declared in components requires but not defined as a recipe requirement
ohanar commented 3 years ago

Seems to work fine when not including the locale module:

$ conan install boost/1.74.0@ -o boost:i18n_backend=icu -o boost:without_locale=True -o boost:without_log=True --build boost
madebr commented 3 years ago

Can you check my pr? I think boost is unable to look for a static icu (and never has). Can you please try the following?

conan create recipes/boost/all boost/1.74.0@ -o boost:i18n_backend=icu -o icu:shared=True

This fails:

conan create recipes/boost/all boost/1.74.0@ -o boost:i18n_backend=icu -o icu:shared=False
ohanar commented 3 years ago

For both icu being shared or static, I get the same error as above for the current recipe.

I get the following error for the pr's recipe when icu is static after boost builds:

ERROR:
        ConanException: Component 'log' required components not found in this package: 'locale'

icu's shared vs static configuration should probably be checked in boost's configure method and raised there.

madebr commented 3 years ago

Oh, it works for me with a shared icu (with the boost from my pr #3872) Did you test that one?

The one from current master does not because it does not find iconv/icu. I think before boost components, locale was never built.

Indeed, it should throw an error when a static icu is used.

ohanar commented 3 years ago

Sorry, yes I did test a shared icu -- that's our default, so that's what gets tried first. Before components you definitely had locale when icu was shared.

madebr commented 3 years ago

Yes, that's the same behaviour I am seeing with #3872 The boost recipe in current master does not support the icu backend, so #3872 is needed.

datalogics-kam commented 3 years ago

I think boost is unable to look for a static icu (and never has).

We're using Boost.Locale with a static ICU to do some conversions in a plugin. I'll try our project and report back.

datalogics-kam commented 3 years ago

Built our product with the branch from #3872 and indeed Boost.Locale works with a static ICU.

madebr commented 3 years ago

@datalogics-kam Weird that you're claiming that locale is working with #3872. Maybe you're using the locale library in header-only mode?

My build log contained the following:

    - icu                      : no
    - icu (lib64)              : no
- Boost.Locale needs either iconv or ICU library to be built.
- Boost.Locale needs either iconv or ICU library to be built.

which means boost could not build a test executable with icu.

Only 5 minutes ago, I pushed a commit to #3872 that allows building (and testing) the Boost.Locale library.

@ohanar Please retest! :smile:

datalogics-kam commented 3 years ago

Maybe you're using the locale library in header-only mode?

@madebr I'm pretty sure it's linking; we were tracking code size changes when using ICU for Unicode case conversions. That's not to say that some other aspect of our project made it work.

Tested again with the updates to #3872, and it works for us, still!

madebr commented 3 years ago

@datalogics-kam Is your project Linux or Macos based? Because the thing that made icu failing was a missing -ldl, which is Linux specific.

datalogics-kam commented 3 years ago

@madebr Ah! It's macOS/Window/Linux and I was testing on macOS (primary development platform for me). Yes, we had to add

boost:extra_b2_flags=cxxstd=11 linkflags=-ldl

to our Conan profiles for Linux so that Boost could successfully probe for ICU. If this fixes that, it's a nice win!

ETA: ...especially since our version of Artifactory fails to index packages that have options with = in them...

madebr commented 3 years ago

@datalogics-kam Yes, this c++ standard thing is also something that makes building boost libraries fail. The new boost.json library only builds on c++11. Do you happen to know what boost library needs the cxxstd=11? This silent disabling of building libraries is irritating imho. Boost.Build says it is enabled, but in reality it is not built.

ohanar commented 3 years ago

@madebr Yes, the error that I saw before with a static icu now has now been resolved.

datalogics-kam commented 3 years ago

Do you happen to know what boost library needs the cxxstd=11?

@madebr Boost.Locale needs that to use ICU as a backend.

b2 probes for the presence of ICU by compiling and running a small program. (Autotools and CMake do similar things.) If the program builds and runs, then it knows ICU is there. Recent versions of ICU require C++11 to build, and as you have found, -ldl to link on Linux.

If the small testing program fails to build or run for any reason, then b2 assumes ICU isn't there (even if it is), and simply turns off the building of Boost.Locale. In our project, we'd find that out when the project doesn't link due to missing Boost.Locale symbols.

cxxstd=11 causes b2 to issue the appropriate flags to the C++ compiler to bring it up to that standards level. gcc defaults to gnu++98, and clang defaults to C++98.

madebr commented 3 years ago

Are you sure this is for boost locale? Isn't this for some other library? Boost.Locale's JamFile does not contain anything c++11 specific. The ICU test also does not do anything fancy.

datalogics-kam commented 3 years ago

Are you sure this is for boost locale? Isn't this for some other library?

Yes, I'm sure of that. We had to set cxxstd=11 linkflags-ldl to use Boost.Locale with boost:i18n_backend=icu.

Boost.Locale's JamFile does not contain anything c++11 specific.

The cxxstd feature is global in b2, so it applies to all the Boost modules.

It's true that Boost.Locale doesn't specifically need C++11, it's more that it's a pass-through requirement of using ICU.

The ICU test also does not do anything fancy.

That's correct, the test is simple, but ICU requires C++ 11 since version 59, and the headers don't compile with an earlier standards level.

madebr commented 3 years ago

I found it weird because neither the icu recipe neither boost checks for anything c++11 related.

theodelrieu commented 3 years ago

Just hit this one, what is the current status of this issue?

madebr commented 3 years ago

See #3872 I'm trying to get it past c3i :smile: