search

abseil / abseil-cpp

Abseil Common Libraries (C++)
https://abseil.io
Apache License 2.0
14.9k stars 2.6k forks source link

Abseil pkgconfig leads to too many arguments #1168

Open Allen-Webb opened 2 years ago

Allen-Webb commented 2 years ago

Including the pkgconfig from abseil results in hundreds of CFLAGS many of which are redundant. I believe they could be restructured to have one common pkconfig that is added to "requires" so the CFLAGS are only included once.

Here is an example of the CFLAGS that are repeated:

-DNOMINMAX
-Wno-float-conversion
-Wno-implicit-float-conversion
-Wno-implicit-int-float-conversion
-Wno-implicit-int-conversion
-Wno-shorten-64-to-32
-Wno-sign-conversion
-Wno-unknown-warning-option
Allen-Webb commented 2 years ago

It looks like you can rely on absl_config.pc to provide the redundant CFLAGS, and strip them from everything else that requires absl_config.

Allen-Webb commented 2 years ago

Here is a patch we are considering using for ChromeOS: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/3630468/1/dev-cpp/abseil-cpp/files/abseil-cpp-20211102.0-fix-pkgconfig.patch

suertreus commented 2 years ago

I see you're making your own absl.pc in src_compile - are you having problems using that one or using the files we provide?

What specific pkg-config invocation is leading to problems? I'd like to reproduce this so I can validate your proposed patch.

Allen-Webb commented 2 years ago

I am having issues with the files provided. Picking an arbitrary sublibrary absl_base: here is the output without my fix:

pkg-config --cflags absl_base
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX \
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX

On doublechecking with my patch it looks like absl_config is being filtered in error, but ideally it would have the shared cflags since it is required by all the other sub-libraries. Here is pkgconfig for it:

pkg-config --cflags absl_config
-Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-float-conversion -Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-sign-conversion -Wno-unknown-warning-option -DNOMINMAX

Notice the lack of repetition.

neverpanic commented 1 year ago

Piling on to this issue, on macOS the abseil pkgconfig files now contain flags that make other software fail:

$ pkg-config --cflags absl_random_internal_randen_hwaes
  -Wno-float-conversion \
  -Wno-implicit-float-conversion \
  -Wno-implicit-int-float-conversion \
  -Wno-unknown-warning-option \
  -DNOMINMAX \
  -Xarch_x86_64 \
  -Xarch_arm64 \
  -Wno-unused-command-line-argument \
  -Wno-float-conversion \
  -Wno-implicit-float-conversion \
  -Wno-implicit-int-float-conversion \
  -Wno-unknown-warning-option \
  -DNOMINMAX \
  -Xarch_x86_64 \
  -Xarch_arm64 \
  -Wno-unused-command-line-argument \
  -Wno-float-conversion \
  -Wno-implicit-float-conversion \
  -Wno-implicit-int-float-conversion \
  -Wno-unknown-warning-option \
  -DNOMINMAX \
  -Wno-float-conversion \
  -Wno-implicit-float-conversion \
  -Wno-implicit-int-float-conversion \
  -Wno-unknown-warning-option \
  -DNOMINMAX \
  -I/opt/local/include

The inclusion of -Xarch_x86_64 and -Xarch_arm64 causes

$ /usr/bin/clang++ \
  -I/opt/local/var/macports/build/_opt_dports_devel_Bear/Bear/work/build/subprojects/Build/BearSource \
  -I/opt/local/var/macports/build/_opt_dports_devel_Bear/Bear/work/build/subprojects/Build/BearSource/intercept/proto \
  -isystem /opt/local/libexec/openssl3/include \
  -pipe \
  -Os \
  -DNDEBUG \
  -I/opt/local/include \
  -stdlib=libc++ \
  -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk \
  -fno-exceptions \
  -DSPDLOG_NO_EXCEPTIONS \
  -DGOOGLE_PROTOBUF_NO_RTTI \
  -Wall \
  -Wextra \
  -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk \
  -mmacosx-version-min=12.0 \
  -Wno-float-conversion \
  -Wno-implicit-float-conversion \
  -Wno-implicit-int-float-conversion \
  -Wno-unknown-warning-option \
  -DNOMINMAX \
  -Xarch_x86_64 \
  -Xarch_arm64 \
  -Wno-unused-command-line-argument \
  -std=c++17 \
  -MD \
  -MT intercept/proto/CMakeFiles/rpc_a.dir/supervise.pb.cc.o \
  -MF CMakeFiles/rpc_a.dir/supervise.pb.cc.o.d \
  -o CMakeFiles/rpc_a.dir/supervise.pb.cc.o \
  -c /opt/local/var/macports/build/_opt_dports_devel_Bear/Bear/work/build/subprojects/Build/BearSource/intercept/proto/supervise.pb.cc
clang: error: invalid Xarch argument: '-Xarch_x86_64 -Xarch_arm64', options requiring arguments are unsupported

Is it really necessary to include all these cflags in the pkg-config files? Shouldn't the pkg-config files only include those flags that are required to compile against abseil (which certainly none of the warning options are, for example)?

Allen-Webb commented 1 year ago

I agree there shouldn't be warning related flags in the pkg-config entries.

judaew commented 1 year ago

The issue which the inclusion of -Xarch_x86_64 and -Xarch_arm64 causes is here:

https://github.com/abseil/abseil-cpp/blob/cde2f0eaaed3fb8581511cb5719d39172a5a2d81/absl/copts/AbseilConfigureCopts.cmake#L41-L47