GH-37352: [C++] Don't put all dependencies to ArrowConfig.cmake/arrow.pc #37399
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kou
requested review from
assignUser,
raulcd,
wgtmac and
lidavidm
as code owners
|
@github-actions crossbow submit -g linux |
|
|
github-actions
bot
added
Component: Parquet
Component: C++
Component: FlightRPC
awaiting committer review
kou
force-pushed
the
packaging-amazon-linux-2023
branch
from
b0acb86
to
b194d0f
Compare
|
Revision: b0acb86 Submitted crossbow builds: ursacomputing/crossbow @ actions-0369c3fde3
|
kou
force-pushed
the
packaging-amazon-linux-2023
branch
2 times, most recently
from
382d666
to
e352564
Compare
…/arrow.pc Split Flight related dependencies such as gRPC to ArrowFlightConfig.cmake/arrow-flight.pc from ArrowConfig.cmake/arrow.pc. Split Parquet related dependencies such as Thrift to ParquetConfig.cmake/parquet.pc from ArrowConfig.cmake/arrow.pc.
kou
force-pushed
the
packaging-amazon-linux-2023
branch
from
e352564
to
0e53976
Compare
|
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit e1227a2. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
|
@github-actions crossbow submit test-r-gcc-11 |
|
Revision: 0e53976 Submitted crossbow builds: ursacomputing/crossbow @ actions-663bda6898
|
kou
added a commit
that referenced
this pull request
Sep 11, 2023
#37603) ### Rationale for this change GH-37399 removed "thrift" pkg-config package dependency from "parquet" pkg-config package accidentally. Before GH-37399, "arrow" pkg-config packages has "thrift" pkg-config package dependency as `Requires.private`. ### What changes are included in this PR? Add "thrift" to `Requires.private` of `parquet.pc`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: #37535 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
…/arrow.pc (apache#37399) ### Rationale for this change Currently, `arrow.pc` has `Requires.private: grpc++` when Flight is enabled and system gRPC is used. `grpc++.pc` depends on `re2.pc` on Amazon Linux 2023 and `re2.pc` has `-std=c++11`. It causes a problem when an user uses `c++ -std=c++17 a.cc $(pkg-config --cflags --libs arrow)`. Because the command will be expanded to `c++ -std=c++17 ... -std=c++11 ...`. The expanded command line uses C++11 not C++17 because `-std=c++11` is appeared after `-std=c++17`. In general, the user must use `c++ ... $(pkg-config --cflags --libs arrow) -std=c++17` to ensure using their `-std=XXX`. But we want to avoid this case as much as possible. ### What changes are included in this PR? Split Flight related dependencies such as gRPC to ArrowFlightConfig.cmake/arrow-flight.pc from ArrowConfig.cmake/arrow.pc. Split Parquet related dependencies such as Thrift to ParquetConfig.cmake/parquet.pc from ArrowConfig.cmake/arrow.pc. With this change, `c++ -std=c++17 ... $(pkg-config --cflags --libs arrow)` works because `arrow.pc` doesn't depend on `grpc++` (`re2`). But `c++ -std=c++17 ... $(pkg-config --cflags --libs arrow-flight)` still doesn't work on Amazon Linux 2023. `c++ ... $(pkg-config --cflags --libs arrow-flight) ... -std=c++17 ...` should be used for the case. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#37352 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
…quet.pc (apache#37603) ### Rationale for this change apacheGH-37399 removed "thrift" pkg-config package dependency from "parquet" pkg-config package accidentally. Before apacheGH-37399, "arrow" pkg-config packages has "thrift" pkg-config package dependency as `Requires.private`. ### What changes are included in this PR? Add "thrift" to `Requires.private` of `parquet.pc`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#37535 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
dgreiss
pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
…quet.pc (apache#37603) ### Rationale for this change apacheGH-37399 removed "thrift" pkg-config package dependency from "parquet" pkg-config package accidentally. Before apacheGH-37399, "arrow" pkg-config packages has "thrift" pkg-config package dependency as `Requires.private`. ### What changes are included in this PR? Add "thrift" to `Requires.private` of `parquet.pc`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#37535 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Currently,
arrow.pchasRequires.private: grpc++when Flight is enabled and system gRPC is used.grpc++.pcdepends onre2.pcon Amazon Linux 2023 andre2.pchas-std=c++11. It causes a problem when an user usesc++ -std=c++17 a.cc $(pkg-config --cflags --libs arrow). Because the command will be expanded toc++ -std=c++17 ... -std=c++11 .... The expanded command line uses C++11 not C++17 because-std=c++11is appeared after-std=c++17. In general, the user must usec++ ... $(pkg-config --cflags --libs arrow) -std=c++17to ensure using their-std=XXX. But we want to avoid this case as much as possible.What changes are included in this PR?
Split Flight related dependencies such as gRPC to ArrowFlightConfig.cmake/arrow-flight.pc from ArrowConfig.cmake/arrow.pc.
Split Parquet related dependencies such as Thrift to ParquetConfig.cmake/parquet.pc from ArrowConfig.cmake/arrow.pc.
With this change,
c++ -std=c++17 ... $(pkg-config --cflags --libs arrow)works becausearrow.pcdoesn't depend ongrpc++(re2). Butc++ -std=c++17 ... $(pkg-config --cflags --libs arrow-flight)still doesn't work on Amazon Linux 2023.c++ ... $(pkg-config --cflags --libs arrow-flight) ... -std=c++17 ...should be used for the case.Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.