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

[C++][regression] Error when building Arrow v18.0.0 against non-LTS absl, abseil-cpp #44606

Open
gorloffslava opened this issue Nov 1, 2024 · 1 comment

Comments

@gorloffslava
Copy link

Describe the bug, including details regarding any error messages, version, and platform.

What causes the issue?

  1. When absl is built from the master and not from the tagged release, it doesn't define LTS_VERSION.
  2. Consequently, when CMake finds the non-LTS absl installation, the absl_VERSION variable is not set.
  3. And when we check this variable here, it expands to an empty string, which, in turn, leads to the cryptic error message:
CMake Error at cmake_modules/ThirdpartyToolchain.cmake:315 (if):
if given arguments:
 
     "VERSION_LESS" "20211102"
 
   Unknown arguments specified
 Call Stack (most recent call first):
   cmake_modules/ThirdpartyToolchain.cmake:4166 (resolve_dependency)
   CMakeLists.txt:546 (include)

Why it's the regression?

  1. Arrow v17.0.0 builds successfully against non-LTS absl.
  2. The code that added the failing check was introduced in e65c1e2

Temporary workaround:

  1. By manually passing -Dabsl_VERSION=<some lts version, like '20240722.0'> bypasses the check.
  2. However, this workaround is dirty. If one day will rely on this version for more granular check, it may break the things.

Why it's important?

  1. absl themselves encourage people to use cutting-edge version built from the latest commit, so it will cause problems for many Arrow consumers.

Component(s)

C++

@assignUser
Copy link
Member

cc @kou @raulcd

absl themselves encourage people to use cutting-edge version built from the latest commit, so it will cause problems for many Arrow consumers.

Even though there is a workaround I think this is a valid concern that might warrant inclusion in the new patch/minor?

@gorloffslava can you test with my changes in #44613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants