After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755660 - [review] libnm-glib: make "nm-version.h" independent from "glib.h" and include in "NetworkManager.h"
[review] libnm-glib: make "nm-version.h" independent from "glib.h" and includ...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-09-26 11:07 UTC by Thomas Haller
Modified: 2015-09-30 21:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[patch-v1] libnm-glib: make "nm-version.h" independent from "glib.h" and include in "NetworkManager.h" (5.72 KB, patch)
2015-09-26 11:07 UTC, Thomas Haller
none Details | Review

Description Thomas Haller 2015-09-26 11:07:41 UTC
Created attachment 312190 [details] [review]
[patch-v1] libnm-glib: make "nm-version.h" independent from "glib.h" and include in "NetworkManager.h"

libnm-glib: make "nm-version.h" independent from "glib.h" and include in "NetworkManager.h"
    
    For libnm-glib library, "NetworkManager.h" contains defines like the D-Bus
    paths of NetworkManager. It would be desirable to have this header usable
    without having a dependency on "glib.h", for example for a QT application.
    For that, commit 159e827a72f420048e12d318f8ba1edd3f641fc8 removed that include.
    
    However, that broke build on PackageKit [1] which expected to get the
    version macros by including "NetworkManager.h". So let's fix it differently,
    by "NetworkManager.h" still including "nm-version.h", but the latter
    now being usable without requiring "glib.h".
    
    Note that for libnm library, the header "NetworkManager.h" is named "nm-dbus-interface.h".
    The change to libnm-glib is done for backward compatibility. For libnm
    this is solved differently as there are probably no users affected by this
    (commit c0852964a890cf43cc2dcaeff41ac6edc5028f24).
    On libnm, you can include "nm-dbus-interface.h" alone, which will not include
    anything else. Especially, the user can copy the file "nm-dbus-interface.h" from
    the source tree without needing the dependency "nm-version.h" or the libnm-devel
    package.
    Regarding "nm-version.h", it still can be used alone, but it will include "glib.h".
    There is no reason to use "nm-version.h" without also using libnm or glib.
    
    [1] https://github.com/hughsie/PackageKit/issues/85
    
    Fixes: 159e827a72f420048e12d318f8ba1edd3f641fc8
Comment 2 Thomas Haller 2015-09-26 11:49:50 UTC
for easier review, patch is here:
th/include-nm-version-bgo755660


http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=th%2Finclude-nm-version-bgo755660
Comment 3 Dan Winship 2015-09-27 15:14:31 UTC
If you're going to do the complicated thing in libnm-glib, you might as well do it in libnm too?

Alternative, the dbus includes only include nm-version.h to get NM_MAJOR_VERSION, NM_MINOR_VERSION, and NM_MICRO_VERSION, so you could either (a) split the deprecation macros out into a different file, or (b) define those in both nm-version.h and nm-dbus-interface.h (with appropriate handling for the case where both get included).