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 736954 - RPM won't build
RPM won't build
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: Distro-specific
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-09-19 08:53 UTC by Lubomir Rintel
Modified: 2014-10-03 15:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
contrib/rpm: Don't hardwire a single ppp version (1.30 KB, patch)
2014-09-19 08:53 UTC, Lubomir Rintel
committed Details | Review
contrib/rpm: libnm-vpn is no more (1.02 KB, patch)
2014-09-19 08:54 UTC, Lubomir Rintel
committed Details | Review

Description Lubomir Rintel 2014-09-19 08:53:12 UTC
There's essentially two different things to review here. One fixes a build on Rawhide (where ppp is too new), while other one fixes a fallback from libnm-vpn eviction:

commit 8538bcedace602d5b9f42fe57dd22c4999a740d3
Author: Lubomir Rintel <lkundrak@v3.sk>
Date:   Fri Sep 19 10:41:16 2014 +0200

    contrib/rpm: libnm-vpn is no more
    
    Removed in: [280b1e506] libnm: merge libnm-vpn into libnm

commit 56665ae1829bb270545b675c20f08648ac58e72d
Author: Lubomir Rintel <lkundrak@v3.sk>
Date:   Sun Sep 7 23:36:35 2014 +0200

    contrib/rpm: Don't hardwire a single ppp version
    
    Build against whatever is actually present.
Comment 1 Lubomir Rintel 2014-09-19 08:53:43 UTC
Created attachment 286602 [details] [review]
contrib/rpm: Don't hardwire a single ppp version
Comment 2 Lubomir Rintel 2014-09-19 08:54:07 UTC
Created attachment 286603 [details] [review]
contrib/rpm: libnm-vpn is no more
Comment 3 Dan Winship 2014-09-19 12:35:15 UTC
Comment on attachment 286603 [details] [review]
contrib/rpm: libnm-vpn is no more

Attachment 286603 [details] pushed as b769c05 - contrib/rpm: libnm-vpn is no more
Comment 4 Dan Winship 2014-09-19 12:38:24 UTC
Comment on attachment 286602 [details] [review]
contrib/rpm: Don't hardwire a single ppp version

>+%define ppp_version %(pppd --help 2>&1 |awk 'BEGIN {v="bad"} /pppd version/ {v=$NF} END {print v}')

I think we want to keep this more-or-less the same as the actual fedora spec file, and using the package itself to figure out what version of the package to pull in clearly won't work on the build machines. I'd say just add another version clause to the %if. (Or maybe we can just switch to ">=" rather than "="?)
Comment 5 Dan Winship 2014-09-19 12:39:43 UTC
Also, maybe we should make it so that "make check" (or at least "make distcheck") tries to do an RPM build on Fedora? This isn't the first time I've forgotten to fix the spec file after making build changes...
Comment 6 Dan Williams 2014-09-19 15:14:54 UTC
(In reply to comment #4)
> (From update of attachment 286602 [details] [review])
> >+%define ppp_version %(pppd --help 2>&1 |awk 'BEGIN {v="bad"} /pppd version/ {v=$NF} END {print v}')
> 
> I think we want to keep this more-or-less the same as the actual fedora spec
> file, and using the package itself to figure out what version of the package to
> pull in clearly won't work on the build machines. I'd say just add another
> version clause to the %if. (Or maybe we can just switch to ">=" rather than
> "="?)

That doesn't work because we need the ppp version for the pppd-plugin-path.  So we really do either need to auto-detect the version, or hardcode it somewhere in the specfile.

Random idea: build a small configure-time program that #includes pppd/patchlevel.h and uses VERSION to print out the pppd version and use that instead of having to specify it?
Comment 7 Lubomir Rintel 2014-09-19 16:58:39 UTC
(In reply to comment #4)
> (From update of attachment 286602 [details] [review])
> >+%define ppp_version %(pppd --help 2>&1 |awk 'BEGIN {v="bad"} /pppd version/ {v=$NF} END {print v}')
> 
> I think we want to keep this more-or-less the same as the actual fedora spec
> file,

Yep, I'd update this in the Fedora spec as soon as it would pass review.

> and using the package itself to figure out what version of the package to
> pull in clearly won't work on the build machines. I'd say just add another
> version clause to the %if. (Or maybe we can just switch to ">=" rather than
> "="?)

It would work. I removed the version macro from the BR, therefore the build time dependency would be satisfied with any version available, and the macro would only expand for the runtime Requires.

(In reply to comment #5)
> Also, maybe we should make it so that "make check" (or at least "make
> distcheck") tries to do an RPM build on Fedora? This isn't the first time I've
> forgotten to fix the spec file after making build changes...

Well, the SPEC runs make check... distcheck might be a good idea though.

(In reply to comment #6)
...
> Random idea: build a small configure-time program that #includes
> pppd/patchlevel.h and uses VERSION to print out the pppd version and use that
> instead of having to specify it?

We'd still need the version in SPEC though, to generate the Require.

Requested addition of a pkg-config file to fix this in the long run: https://github.com/paulusmack/ppp/pull/22
Comment 8 Thomas Haller 2014-09-24 15:22:37 UTC
I would prefer some hard-coded %if-%else preamble...

%define ppp_version 2.4.5
%if (0%{?fedora}
%if 0%{?fedora} == 20)
%define ppp_version 2.4.6
%else
%define ppp_version 2.4.7
%endif
%endif


I prefer that, because the spec file should produce a similar result to the official packages. And the official spec files should specify the version explicitly, not detecting it.
Comment 9 Lubomir Rintel 2014-10-01 09:47:43 UTC
(In reply to comment #8)
> And the official spec files should specify the version explicitly, not detecting it.

What would be the point of doing that?
Comment 10 Thomas Haller 2014-10-02 14:33:28 UTC
The point would be that as a distribution is something relatively static, also the SPEC file explicitly names the version -- for each distri/release.

Anyway.


I applied the patch as:

http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=53b9504544cbfabb74b97b32ab52f14c7d202d93
Comment 12 Lubomir Rintel 2014-10-03 15:18:27 UTC
Thank you for applying it, as well as subsequent fix.