Bug 99586 - CMake fixes
Summary: CMake fixes
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-29 09:25 UTC by Ralf Habacker
Modified: 2017-01-30 22:57 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Fix issue missing temporary dir on cross compiling for windows with cmake. (1.40 KB, patch)
2017-01-29 09:25 UTC, Ralf Habacker
Details | Splinter Review
Replace deprecated cmake install_ functions with related install(...) calls. (5.43 KB, patch)
2017-01-29 09:25 UTC, Ralf Habacker
Details | Splinter Review
Fix broken install of dbus-arch-deps.h. (783 bytes, patch)
2017-01-29 09:26 UTC, Ralf Habacker
Details | Splinter Review
Replace deprecated cmake install_ functions with related install(...) calls. (4.92 KB, patch)
2017-01-29 09:48 UTC, Ralf Habacker
Details | Splinter Review
Add test-uid-permissions test case to cmake build system. (1.03 KB, patch)
2017-01-29 09:52 UTC, Ralf Habacker
Details | Splinter Review
Do not define unused variable DBUS_SESSION_SOCKET_DIR setting with cmake on Windows. (2.66 KB, patch)
2017-01-30 18:38 UTC, Ralf Habacker
Details | Splinter Review
Do not dump unused DBUS_SYSTEM_BUS_DEFAULT_ADDRESS variable on windows. (2.71 KB, patch)
2017-01-30 18:39 UTC, Ralf Habacker
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Habacker 2017-01-29 09:25:52 UTC

    
Comment 1 Ralf Habacker 2017-01-29 09:25:55 UTC
Created attachment 129211 [details] [review]
Fix issue missing temporary dir on cross compiling for windows with cmake.

Bug:
Comment 2 Ralf Habacker 2017-01-29 09:25:58 UTC
Created attachment 129212 [details] [review]
Replace deprecated cmake install_ functions with related install(...) calls.

Bug:
Comment 3 Ralf Habacker 2017-01-29 09:26:01 UTC
Created attachment 129213 [details] [review]
Fix broken install of dbus-arch-deps.h.

Bug:
Comment 4 Ralf Habacker 2017-01-29 09:48:08 UTC
Created attachment 129214 [details] [review]
Replace deprecated cmake install_ functions with related install(...) calls.

Bug:
Comment 5 Ralf Habacker 2017-01-29 09:52:54 UTC
Created attachment 129215 [details] [review]
Add test-uid-permissions test case to cmake build system.

Bug:
Comment 6 Simon McVittie 2017-01-30 11:57:21 UTC
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 7 Simon McVittie 2017-01-30 11:57:59 UTC
Comment on attachment 129213 [details] [review]
Fix broken install of dbus-arch-deps.h.

Review of attachment 129213 [details] [review]:
-----------------------------------------------------------------

Looks fine
Comment 8 Simon McVittie 2017-01-30 11:58:45 UTC
Comment on attachment 129214 [details] [review]
Replace deprecated cmake install_ functions with related install(...) calls.

Review of attachment 129214 [details] [review]:
-----------------------------------------------------------------

Seems fine
Comment 9 Simon McVittie 2017-01-30 11:58:58 UTC
Comment on attachment 129215 [details] [review]
Add test-uid-permissions test case to cmake build system.

Review of attachment 129215 [details] [review]:
-----------------------------------------------------------------

Fine
Comment 10 Simon McVittie 2017-01-30 12:19:03 UTC
(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.
Comment 11 Ralf Habacker 2017-01-30 18:38:51 UTC
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:
Comment 12 Ralf Habacker 2017-01-30 18:39:03 UTC
Created attachment 129236 [details] [review]
Do not dump unused DBUS_SYSTEM_BUS_DEFAULT_ADDRESS variable on windows.

Bug:
Comment 13 Simon McVittie 2017-01-30 20:33:13 UTC
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 14 Simon McVittie 2017-01-30 20:33:49 UTC
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
Comment 15 Ralf Habacker 2017-01-30 22:57:50 UTC
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.