Bug 89450

Summary: cmake 3.1 fixes
Product: dbus Reporter: Ralf Habacker <ralf.habacker>
Component: coreAssignee: D-Bus Maintainers <dbus>
Status: RESOLVED FIXED QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: medium    
Version: git master   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Fix cmake build system bug not generating versioned library name in case LT_REVISION is zero
Fix cmake 3.1.3 policy CMP0026 warning
Fix cmake 3.1.3 policy CMP0053 warning
Fix cmake 3.1.3 policy CMP0054 warning

Description Ralf Habacker 2015-03-05 22:37:09 UTC
The appended patches fixes issues compiling dbus with cmake 3.1
Comment 1 Ralf Habacker 2015-03-05 23:12:20 UTC
Created attachment 114053 [details] [review]
Fix cmake build system bug not generating versioned library name in case LT_REVISION is zero
Comment 2 Ralf Habacker 2015-03-05 23:12:50 UTC
Created attachment 114054 [details] [review]
Fix cmake 3.1.3 policy CMP0026 warning
Comment 3 Ralf Habacker 2015-03-05 23:13:16 UTC
Created attachment 114055 [details] [review]
Fix cmake 3.1.3 policy CMP0053 warning
Comment 4 Ralf Habacker 2015-03-05 23:13:43 UTC
Created attachment 114056 [details] [review]
Fix cmake 3.1.3 policy CMP0054 warning
Comment 5 Simon McVittie 2015-03-06 11:58:53 UTC
Comment on attachment 114053 [details] [review]
Fix cmake build system bug not generating versioned library name in case LT_REVISION is zero

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

Yes
Comment 6 Simon McVittie 2015-03-06 12:02:35 UTC
Comment on attachment 114054 [details] [review]
Fix cmake 3.1.3 policy CMP0026 warning

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

I'd prefer a commit message that gives more of a clue what is changing to a reader not following the link, perhaps something like this as a replacement for the first line:

cmake: stop using deprecated LOCATION property (policy CMP0026)

but the actual change looks fine.
Comment 7 Simon McVittie 2015-03-06 12:06:37 UTC
Comment on attachment 114055 [details] [review]
Fix cmake 3.1.3 policy CMP0053 warning

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

Again, I'd prefer the first line of the commit message to give more clue what's going on here and why. If I'm understanding it correctly, perhaps it's something like this:

cmake: @VAR@ substitutions in set() are deprecated, use string(CONFIGURE) instead (policy CMP0053)
Comment 8 Simon McVittie 2015-03-06 12:09:03 UTC
Comment on attachment 114056 [details] [review]
Fix cmake 3.1.3 policy CMP0054 warning

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

cmake: opt-in to not implicitly expanding variables in if() (policy CMP0054)

or something?
Comment 9 Ralf Habacker 2015-03-06 19:33:05 UTC
Comment on attachment 114053 [details] [review]
Fix cmake build system bug not generating versioned library name in case LT_REVISION is zero

applied
Comment 10 Ralf Habacker 2015-03-06 19:35:05 UTC
Comment on attachment 114054 [details] [review]
Fix cmake 3.1.3 policy CMP0026 warning

applied with modified commit message
Comment 11 Ralf Habacker 2015-03-06 19:37:36 UTC
Comment on attachment 114055 [details] [review]
Fix cmake 3.1.3 policy CMP0053 warning

applied with modified commit text
Comment 12 Ralf Habacker 2015-03-06 19:37:57 UTC
Comment on attachment 114056 [details] [review]
Fix cmake 3.1.3 policy CMP0054 warning

applied with modified commit text
Comment 13 Simon McVittie 2015-03-10 12:52:55 UTC
All are applied. Is there anything more to do here?
Comment 15 Ralf Habacker 2015-03-17 16:00:48 UTC
(In reply to Simon McVittie from comment #13)
> All are applied. Is there anything more to do here?

There are still a few issues like the missing '=' in the printed information shown below:


 D-BUS 1.9.11                               
 ===========  


I will add related patches soon.
Comment 16 Simon McVittie 2015-03-17 19:31:44 UTC
(In reply to Ralf Habacker from comment #14)
> This one
> http://cgit.freedesktop.org/~smcv/dbus/commit/?h=wip-
> dbussocket&id=73c6219e087d00bcf97d5e94de252a7631a29180 belongs to cmake
> support ?

Yes, or something... the build regressed on cmake 3.0, which is what I have, as a result of the changes seen on this bug.

That branch isn't fully tidied up yet, but if you're OK with that commit, feel free to (consider it to be part of this bug and) apply it to master.

Of the commits in that branch, "Rename _dbus_full_duplex_pipe() to more descriptive name _dbus_socketpair()" is good to apply whenever you want, "Fix assorted compiler warnings on Windows" would ideally be multiple commits but I've been busy with non-D-Bus things (and it would be OK to apply as-is if you approve), and "cmake: only set CMP0053, CMP0054 on CMake >= 3.1" is really this bug.

Subsequent commits, including the fixed version of your "Use typedef DBusSocket for sockets fd's to avoid conversion warnings", shouldn't be applied unless all of them are. I'll say more about that on Bug #89444.
Comment 17 Ralf Habacker 2015-03-24 07:41:01 UTC
(In reply to Ralf Habacker from comment #15)
> I will add related patches soon.
applied to master.
Comment 18 Ralf Habacker 2015-03-24 07:42:06 UTC
(In reply to Simon McVittie from comment #16)

> Of the commits in that branch, "Rename _dbus_full_duplex_pipe() to more
> descriptive name _dbus_socketpair()" is good to apply whenever you want,
applied to master

> "Fix assorted compiler warnings on Windows" would ideally be multiple
> commits but I've been busy with non-D-Bus things (and it would be OK to
> apply as-is if you approve)
rebased and applied to master

> and "cmake: only set CMP0053, CMP0054 on CMake
> >= 3.1" is really this bug.
applied to 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.