search

apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
14.36k stars 3.49k forks source link

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

Open asfimport opened 3 years ago

asfimport commented 3 years ago

https://github.com/apache/arrow/pull/8908#issuecomment-744780934

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.

https://github.com/apache/arrow/pull/8908#issuecomment-744898897

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.

https://github.com/apache/arrow/pull/8908#issuecomment-744954917

Why does it need the ARROW_ namespace prefix?

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

https://github.com/apache/arrow/pull/8908#issuecomment-745005158

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 commented 3 years ago

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

asfimport commented 3 years ago

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 commented 2 years ago

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 commented 1 year ago

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.