Created attachment 129211 [details] [review] Fix issue missing temporary dir on cross compiling for windows with cmake. Bug:
Created attachment 129212 [details] [review] Replace deprecated cmake install_ functions with related install(...) calls. Bug:
Created attachment 129213 [details] [review] Fix broken install of dbus-arch-deps.h. Bug:
Created attachment 129214 [details] [review] Replace deprecated cmake install_ functions with related install(...) calls. Bug:
Created attachment 129215 [details] [review] Add test-uid-permissions test case to cmake build system. Bug:
Comment on attachment 129211 [details] [review] Fix issue missing temporary dir on cross compiling for windows with cmake. Review of attachment 129211 [details] [review]: ----------------------------------------------------------------- ::: cmake/CMakeLists.txt @@ +441,5 @@ > set (DBUS_SESSION_SOCKET_DIR $ENV{TMP}) > else (NOT $ENV{TMP} STREQUAL "") > if (WIN32) > + if (CMAKE_CROSSCOMPILING) > + set (DBUS_SESSION_SOCKET_DIR /tmp) In a build for Windows, where is DBUS_SESSION_SOCKET_DIR used? Given that Windows has no concept of sockets that exist in a directory (that I know of), any code that uses this variable in a non-Unix build is almost certainly wrong. The uses I can see are tools/dbus-cleanup-sockets.c (only compiled for Unix and only meaningful on Unix), TEST_LISTEN via TEST_SOCKET_DIR (on Unix only), and DBUS_SESSION_BUS_LISTEN_ADDRESS (on Unix only). DBUS_SESSION_SOCKET_DIR and TEST_SOCKET_DIR should both become a Unix-only thing, both for Autotools and CMake: there is no value that they could correctly have on Windows.
Comment on attachment 129213 [details] [review] Fix broken install of dbus-arch-deps.h. Review of attachment 129213 [details] [review]: ----------------------------------------------------------------- Looks fine
Comment on attachment 129214 [details] [review] Replace deprecated cmake install_ functions with related install(...) calls. Review of attachment 129214 [details] [review]: ----------------------------------------------------------------- Seems fine
Comment on attachment 129215 [details] [review] Add test-uid-permissions test case to cmake build system. Review of attachment 129215 [details] [review]: ----------------------------------------------------------------- Fine
(In reply to Simon McVittie from comment #6) > Comment on attachment 129211 [details] [review] > Fix issue missing temporary dir on cross compiling for windows with cmake. ... > DBUS_SESSION_SOCKET_DIR and TEST_SOCKET_DIR should both become a Unix-only > thing, both for Autotools and CMake: there is no value that they could > correctly have on Windows. I'm testing a patch that does this change instead.
Created attachment 129235 [details] [review] Do not define unused variable DBUS_SESSION_SOCKET_DIR setting with cmake on Windows. This fixes also an undefined temp dir cmake error on cross compiling for windows. Bug:
Created attachment 129236 [details] [review] Do not dump unused DBUS_SYSTEM_BUS_DEFAULT_ADDRESS variable on windows. Bug:
Comment on attachment 129235 [details] [review] Do not define unused variable DBUS_SESSION_SOCKET_DIR setting with cmake on Windows. Review of attachment 129235 [details] [review]: ----------------------------------------------------------------- Looks fine. I should clean this up further, but it isn't my top priority right now.
Comment on attachment 129236 [details] [review] Do not dump unused DBUS_SYSTEM_BUS_DEFAULT_ADDRESS variable on windows. Review of attachment 129236 [details] [review]: ----------------------------------------------------------------- Looks good
patches pushed to git master
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.