Skip to content

Adapt google-glog detection to use CONFIG mode with MODULE fallback and pass… #2474

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −7
Open

Adapt google-glog detection to use CONFIG mode with MODULE fallback and pass… #2474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions CMake/folly-deps.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,18 @@ if(LIBGFLAGS_FOUND)
set(FOLLY_LIBGFLAGS_INCLUDE ${LIBGFLAGS_INCLUDE_DIR})
endif()

find_package(Glog MODULE)
set(FOLLY_HAVE_LIBGLOG ${GLOG_FOUND})
list(APPEND FOLLY_LINK_LIBRARIES ${GLOG_LIBRARY})
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${GLOG_INCLUDE_DIR})
# Detect glog with CONFIG first
find_package(glog CONFIG QUIET)
if(glog_FOUND)
set(FOLLY_HAVE_LIBGLOG True)
list(APPEND FOLLY_LINK_LIBRARIES glog::glog)
else()
# Fallback to module mode
find_package(Glog MODULE REQUIRED)
set(FOLLY_HAVE_LIBGLOG ${GLOG_FOUND})
list(APPEND FOLLY_LINK_LIBRARIES ${GLOG_LIBRARY})
list(APPEND FOLLY_INCLUDE_DIRECTORIES ${GLOG_INCLUDE_DIR})
endif()

find_package(LibEvent MODULE REQUIRED)
list(APPEND FOLLY_LINK_LIBRARIES ${LIBEVENT_LIB})
Expand Down
19 changes: 16 additions & 3 deletions folly/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,21 @@ if (PYTHON_EXTENSIONS)
# Tell setup.py where to find includes and libfolly.so
set(prop "$<TARGET_PROPERTY:folly_base,INCLUDE_DIRECTORIES>")
set(incs "$<$<BOOL:${prop}>:-I$<JOIN:${prop},:>>")
set(glog_lib "${GLOG_LIBRARY}")
cmake_path(REMOVE_FILENAME glog_lib)
set(libs "-L${CMAKE_BINARY_DIR}:${glog_lib}")

set(libs "-L${CMAKE_BINARY_DIR}")
if(TARGET glog::glog)
# Modern glog with imported targets - get the library location
get_target_property(glog_location glog::glog IMPORTED_LOCATION)
if(glog_location)
cmake_path(REMOVE_FILENAME glog_location OUTPUT_VARIABLE glog_lib_dir)
set(libs "${libs}:${glog_lib_dir}")
endif()
elseif(GLOG_LIBRARY)
# Traditional FindModule approach fallback
set(glog_lib "${GLOG_LIBRARY}")
cmake_path(REMOVE_FILENAME glog_lib)
set(libs "${libs}:${glog_lib}")
endif()

add_custom_target(folly_python_bindings ALL
DEPENDS folly create_binding_symlink
Expand Down Expand Up @@ -112,6 +124,7 @@ if (PYTHON_EXTENSIONS)
add_dependencies(folly_python_cpp folly_python_bindings create_post_binding_symlink)
set_property(TARGET folly_python_cpp PROPERTY VERSION ${PACKAGE_VERSION})
target_compile_definitions(folly_python_cpp PRIVATE BOOST_NO_AUTO_PTR)
target_compile_definitions(folly_python_cpp PRIVATE GLOG_USE_GLOG_EXPORT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment I made down below in setup.py.

target_include_directories(folly_python_cpp PRIVATE "${_cybld}")
target_link_libraries(folly_python_cpp PUBLIC folly)
apply_folly_compile_options_to_target(folly_python_cpp)
Expand Down
2 changes: 2 additions & 0 deletions folly/python/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@
"folly.executor",
sources=["folly/executor.pyx", "folly/ProactorExecutor.cpp"],
libraries=["folly", "glog"],
define_macros=[("GLOG_USE_GLOG_EXPORT", "1")],
),
Extension(
"folly.iobuf",
sources=["folly/iobuf.pyx", "folly/iobuf_ext.cpp"],
libraries=["folly", "glog"],
define_macros=[("GLOG_USE_GLOG_EXPORT", "1")],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The export should only be needed when using glog as a .dll, right? There should already be something in the cmake config to detect and propagate that IIRC. (though I'm actually not sure if this python code path can actually make use of that detection in cmake)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've only been using glog as a shared library yes - without the macro export the python module compilation doesn't know how to locate the library.

Without the setup.py modifications:

[ 33%] Linking CXX shared library singleton_thread_local_overload.so
[ 33%] Built target singleton_thread_local_overload
/usr/lib/python3.13/site-packages/setuptools/__init__.py:92: _DeprecatedInstaller: setuptools.installer and fetch_build_eggs are deprecated.
!!

        ********************************************************************************
        Requirements should be satisfied by a PEP 517 installer.
        If you are using pip, you can try `pip install --use-pep517`.

        By 2025-Oct-31, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.
        ********************************************************************************

!!
  dist.fetch_build_eggs(dist.setup_requires)
running build_ext
building 'folly.executor' extension
/usr/bin/c++ -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wp,-D_GLIBCXX_ASSERTIONS -fPIC -I/Downloads/20250715174717_folly/folly/src/folly -I/Downloads/20250715174717_folly/folly/src/folly/build -I/usr/include -I/usr/include/libiberty -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include/python3.13 -c folly/ProactorExecutor.cpp -o build/temp.linux-x86_64-cpython-313/folly/ProactorExecutor.o
In file included from /Downloads/20250715174717_folly/folly/src/folly/folly/synchronization/HazptrObj.h:22,
                 from /Downloads/20250715174717_folly/folly/src/folly/folly/synchronization/HazptrDomain.h:27,
                 from /Downloads/20250715174717_folly/folly/src/folly/folly/synchronization/Hazptr.h:20,
                 from /Downloads/20250715174717_folly/folly/src/folly/folly/concurrency/detail/ConcurrentHashMap-detail.h:28,
                 from /Downloads/20250715174717_folly/folly/src/folly/folly/concurrency/ConcurrentHashMap.h:23,
                 from /Downloads/20250715174717_folly/folly/src/folly/folly/python/ProactorExecutor.h:19,
                 from folly/ProactorExecutor.cpp:17:
/usr/include/glog/logging.h:60:4: error: #error <glog/logging.h> was not included correctly. See the documentation for how to consume the library.
   60 | #  error <glog/logging.h> was not included correctly. See the documentation for how to consume the library.
      |    ^~~~~
In file included from /usr/include/glog/logging.h:63:
/usr/include/glog/flags.h:45:4: error: #error <glog/flags.h> was not included correctly. See the documentation for how to consume the library.
   45 | #  error <glog/flags.h> was not included correctly. See the documentation for how to consume the library.
      |    ^~~~~
In file included from /usr/include/glog/logging.h:77:
/usr/include/glog/log_severity.h:38:4: error: #error <glog/log_severity.h> was not included correctly. See the documentation for how to consume the library.
   38 | #  error <glog/log_severity.h> was not included correctly. See the documentation for how to consume the library.
      |    ^~~~~
In file included from /usr/include/glog/logging.h:78:
/usr/include/glog/vlog_is_on.h:71:4: error: #error <glog/vlog_is_on.h> was not included correctly. See the documentation for how to consume the library.
   71 | #  error <glog/vlog_is_on.h> was not included correctly. See the documentation for how to consume the library.
      |    ^~~~~
      ```

),
]

Expand Down
Loading