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++] Improve *_SOURCE CMake variables naming #26842

Open
asfimport opened this issue Dec 15, 2020 · 8 comments
Open

[C++] Improve *_SOURCE CMake variables naming #26842

asfimport opened this issue Dec 15, 2020 · 8 comments

Comments

@asfimport
Copy link
Collaborator

#8908 (comment)

This change also renamed our Boost dependency name to "Boost" from
"BOOST". It means that users need to use -DBoost_SOURCE not
-DBOOST_SOURCE. To keep backward compatibility, -DBOOST_SOURCE is
still accepted when -DBoost_SOURCE isn't specified.

Users also need to use -Dre2_SOURCE not -DRE2_SOURCE. To keep backward
compatibility, -DRE2_SOURCE is still accepted when -Dre2_SOURCE isn't
specified.

I would love to have this kind of case-insensitive handling for all dependencies. This has tripped me up many times and it is difficult to explain to others why everything else is ALL_CAPS but these dependencies are a mix.

#8908 (comment)

OK. How about using ARROW_${UPPERCASE_DEPENDENCY_NAME}_SOURCE CMake variables for them like ARROW_\*_USE_SHARED?

If it sounds reasonable, we can work on it as a separated task.

#8908 (comment)

Why does it need the ARROW_ namespace prefix?

I'm fine with anything that is intuitive and trivial to document.

#8908 (comment)

Because of consistency.
If we use ARROW_${UPPERCASE_DEPENDENCY_NAME}_SOURCE not ${UPPERCASE_DEPENDENCY_NAME}_SOURCE, we can explain that you can customize how to use ${DEPENDENCY} by ARROW_${UPPERCASE_DEPENDENCY_NAME}_{SOURCE,USE_SHARED} CMake variables. It'll more intuitive than using ${UPPERCASE_DEPENDENCY_NAME}_SOURCE and ARROW_${UPPERCASE_DEPENDENCY_NAME}_USE_SHARED.

Reporter: Kouhei Sutou / @kou
Assignee: Kouhei Sutou / @kou

Note: This issue was originally created as ARROW-10911. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
@xhochy What do you think about this?

@asfimport
Copy link
Collaborator Author

Uwe Korn / @xhochy:
I would suggest to use ARROW_${UPPERCASE_DEPENDENCY_NAME}_SOURCE to be in line with the USE_SHARED. I don't have a strong preference regarding the uppercasing but the ARROW_ prefix is important to separate it from variables that are used in the Find*.cmake scripts.

@asfimport
Copy link
Collaborator Author

Todd Farmer / @toddfarmer:
This issue was last updated over 90 days ago, which may be an indication it is no longer being actively worked. To better reflect the current state, the issue is being unassigned. Please feel free to re-take assignment of the issue if it is being actively worked, or if you plan to start that work soon.

@asfimport
Copy link
Collaborator Author

Apache Arrow JIRA Bot:
This issue was last updated over 90 days ago, which may be an indication it is no longer being actively worked. To better reflect the current state, the issue is being unassigned per project policy. Please feel free to re-take assignment of the issue if it is being actively worked, or if you plan to start that work soon.

@kou
Copy link
Member

kou commented Feb 25, 2025

We may want to enable https://cmake.org/cmake/help/latest/policy/CMP0144.html when we work on this.

@wgtmac
Copy link
Member

wgtmac commented Feb 26, 2025

CMP0144 requires CMake 3.27 so we have to wait?

@kou
Copy link
Member

kou commented Feb 26, 2025

We don't need to wait CMake 3.27. We can emulate this easily by something like:

if(NOT POLICY CMP0144)
  if(DEFINED BOOST_ROOT AND NOT DEFINED Boost_ROOT)
    set(Boost_ROOT ${BOOST_ROOT})
  endif()
endif()

@wgtmac
Copy link
Member

wgtmac commented Feb 26, 2025

That sounds good!

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

3 participants