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

[arrow:x64-linux] [brotli:x64-linux] Can not be used together. #9838

Open
takacsd opened this issue Jan 29, 2020 · 8 comments
Open

[arrow:x64-linux] [brotli:x64-linux] Can not be used together. #9838

takacsd opened this issue Jan 29, 2020 · 8 comments
Assignees
Labels
category:port-bug The issue is with a library, which is something the port should already support

Comments

@takacsd
Copy link

takacsd commented Jan 29, 2020

Describe the bug
I built arrow with vcpkg install arrow. It built everything fine, it built all the dependencies one of which is brotli.
Now I try to build an application using it. Since it is linux, the build is static, so I need to add all dependencies to my project. Which I can not...
Here is the issue:
arrow expects a target named Brotli::brotlienc:

# arrowTargets.cmake line 63:
set_target_properties(arrow_static PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
   INTERFACE_LINK_LIBRARIES "double-conversion::double-conversion;OpenSSL::Crypto;OpenSSL::SSL;Brotli::brotlienc;Brotli::brotlidec;Brotli::brotlicommon;LZ4::lz4;Snappy::snappy;ZLIB::ZLIB;ZSTD::zstd;GLOG::glog;boost_filesystem;boost_system;boost_regex;Threads::Threads;rt"
)

but brotli exports unofficial::brotli::brotlienc:

+ install(EXPORT brotli FILE unofficial-brotli-config.cmake NAMESPACE unofficial::brotli:: DESTINATION share/unofficial-brotli)

Environment

  • OS: Ubuntu 19.10
  • Compiler: g++ 9.2.1

To Reproduce
Steps to reproduce the behavior:

  1. ./vcpkg install arrow
  2. Build an application using it.

Expected behavior
Obviously it should just work. I don't know which other projects using Brotli, but it is safe to say they are also broken. If the Brotli:: namespace is generally used, I think vcpkg should use the same. If not, vcpkg should patch arrow to use the unofficial::brotli:: namespace.

@takacsd
Copy link
Author

takacsd commented Jan 29, 2020

It seems we either need to patch FIndBrotli.cmake in arrow:
https://github.com/apache/arrow/blob/apache-arrow-0.15.1/cpp/cmake_modules/FindBrotli.cmake#L115-L126

or change the current patch of brotli

+ install(EXPORT brotli FILE unofficial-brotli-config.cmake NAMESPACE unofficial::brotli:: DESTINATION share/unofficial-brotli)

@takacsd
Copy link
Author

takacsd commented Jan 29, 2020

After more investigation it seems like multiple dependencies can't be found which has it's own Find*.cmake in the arrow project, like:

  • brotli (Brotli:: vs unofficial::brotli::)
  • lz4 (LZ4:: vs lz4::)
  • glog (GLOG:: vs glog::)
  • zstd (no cmake config file generated)
  • boost (boost_filesystem vs Boost::filesystem etc.)

In case of lz4, vcpkg provides the CMakeLists.txt, so it should be unofficial::lz4::. Or it is not a convention?
In case of zstd, no released version provides cmake config, but the version in the devel branch does, so we may need to backport the patch to the latest released version.

@philjdf
Copy link
Contributor

philjdf commented Feb 10, 2020

Consuming Arrow seems to be more manual than we'd like. ParquetSharp's Find*.cmake files might be of use to you https://github.com/G-Research/ParquetSharp/tree/master/cmake but it would be nice not to need these.

How do we go about fixing this stuff? Ideally we don't want to have to maintain a lot of patching in the Arrow port to rename the dependencies Arrow looks for.

By the way, Arrow has recently been updated to v0.16 in vcpkg.

@GPSnoopy
Copy link
Contributor

GPSnoopy commented Feb 10, 2020

@philjdf is correct. Whoever manages to patch ParquetSharp such that it builds from vcpkg cmake toolchain without needing custom Find*.cmake files will have done a great service in proving that vcpkg is a bit closer to being self-consistent and well behaved. Smaller projects are easy, but arrow dependencies are numerous, making the whole exercise less academic.

Every time we tried, it ended up in tears.

@takacsd
Copy link
Author

takacsd commented Feb 11, 2020

How do we go about fixing this stuff? Ideally we don't want to have to maintain a lot of patching in the Arrow port to rename the dependencies Arrow looks for.

Actually, I don't see any other solution than patching arrow. (And adding cmake config for zstd.)
Is there some sed like functionality implemented in vcpkg?
We can also try to convince them to change the naming of the targets.

Every time we tried, it ended up in tears.

I changed the target names to match the vcpkg ones and now I can use arrow without any problems.

@philjdf
Copy link
Contributor

philjdf commented Feb 14, 2020

Getting everything to agree what everything is called seems hard. It would be nice if there were strong Vcpkg guidelines on the right way of doing stuff. Last time I checked it was still missing that.

Re your comment about sed. Is your thinking to just make it easy for each port to have local names for dependencies in vcpkg? E.g. what it wants to call Brotli:: is actually unofficial::brotli::? This sounds like a nice solution. Maybe vcpkg could support that explicitly, where each port can have a table of local renamings. This would be far nicer than having to deal with large patches when updating/creating ports.

@takacsd
Copy link
Author

takacsd commented Feb 14, 2020

Re your comment about sed. Is your thinking to just make it easy for each port to have local names for dependencies in vcpkg? E.g. what it wants to call Brotli:: is actually unofficial::brotli::?

I was actually thinking patching arrow with sed instead of maintaining patches, but your idea is much cleaner.
If we could define such binding that would be ideal. I actually tried to use cmake's alias targets, but it gave me strange errors and I couldn't make it work. (But I'm also not a cmake expert).

@PhoebeHui PhoebeHui assigned NancyLi1013 and unassigned LilyWangL Nov 12, 2020
@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label May 14, 2021
@PhoebeHui PhoebeHui assigned LilyWangLL and unassigned NancyLi1013 Nov 22, 2021
@dg0yt
Copy link
Contributor

dg0yt commented Jul 26, 2022

IIUC the original brotli issues were resolved with the port pdate to 8.0.0 (#24714).
But with 8.0.0, upstream broke the export of static dependencies so arrow is hardly usable with any static triplet. This includes x64-linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

No branches or pull requests

7 participants