Uh oh!
There was an error while loading. Please reload this page.
ARROW-17231: [C++] Restore STATIC_INSTALL_INTERFACE_LIBS for arrow_static - #13707
ARROW-17231: [C++] Restore STATIC_INSTALL_INTERFACE_LIBS for arrow_static#13707dg0yt wants to merge 2 commits into
Conversation
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename pull request title in the following format? or See also: |
kou
commented
Jul 26, 2022
Good catch! |
dg0yt
commented
Jul 26, 2022
This is pretty much overhead for a single word contribution, isn't it. I would be glad if project members could handle that. |
kou
commented
Jul 27, 2022
OK. I want a test for this to prevent this problem again. |
dg0yt
commented
Jul 27, 2022
Hm, this means a post-install test so that it runs in arrow CI. But I don't know where to start in arrow (CI). Is there any existing post-install test? Which CI job does static linkage with dependencies provided by the system? What I did in vcpkg is to install the minimal_build example source code with the arrow port, and add another port |
kou
commented
Jul 27, 2022
Yes. Here are CI jobs for us:
I think that we can add one more CI job for this case. |
kou
commented
Jul 27, 2022
Could you show an error message without this change? |
dg0yt
commented
Jul 28, 2022
In other GH projects, I would have temporarily pushed a revert of the original change, and force-push without the revert after the CI run. (Or I would have pushed the test as first commit.) I can do that here, but I need maintainer approval for CI runs. For reference, this is from vcpgk with the patch disactivated. Not a single static system lib passed in the linking step. |
dg0yt
commented
Jul 28, 2022
TBH I don't really understand the issue title "arrow_static requires needless dependencies". The private dependencies are not "needless". |
kou
commented
Jul 28, 2022
I've added "Exported" to "arrow_static". (It seems that you should have created a JIRA issue by yourself because this is not "a single word contribution".) |
kou
commented
Jul 28, 2022
Thanks for the error log. |
| - name: Run minimal example (static, system dependencies) | ||
| run: | | ||
| cd cpp/examples/minimal_build | ||
| docker-compose run --rm static-system-dependency |
There was a problem hiding this comment.
Does this reproduce this problem?
I couldn't reproduce this problem by this without your cpp/src/arrow/CMakeLists.txt change.
There was a problem hiding this comment.
I admit I can't test this particular part locally.
FTR: is this already run elsewhere in the CI configurations?
There was a problem hiding this comment.
This change restore the
STATIC_INSTALL_INTERFACE_LIBSkeyword for lib arrow, which was lost in 2fea818.It fixes the usability of the exported config for static builds with system dependencies, which lacked all transitive usage requirements.
Noticed in building the minimal example in vcpkg, as part of microsoft/vcpkg#25983.