Bug 85418 - Which features are not checked, and just assumed not to be present under cmake
Summary: Which features are not checked, and just assumed not to be present under cmake
Status: RESOLVED MOVED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.8
Hardware: Other All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-24 20:26 UTC by Ralf Habacker
Modified: 2018-10-12 21:21 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
example config.h autotools cmake diff on linux (7.03 KB, text/plain)
2014-10-24 20:27 UTC, Ralf Habacker
Details
example command line autotools cmake macro diff (4.18 KB, text/plain)
2014-10-24 20:29 UTC, Ralf Habacker
Details
Add cmake macro autoheaderchecks() (2.53 KB, patch)
2015-03-05 08:07 UTC, Ralf Habacker
Details | Splinter Review
Keep include file checks in sync with autotools (3.34 KB, patch)
2015-03-05 08:07 UTC, Ralf Habacker
Details | Splinter Review
Add check to cmake build system if config.h.cmake is in sync with autotools (3.23 KB, patch)
2015-03-05 12:48 UTC, Ralf Habacker
Details | Splinter Review
Move include file checks to ConfigureChecks.cmake for cmake build system. (2.17 KB, patch)
2015-03-05 12:49 UTC, Ralf Habacker
Details | Splinter Review
Keep cmake generated defines for include files in sync with autotools (3.28 KB, patch)
2015-03-05 12:50 UTC, Ralf Habacker
Details | Splinter Review
Check config.h differences with cmake build system only when config.h.in is present (1.02 KB, patch)
2015-03-05 19:49 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 2014-10-24 20:26:45 UTC
While working on bug #73689 it has been recognized that cmake build system 

- does not check the same features as autotools does and
- do not define cpp macros in config.h in the same way as autotools
- do not define cpp macros on the command line in the same way as autotools

which makes it harder to find bugs especially on windows.
Comment 1 Ralf Habacker 2014-10-24 20:27:30 UTC
Created attachment 108368 [details]
example config.h autotools cmake diff on linux
Comment 2 Ralf Habacker 2014-10-24 20:29:15 UTC
Created attachment 108369 [details]
example  command line autotools cmake macro diff
Comment 3 Ralf Habacker 2015-03-05 08:07:06 UTC
Created attachment 114020 [details] [review]
Add cmake macro autoheaderchecks()
Comment 4 Ralf Habacker 2015-03-05 08:07:36 UTC
Created attachment 114021 [details] [review]
Keep include file checks in sync with autotools
Comment 5 Simon McVittie 2015-03-05 12:47:04 UTC
Comment on attachment 114020 [details] [review]
Add cmake macro autoheaderchecks()

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

Interesting approach! Patch looks good, thanks.
Comment 6 Simon McVittie 2015-03-05 12:47:14 UTC
Comment on attachment 114021 [details] [review]
Keep include file checks in sync with autotools

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

Sure, looks good.
Comment 7 Ralf Habacker 2015-03-05 12:48:37 UTC
Created attachment 114029 [details] [review]
Add check to cmake build system if config.h.cmake is in sync with autotools
Comment 8 Ralf Habacker 2015-03-05 12:49:41 UTC
Created attachment 114030 [details] [review]
Move include file checks to ConfigureChecks.cmake for cmake build system.
Comment 9 Ralf Habacker 2015-03-05 12:50:09 UTC
Created attachment 114031 [details] [review]
Keep cmake generated defines for include files in sync with autotools
Comment 10 Simon McVittie 2015-03-05 13:00:31 UTC
Comment on attachment 114029 [details] [review]
Add check to cmake build system if config.h.cmake is in sync with autotools

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

OK
Comment 11 Simon McVittie 2015-03-05 13:00:53 UTC
Comment on attachment 114030 [details] [review]
Move include file checks to ConfigureChecks.cmake for cmake build system.

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

OK
Comment 12 Simon McVittie 2015-03-05 13:03:14 UTC
Comment on attachment 114031 [details] [review]
Keep cmake generated defines for include files in sync with autotools

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

::: cmake/config.h.cmake
@@ +172,5 @@
> +/* Define to 1 if you have ws2tcpip.h */
> +#cmakedefine   HAVE_WS2TCPIP_H
> +
> +/* Define to 1 if you have unistd.h */
> +#cmakedefine   HAVE_UNISTD_H 1

Sure, looks good. Having defines for all the checks is better, and wherever the order is not significant, alphabetical order is good.

Any particular reason why ws2tcpip.h and unistd.h are not in alphabetical order? (Not a merge blocker regardless, the patch is an improvement.)
Comment 13 Ralf Habacker 2015-03-05 13:05:32 UTC
(In reply to Simon McVittie from comment #12)
> Comment on attachment 114031 [details] [review] [review]
> Keep cmake generated defines for include files in sync with autotools
> 
> Review of attachment 114031 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: cmake/config.h.cmake
> @@ +172,5 @@
> > +/* Define to 1 if you have ws2tcpip.h */
> > +#cmakedefine   HAVE_WS2TCPIP_H
> > +
> > +/* Define to 1 if you have unistd.h */
> > +#cmakedefine   HAVE_UNISTD_H 1
> 
> Sure, looks good. Having defines for all the checks is better, and wherever
> the order is not significant, alphabetical order is good.
> 
> Any particular reason why ws2tcpip.h and unistd.h are not in alphabetical
> order? (Not a merge blocker regardless, the patch is an improvement.)

no just oversaw it. Will commit with corrected order.
Comment 14 Ralf Habacker 2015-03-05 13:11:40 UTC
Comment on attachment 114029 [details] [review]
Add check to cmake build system if config.h.cmake is in sync with autotools

applied
Comment 15 Ralf Habacker 2015-03-05 13:11:45 UTC
Comment on attachment 114021 [details] [review]
Keep include file checks in sync with autotools

applied
Comment 16 Ralf Habacker 2015-03-05 13:11:54 UTC
Comment on attachment 114020 [details] [review]
Add cmake macro autoheaderchecks()

applied
Comment 17 Ralf Habacker 2015-03-05 13:12:09 UTC
Comment on attachment 114030 [details] [review]
Move include file checks to ConfigureChecks.cmake for cmake build system.

applied
Comment 18 Ralf Habacker 2015-03-05 13:34:38 UTC
Comment on attachment 114031 [details] [review]
Keep cmake generated defines for include files in sync with autotools

applied
Comment 19 Simon McVittie 2015-03-05 13:37:19 UTC
This isn't working for me on Debian (cmake 3.0.2):

CMake Error at ConfigureChecks.cmake:26 (check_include_files):
  Unknown CMake command "check_include_files".
Call Stack (most recent call first):
  CMakeLists.txt:130 (INCLUDE)

Is that a recently-added macro, or one that needs third-party modules?

<sys/event.h> is for bus/dir-watch-kqueue.c, which is a BSD-specific feature - I don't mind that not working under CMake. Everyone uses Autotools on *BSD anyway.
Comment 20 Ralf Habacker 2015-03-05 13:48:02 UTC
(In reply to Simon McVittie from comment #19)
> This isn't working for me on Debian (cmake 3.0.2):
> 
> CMake Error at ConfigureChecks.cmake:26 (check_include_files):
>   Unknown CMake command "check_include_files".
> Call Stack (most recent call first):
>   CMakeLists.txt:130 (INCLUDE)
> 
> Is that a recently-added macro, or one that needs third-party modules?
> 
> <sys/event.h> is for bus/dir-watch-kqueue.c, which is a BSD-specific feature
> - I don't mind that not working under CMake. Everyone uses Autotools on *BSD
> anyway.

There is an include file CheckIncludeFiles missing in cmake/ConfigureChecks.cmake and already fixed in git master.
Comment 21 Ralf Habacker 2015-03-05 19:39:42 UTC
Comment on attachment 114020 [details] [review]
Add cmake macro autoheaderchecks()

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

::: cmake/CMakeLists.txt
@@ +17,4 @@
>  include(MacrosAutotools)
>  autoinit(../configure.ac)
>  autoversion(dbus)
> +autoheaderchecks(../config.h.in ConfigureChecks.cmake)

Found out on another installation, that ../config.h.in is not included in git repo, but 
generated by autogen.sh

I need to add an additional patch like this
if(EXITST ../config.h.in)
    autoheaderchecks(../config.h.in ConfigureChecks.cmake)
endif()
Comment 22 Ralf Habacker 2015-03-05 19:49:08 UTC
Created attachment 114040 [details] [review]
Check config.h differences with cmake build system only when config.h.in is present
Comment 23 Ralf Habacker 2015-03-06 08:24:26 UTC
(In reply to Simon McVittie from comment #19)
> This isn't working for me on Debian (cmake 3.0.2):
> 
> CMake Error at ConfigureChecks.cmake:26 (check_include_files):
>   Unknown CMake command "check_include_files".
> Call Stack (most recent call first):
>   CMakeLists.txt:130 (INCLUDE)
> 
> Is that a recently-added macro, or one that needs third-party modules?
> 
> <sys/event.h> is for bus/dir-watch-kqueue.c, which is a BSD-specific feature
> - I don't mind that not working under CMake. Everyone uses Autotools on *BSD
> anyway.

This macro has been moved from top level CMakeList.txt into ConfigureChecks.cmake with attachment 114030 [details] [review] and has been introduced with commit http://cgit.freedesktop.org/dbus/dbus/commit/?id=1ec2e5a5387d131efc6affb7bfc49642c8186df8
Comment 24 Simon McVittie 2015-03-06 10:32:32 UTC
Comment on attachment 114040 [details] [review]
Check config.h differences with cmake build system only when config.h.in is present

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

Makes sense
Comment 25 Simon McVittie 2015-03-06 11:23:51 UTC
(In reply to Ralf Habacker from comment #20)
> There is an include file CheckIncludeFiles missing in
> cmake/ConfigureChecks.cmake and already fixed in git master.

Right, commit ee9c52c resolves this for me.
Comment 26 Ralf Habacker 2015-04-20 21:25:24 UTC
(In reply to Simon McVittie from comment #24)
> Comment on attachment 114040 [details] [review] [review]
> Check config.h differences with cmake build system only when config.h.in is
> present
> 
> Makes sense

Has been superseeded by commit b7086e051347cff5a71b63a5010e4c280c4adf4b

The remaining part is to print a cmake status message if config.h.in is not present. As the patch has been already reviewed, I'm going to commit the rest with a modified commit message.
Comment 27 Ralf Habacker 2015-04-20 21:31:03 UTC
Comment on attachment 114040 [details] [review]
Check config.h differences with cmake build system only when config.h.in is present

committed to master
Comment 28 GitLab Migration User 2018-10-12 21:21:57 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/117.


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.