Bug 362736 - Compile vs. runtime symbols discrepancy
Summary: Compile vs. runtime symbols discrepancy
Status: RESOLVED FIXED
Alias: None
Product: frameworks-networkmanager-qt
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Jan Grulich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-06 08:17 UTC by Palo Kisa
Modified: 2016-07-02 10:13 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.24


Attachments
Proposed runtime version checks for manager.* (26.63 KB, patch)
2016-05-06 10:27 UTC, Palo Kisa
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Palo Kisa 2016-05-06 08:17:30 UTC
Currently I'm facing a downstream/packaing problem with the networkmanager-qt. Please, take a look on this scenario.

Consider the NetworkManager-qt code:
#if NM_CHECK_VERSION(1, 2, 0)
    /**
     * @return If TRUE, indicates the NetworkManager plugin for the device is likely
     * missing or misconfigured.
     * @since 5.14.0
     */
    bool nmPluginMissing() const;
#endif

If the binary/library package was compiled against networkmanager < 1.2.0, then the nmPluginMissing() symbol is not present in the library.

Fair enough... so we can add the very same conditional into our code:
#if NM_CHECK_VERSION(1, 2, 0)
    connect(device.data(), &NetworkManager::Device::nmPluginMissingChanged, this, &NmModelPrivate::onDeviceUpdated);
#endif  

But if we are compiling our code on a system with networkmanager >= 1.2.0 (e.g. the up-to-date arch, debian unstable...), we are getting the linker error about missing nmPluginMissing() symbol.

One of the solution is to give the networkmanager-qt a *stong*  dependency on the networkmanager (version = build time version).

But I thing it's also worth to report this here as the networkmanager-qt should state/define presence of symbols on its own and not based on current (compile time) presence of networkmanager. 

Thanks.
Comment 1 Palo Kisa 2016-05-06 08:29:19 UTC
Sorry, for the wrong example. For our code, it should be e.g.:
#if NM_CHECK_VERSION(1, 2, 0)
    return dev->nmMissingPlugin();
#endif
Comment 2 Jan Grulich 2016-05-06 08:43:46 UTC
I'm aware of this problem. Problem is that if we define all properties/signals/functions in nm-qt  not based on available NetworkManager then what to return instead or how to behave instead? In your scenario we would need to probably return "false" in nmMissingPlugin() all the time in case nm-qt was build against NM < 1.2.0 which might confuse someone and make him think that it's either broken or he has something broken.
Comment 3 Palo Kisa 2016-05-06 09:05:23 UTC
Networkmanger provides version information in runtime... it's stored in NetworkManager::NetworkManagerPrivate and the NetworkManager::compareVersion() can be used to check. Right?

So it would be feasible to add the runtime version conditional into all such "NM_CHECK_VERSION" functions. And/or e.g. give a note into the doc that signal will not be emited if run with lower version.
Comment 4 Palo Kisa 2016-05-06 10:27:18 UTC
Created attachment 98804 [details]
Proposed runtime version checks for manager.*
Comment 5 Palo Kisa 2016-05-06 10:33:32 UTC
Can you, guys, have a look on the proposed diff/patch for the runtime version checking?

I think such changes can be done in all remaining sources... is it a way to go?
Comment 6 Jan Grulich 2016-05-06 12:42:07 UTC
Submit your patch please to git.reviewboard.org and add "networkmanagement" group and me and Lamarque Souza as reviewers and we will take a look. Thanks.
Comment 7 Palo Kisa 2016-06-14 20:05:46 UTC
FYI ... https://git.reviewboard.kde.org/r/128093/
Comment 8 Lamarque V. Souza 2016-06-19 20:03:43 UTC
Git commit 751f79ea7b054c2fa7d5c1e7d996b70198e2dcc9 by Lamarque V. Souza, on behalf of Palo Kisa.
Committed on 19/06/2016 at 20:03.
Pushed by lvsouza into branch 'master'.

Make network manager version checks in runtime (to avoid compile vs. run-time
discrepancies).
FIXED-IN: 5.24
REVIEW: 128093

M  +1    -1    CMakeLists.txt
M  +18   -35   src/CMakeLists.txt
M  +3    -9    src/accesspoint.cpp
M  +2    -4    src/accesspoint.h
M  +0    -2    src/accesspoint_p.h
M  +0    -19   src/activeconnection.cpp
M  +0    -4    src/activeconnection.h
M  +0    -2    src/activeconnection_p.h
M  +5    -10   src/connection.cpp
M  +1    -8    src/connection.h
M  +0    -2    src/connection_p.h
M  +12   -37   src/device.cpp
M  +13   -28   src/device.h
M  +0    -6    src/device_p.h
M  +116  -134  src/ipconfig.cpp
M  +2    -6    src/ipconfig.h
M  +40   -72   src/manager.cpp
M  +39   -30   src/manager.h
M  +6    -10   src/manager_p.h
M  +4    -32   src/settings.cpp
M  +0    -4    src/settings.h
M  +5    -16   src/settings/bondsetting.cpp
M  +1    -1    src/settings/bondsetting.h
M  +2    -16   src/settings/bridgesetting.cpp
M  +24   -73   src/settings/connectionsettings.cpp
M  +0    -10   src/settings/connectionsettings.h
M  +0    -6    src/settings/connectionsettings_p.h
M  +6    -29   src/settings/gsmsetting.cpp
M  +2    -2    src/settings/gsmsetting.h
M  +0    -12   src/settings/infinibandsetting.cpp
M  +0    -2    src/settings/infinibandsetting.h
M  +0    -2    src/settings/infinibandsetting_p.h
M  +0    -6    src/settings/setting.cpp
M  +6    -16   src/settings/teamsetting.cpp
M  +1    -1    src/settings/teamsetting.h
M  +9    -2    src/settings/tunsetting.cpp
M  +5    -16   src/settings/vlansetting.cpp
M  +1    -1    src/settings/vlansetting.h
M  +5    -16   src/settings/wirelesssetting.cpp
M  +1    -0    src/settings/wirelesssetting.h
M  +0    -7    src/settings_p.h
M  +9    -9    src/vlandevice.cpp
M  +2    -4    src/vlandevice.h
M  +0    -2    src/vlandevice_p.h
M  +0    -14   src/wimaxdevice.cpp
M  +0    -15   src/wirelessdevice.cpp
M  +1    -2    src/wirelessdevice.h

http://commits.kde.org/networkmanager-qt/751f79ea7b054c2fa7d5c1e7d996b70198e2dcc9
Comment 9 Lamarque V. Souza 2016-07-02 10:13:50 UTC
Git commit bb8f19e9617511c4fff449af1daa563c16dd1e7d by Lamarque V. Souza.
Committed on 02/07/2016 at 10:08.
Pushed by lvsouza into branch 'master'.

Fix unit test.

Wimax support was removed in NM 1.2.0.

M  +2    -2    src/manager.cpp

http://commits.kde.org/networkmanager-qt/bb8f19e9617511c4fff449af1daa563c16dd1e7d