Bug 950282

Created attachment 734156 [details]
Patch to fix spec file

Note about patch:
Contains a deliberate error (TODO) and needs to be replaced with latest known version, because of the package split. As Dan (Williams) mentioned, it is mandatory for updates to work correctly.

Came across this while looking for packages that do scriptlets wrong. Looks good, but a few comments:

1. TODO Obsoletes: NetworkManager-pptp = REPLACE_WITH_LATEST_KNOWN_VERSION

Why? That doesn't appear to make any sense at all. Why would you want NetworkManager-pptp to obsolete...NetworkManager-pptp ?

2. Since upstream claims that disabling the .desktop file and icon is temporary - the stuff in Makefile.am is commented out with the note "uncomment when nmce gets --import support" - wouldn't it be better to comment out rather than remove the bits that are related to that (the Post and PostUn requirements, the scriptlets themselves, and the filenames)? Or are these now never planned to come back (in which case upstream should be adjusted)?

3. While the spec files are being revamped, what about https://bugzilla.redhat.com/show_bug.cgi?id=811930 ? stefw suggests that all these NM plugins should depend on libgnome-keyring, not gnome-keyring.

Thanks!

Ah. dcbw explains the idea is that having Obsoletes statements in both the new -gnome subpackage and the main package will ensure that both are installed on updates.

I think that, since NetworkManager-pptp-gnome will require NetworkManager-pptp, you only need an Obsoletes: statement in NetworkManager-pptp-gnome for things to work. And I think it should be something like:

Obsoletes: NetworkManager-pptp < 1:0.9.3.997-5

rather than using a = (if you use = it will only obsolete one specific version, so won't that break if someone happens to be updating from something older?)

It's quite hard to use = in Obsoletes statements in general because you have to get the whole NEVR precisely correct, so I picked up the trick of just finding the highest build you want to obsolete, adding 1 to the revision, and using <.

I just play a packaging expert on TV, though, so you might want to run this by Spot or Toshio or someone.

Hey Adam,

Thanks for your comments. I an definitely not an expert and this ticket was opened as a suggestion in order to conform better with packaging standards. I'm still on the "steep learning curve" side of things...

1. The 'Obsoletes:' line came after dcbw suggested me to add it. I understand your concern in your second comment however I think I'm not the best person to comment as I am not very familiar with upgrading and obsoleting...

2. Can comment it if you insist, but I personally don't like unused commented clutter. Comments are comments, not features for future integration of who knows what. It'll be in the git history should we ever require to add it again.

3. Makes sense. Can definitely change it if we're sure about it. Perhaps also make the other bug depending on this one?

Happy to prepare new patches in all of these tickets once these issues are resolved.

Cheers!

Hi Dan!

1: it may be best if we just run this by someone on the packaging committee as a tie breaker I guess :) I think I'm right, but I don't want to tell you guys to do something that turns out to be wrong...

2: oh, I don't insist on anything, I'm just sticking my nose in :) Usually I like clean specs too, I hate when people comment out dropped patches and stuff, but I do like commenting out stuff that's explicitly _temporarily_ disabled because it acts as a reminder to put it back in. If you have the .desktop-related things in there but commented out, then as soon as you do a build which fails because upstream activated that stuff again, you'll know "hey, I should re-enable all these other bits too". The git history is a bit too 'hidden' for that purpose, it's not looking you in the face all the time. Just my opinion, though, like I said. It may be worth clarifying with dcbw whether he's still actually planning to turn that stuff back on upstream, or whether it's kind of dead now.

3: Sure, we could do that - again I'd probably check with dcbw if he thinks stefw is correct, if there's not some special reason for the dep, but it looks like it might as well be changed.

thanks for taking my thoughts into consideration!

NetworkManager-pptp-0.9.8.2-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/NetworkManager-pptp-0.9.8.2-2.fc19

Package NetworkManager-pptp-0.9.8.2-2.fc19:
* should fix your issue,
* was pushed to the Fedora 19 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing NetworkManager-pptp-0.9.8.2-2.fc19'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-12769/NetworkManager-pptp-0.9.8.2-2.fc19
then log in and leave karma (feedback).

*** Bug 913860 has been marked as a duplicate of this bug. ***

NetworkManager-pptp-0.9.8.2-2.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.