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.

Sign up for GitHub

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

Jump to bottom

[C++] arrow.pc should have -DARROW_STATIC for Windows static builds #14869

Closed
lukester1975 opened this issue Dec 7, 2022 · 3 comments · Fixed by #14900
Closed

[C++] arrow.pc should have -DARROW_STATIC for Windows static builds #14869

lukester1975 opened this issue Dec 7, 2022 · 3 comments · Fixed by #14900
Assignees
@lukester1975
Labels
Component: C++ - Gandiva Component: C++ Component: FlightRPC Component: Parquet Component: Python Type: enhancement
Milestone
11.0.0

Comments

@lukester1975
Copy link
Contributor

Describe the enhancement requested

Without, the generated pc file is insufficient (at least without "manually" defining ARROW_STATIC, which is unpleasant).

Quick hack fix: lukester1975@2a8efd9

  • Cflags.private is quite new (https://gitlab.freedesktop.org/pkg-config/pkg-config/-/merge_requests/13), so this approach might be unpalatable.
  • pkgconfiglite is too old to include that (no commits since 2016??). It does quietly ignore the field, though.
  • vcpkg does its own merging of Cflags.private into Cflags, so not an issue if pkg-config doesn't understand Cflags.private there (my case).
  • Obviously this is applying to all platforms, not just Windows, but should do no harm...?

Seems like there should be some sort of fix here rather than asking vcpkg to patch it!

Regards

Component(s)

C++
@kou
Copy link
Member

kou commented Dec 7, 2022

Could you open a pull request with your approach?
pkgconf that is a drop-in replacement of pkg-config supports Cflags.private: http://pkgconf.org/features.html
So we can use Cflags.private for it.

We also need similar change to all other *.pc.in too.

@lukester1975
Copy link
Contributor Author

Sure, can do.

Are the other pc.ins required, though? They all Requires: arrow (directly or indirectly) so should inherit the flag from there.

@kou
Copy link
Member

kou commented Dec 8, 2022

We have separated _STATIC macro for each modules. For example, we have PARQUET_STATIC for Parquet and ARROW_DS_STATIC for Arrow Dataset.

lukester1975 added a commit to lukester1975/arrow that referenced this issue Dec 9, 2022
@lukester1975
apacheGH-14869: [C++] Add Cflags.private defining <lib>_STATIC to <li…
109a40a 
…b>.pc.in.

Required for Windows usage of static builds via vcpkg & pkgconfig.
@lukester1975 lukester1975 mentioned this issue Dec 9, 2022
Merged
@kou kou closed this as completed in #14900 Dec 9, 2022
kou pushed a commit that referenced this issue Dec 9, 2022
@lukester1975
GH-14869: [C++] Add Cflags.private defining <lib>_STATIC to <lib>.pc.…
a7502c9 
…in. (#14900)

* Closes: #14869

Authored-by: Luke Elliott <luke.b.elliott@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 11.0.0 milestone Dec 9, 2022
@lukester1975 lukester1975 mentioned this issue Dec 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukester1975 lukester1975

Labels
Component: C++ - Gandiva Component: C++ Component: FlightRPC Component: Parquet Component: Python Type: enhancement
Projects
None yet
Milestone
11.0.0
Development

Successfully merging a pull request may close this issue.

GH-14869: [C++] Add Cflags.private defining <lib>_STATIC to <lib>.pc.in. lukester1975/arrow
2 participants
@kou @lukester1975