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

Allow using custom mir libraries directory #198

Merged
merged 5 commits into from
Aug 9, 2024
Merged

Allow using custom mir libraries directory #198

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d68b9aa
allow using custom mir libraries directory
TibboddiT Aug 2, 2024
ae7912c
Merge branch 'mattkae:develop' into custom-mir-libs
TibboddiT Aug 4, 2024
6c2dedc
update cmake options ordering to improve readability
TibboddiT Aug 4, 2024
4aa767b
CI: install libmirwayland-dev
TibboddiT Aug 9, 2024
5a298d8
CI: install libmirwayland-dev
TibboddiT Aug 9, 2024
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
6 changes: 5 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(ENV{PKG_CONFIG_PATH} "/usr/local/lib/pkgconfig/")
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)

set(MIR_LIBRARIES_PKGCONFIG_DIRECTORY "" CACHE STRING "Search for Mir libraries pc files in this directory")
option(SNAP_BUILD "Building as a snap?" OFF)

set(ENV{PKG_CONFIG_PATH} "${MIR_LIBRARIES_PKGCONFIG_DIRECTORY}:/usr/local/lib/pkgconfig/")
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks any existing PKG_CONFIG_PATH setup.

$ echo $PKG_CONFIG_PATH
/nix/store/b3a0wh9v9md9kc4qdixsbwjy3799077r-boost-1.81.0-dev/lib/pkgconfig:/nix/store/sa2vr5rkh19pvy9j4hkb4lk95nlhqpix-glib-2.80.4-dev/lib/pkgconfig:/nix/store/vsygj2vzwqr9x7rzq4v3prcv2blkggmx-zlib-1.3.1-dev/lib/pkgconfig:/nix/store/niabvl1jq1hl7s3n1yx5zxa27l9mipkm-libffi-3.4.6-dev/lib/pkgconfig:[...]
$ $PKG_CONFIG --cflags miral
-I/nix/store/457888a9j750b2skjvi33h4xn66y05gi-mir-2.17.0-dev/include/miral -I/nix/store/457888a9j750b2skjvi33h4xn66y05gi-mir-2.17.0-dev/include/mircommon -I/nix/store/457888a9j750b2skjvi33h4xn66y05gi-mir-2.17.0-dev/include/mircore

/build/source/CMakeLists.txt(15):  set(MIR_LIBRARIES_PKGCONFIG_DIRECTORY  CACHE STRING Search for Mir libraries pc files in this directory )
/build/source/CMakeLists.txt(16):  option(SNAP_BUILD Building as a snap? OFF )
/build/source/CMakeLists.txt(18):  set(ENV{PKG_CONFIG_PATH} :/var/empty/local/lib/pkgconfig/ )
[...]
/nix/store/sa1yjb7xxrzcbd1bysj3r561vidfxxvb-cmake-3.29.6/share/cmake-3.29/Modules/FindPkgConfig.cmake(435):  set(ENV{PKG_CONFIG_PATH} /var/empty/local/lib/pkgconfig )
[...]
-- Checking for module 'miral'
--   No package 'miral' found

Copy link
Contributor

@OPNA2608 OPNA2608 Aug 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ah sorry. The pre-existing code already broke it, and I see this was supposed to close the issue for this: #196

Why is setting PKG_CONFIG_PATH=/usr/local/lib/pkgconfig and further messing around with it in CMake even necessary, is something wrong with some build environment somewhere? I feel like this setup should just be left to whatever build environment CMake is executed in, prepending a custom Mir path as desired.

Submitted #211 to instead use the existing ENV{PKG_CONFIG_PATH}, or set /usr/local/lib/pkgconfig as a fallback if it's empty.


find_package(PkgConfig)
pkg_check_modules(MIRAL miral REQUIRED)
pkg_check_modules(MIROIL miroil REQUIRED)
Expand All @@ -24,6 +26,7 @@ pkg_check_modules(MIRCOMMON mircommon REQUIRED)
pkg_check_modules(MIRCOMMON_INTERNAL mircommon-internal REQUIRED)
pkg_check_modules(MIRSERVER mirserver REQUIRED)
pkg_check_modules(MIRSERVER_INTERNAL mirserver-internal REQUIRED)
pkg_check_modules(MIRWAYLAND mirwayland REQUIRED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would Mirwayland be required here? I don't remember using it in the project anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only required at link time, otherwise building fails with

/usr/bin/ld: warning: libmirwayland.so.5, needed by <mir_install_directory>/lib/libmirplatform.so, not found (try using -rpath or -rpath-link)

and a bunch of undefined symbols.

Maybe mirplatform should require mirwayland in its pc file ? Currently it's just:

prefix=<mir_install_directory>
libdir=${prefix}/lib
includedir=${prefix}/include

Name: mirplatform
Description: Mir platform library
Version: 2.18.0
Requires: mircommon, mircore
Libs: -L${libdir} -lmirplatform
Cflags: -I${includedir}/mirplatform

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah okay that's probably an issue on their end, but I am happy to patch it on our end in the meantime.

Filed the bug: canonical/mir#3536

pkg_check_modules(GLIB REQUIRED IMPORTED_TARGET glib-2.0)
pkg_check_modules(YAML REQUIRED IMPORTED_TARGET yaml-cpp)
pkg_check_modules(LIBEVDEV REQUIRED IMPORTED_TARGET libevdev)
Expand Down Expand Up @@ -91,6 +94,7 @@ target_link_libraries(miracle-wm-implementation
${MIRPLATFORM_LDFLAGS}
${MIRSERVER_LDFLAGS}
${MIRSERVER_INTERNAL_LDFLAGS}
${MIRWAYLAND_LDFLAGS}
PkgConfig::YAML
PkgConfig::GLIB
PkgConfig::LIBEVDEV
Expand Down
5 changes: 5 additions & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ pkg_check_modules(MIRSERVER mirserver REQUIRED)
find_package(GTest REQUIRED)
pkg_check_modules(YAML REQUIRED IMPORTED_TARGET yaml-cpp)

if(MIR_LIBRARIES_PKGCONFIG_DIRECTORY)
list( PREPEND CMAKE_INSTALL_RPATH ${MIRAL_LIBRARY_DIRS} )
set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
endif()

add_executable(miracle-wm-tests
filesystem_configuration_test.cpp
tiling_window_tree_test.cpp
Expand Down
Loading