search

conda-forge / glog-feedstock

A conda-smithy repository for glog.
BSD 3-Clause "New" or "Revised" License
0 stars 12 forks source link

glog 0.7 breaks arrow, `GLOG_USE_GLOG_EXPORT` not working as intended? #26

Open h-vetinari opened 4 months ago

h-vetinari commented 4 months ago

Trying to compile arrow 15 (and below) with glog 0.7 runs into:

[138/498] Building CXX object src/arrow/CMakeFiles/arrow_objlib.dir/util/logging.cc.o
FAILED: src/arrow/CMakeFiles/arrow_objlib.dir/util/logging.cc.o 
$BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DARROW_EXPORTING -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HDFS -DARROW_MIMALLOC -DARROW_USE_GLOG -DARROW_WITH_BACKTRACE -DARROW_WITH_BROTLI -DARROW_WITH_BZ2 -DARROW_WITH_LZ4 -DARROW_WITH_RE2 -DARROW_WITH_SNAPPY -DARROW_WITH_TIMING_TESTS -DARROW_WITH_UTF8PROC -DARROW_WITH_ZLIB -DARROW_WITH_ZSTD -DAWS_AUTH_USE_IMPORT_EXPORT -DAWS_CAL_USE_IMPORT_EXPORT -DAWS_CHECKSUMS_USE_IMPORT_EXPORT -DAWS_COMMON_USE_IMPORT_EXPORT -DAWS_COMPRESSION_USE_IMPORT_EXPORT -DAWS_CRT_CPP_USE_IMPORT_EXPORT -DAWS_EVENT_STREAM_USE_IMPORT_EXPORT -DAWS_HTTP_USE_IMPORT_EXPORT -DAWS_IO_USE_IMPORT_EXPORT -DAWS_MQTT_USE_IMPORT_EXPORT -DAWS_S3_USE_IMPORT_EXPORT -DAWS_SDKUTILS_USE_IMPORT_EXPORT -DAWS_SDK_VERSION_MAJOR=1 -DAWS_SDK_VERSION_MINOR=11 -DAWS_SDK_VERSION_PATCH=242 -DAWS_USE_EPOLL -DBOOST_ALL_DYN_LINK -DURI_STATIC_BUILD -DUSE_IMPORT_EXPORT=1 -I$SRC_DIR/cpp/build/src -I$SRC_DIR/cpp/src -I$SRC_DIR/cpp/src/generated -isystem $SRC_DIR/cpp/thirdparty/flatbuffers/include -isystem $SRC_DIR/cpp/thirdparty/hadoop/include -isystem $SRC_DIR/cpp/build/jemalloc_ep-prefix/src -isystem $SRC_DIR/cpp/build/mimalloc_ep/src/mimalloc_ep/include/mimalloc-2.0 -Wno-noexcept-type -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/apache-arrow-15.0.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -fdiagnostics-color=always -fuse-ld=gold  -Wall -fno-semantic-interposition -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/apache-arrow-15.0.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -O3 -DNDEBUG -O2 -ftree-vectorize  -std=c++17 -fPIC -pthread -fPIC -MD -MT src/arrow/CMakeFiles/arrow_objlib.dir/util/logging.cc.o -MF src/arrow/CMakeFiles/arrow_objlib.dir/util/logging.cc.o.d -o src/arrow/CMakeFiles/arrow_objlib.dir/util/logging.cc.o -c $SRC_DIR/cpp/src/arrow/util/logging.cc
In file included from $SRC_DIR/cpp/src/arrow/util/logging.cc:31:
$PREFIX/include/glog/logging.h:60:4: error: #error <glog/logging.h> was not included correctly. See the documention for how to consume the library.
   60 | #  error <glog/logging.h> was not included correctly. See the documention for how to consume the library.
      |    ^~~~~

and then a very long stream of errors.

The release notes don't point to any such hard breaking changes, and looking at the code, it seems that we'd now need to unconditionally set GLOG_USE_GLOG_EXPORT.

Indeed, glog's CMakeLists.txt unconditionally puts this symbol in the interface of libglog, which makes it all the more surprising that this isn't working out of the box.

Not sure if there's a bug in 0.7 or in the packaging, or somewhere else. In particular, arrow's usage of glog is be "CMake-native" and AFAICT should respect target_compile_definitions.

h-vetinari commented 4 months ago

Or perhaps our generated export.h (not part of the sources) does not correctly define GLOG_EXPORT?

h-vetinari commented 4 months ago

Also, the problem does not appear on windows.

h-vetinari commented 4 months ago

@xhochy, any thoughts here?

xhochy commented 4 months ago

Yes, I know the issue. I'll push a fix later. This is special to arrow.

h-vetinari commented 4 months ago

This is not specific to arrow it seems. It also happens in https://github.com/conda-forge/arcticdb-feedstock/pull/177:

  $PREFIX/include/glog/logging.h:60:4: error: #error <glog/logging.h> was not included correctly. See the documention for how to consume the library.
     60 | #  error <glog/logging.h> was not included correctly. See the documention for how to consume the library.
        |    ^~~~~

Rather than set INTERFACE_COMPILE_DEFINITIONS "GLOG_USE_GLOG_EXPORT" on consumers (as in https://github.com/apache/arrow/pull/40230), why don't we do

  target_compile_definitions(
    glog
    INTERFACE
      GLOG_USE_GLOG_EXPORT
  )

in this feedstock? That seems like a much more scalable solution to me.

h-vetinari commented 4 months ago

why don't we do [...]

As it turns out, glog already does do that. I don't understand why the CMake target_compile_definitions aren't working anymore - it's not the first time I see this recently - could this be a CMake regression?

h-vetinari commented 4 months ago

Folly was another case where this definitely shouldn't have been necessary - it definitely does find_package(glog).

I ended up opening a post on the cmake discourse.

xhochy commented 4 months ago

I think this breakage comes down to the issue that the all the projects have custom FindGlog.cmake files that take preference over the official one. At least for Arrow and Folly, this was the underlying issue. In Arrow, it was easy to redirect to the official one (if there is one), in folly, I didn't understand their build system.