Bug 875450

Summary: Review Request: sstp-client - SSL based VPN to Microsoft Infrastructure
Product: [Fedora] Fedora Reporter: Andreas Muehlemann <amuehlem>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: axel.thimm, i, mail, misc, notting, package-review
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-06-21 08:09:27 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Andreas Muehlemann 2012-11-11 05:24:10 EST
Spec URL: http://www.muehlemann.net/rpm/sstp-client.spec
SRPM URL: http://www.muehlemann.net/rpm/sstp-client-1.0.8-1.fc17.src.rpm
Description: A client implementation of Secure Socket Tunneling Protocol (SSTP) for Linux / Mac OS-X that allows remote access via SSTP VPN to Microsoft Windows 2008 Server.
Fedora Account System Username: amuehlem

This is my first package for the fedora project. I'm looking for a sponsor who helps me to get this package into the fedora packaging system. I did a test build in koji, the details can be found here http://koji.fedoraproject.org/koji/taskinfo?taskID=4676791

regards
Andreas
Comment 1 Fabian Affolter 2012-11-11 06:28:44 EST
http://www.muehlemann.net/rpm/sstp-client.spec gives me a 403.
Comment 2 Andreas Muehlemann 2012-11-11 17:01:42 EST
(In reply to comment #1)
> http://www.muehlemann.net/rpm/sstp-client.spec gives me a 403.

Sorry, my bad, I only tested the SRPM. It's fixed.
Comment 3 Fabian Affolter 2012-12-09 05:32:58 EST
Just some quick comment:

- There are static libs in your -devel package.
  https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries
- Please check your %changelog entry
  https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
- Please use %global instead of %define
- The rpmlint output is not clean:

[fab@laptop11 review-srpm]$ rpmlint sstp-client-1.0.8-1.fc17.src.rpm 
sstp-client.src: E: description-line-too-long C A client implementation of Secure Socket Tunneling Protocol (SSTP) for Linux / Mac OS-X that allows remote access via SSTP VPN to Microsoft Windows 2008 Server.
sstp-client.src: W: no-version-in-last-changelog
sstp-client.src: W: strange-permission sstp-client.spec 0600L
1 packages and 0 specfiles checked; 1 errors, 2 warnings.
Comment 4 Andreas Muehlemann 2012-12-10 14:36:30 EST
Hi Fabian

Thank you for your input. I've update my SPEC and SRPM accordingly. rpmlint does not show any errors and I disabled the static libraries.

You'll find the updated files here

Spec URL: http://www.muehlemann.net/rpm/sstp-client.spec
SRPM URL: http://www.muehlemann.net/rpm/sstp-client-1.0.8-1.fc17.src.rpm

regards
Andreas
Comment 5 Fabian Affolter 2012-12-20 08:35:06 EST
- Do you plan to include your package in older EPEL releases? If not, please remove the obsolete stuff (e.g. %defattr, Group:, and %clean).

- Please use macros instead of hardcoded path descriptions. Meaning replace /usr/sbin/ with %{_sbindir} and so on.

- Please check if the *.la files are needed by the package.
  https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries

- The unversioned shared system library files must go to the -devel package.
  https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

- man pages do not need to be marked as %doc.

- Please give a brief summary of changes in the changelog about the changes you have made. This way it's much easier to follow the development of the spec file.

If you are still looking for a sponsor, please follow https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Comment 6 Andreas Muehlemann 2012-12-20 10:37:11 EST
Hi Fabian

Why should the unversioned shared system library go to the -devel package?

From the packaging guidelines:
When a shared library file is only provided in an unversioned format, the packager should ask upstream to consider providing a properly versioned library file. However, in such cases, if the shared library file is necessary for users to run programs linked against it, it must go into the base package.

Does this section not cover exactly this package?

Or shall I try to move the unversioned library to a versioned file and replace the unversioned library by a symlink?

Best regards
Andreas
Comment 7 Axel Thimm 2012-12-21 06:31:13 EST
Hi all,

I'm sponsoring Andreas, do I need to remove the FE-NEEDSPONSOR blocker dependency? I guess I should, but it's not mentioned on https://fedoraproject.org/wiki/How_to_sponsor_a_new_contributor, so I better ask. :)

I can't really review the package, though, as I have no access to MS VPN infrastructure. Just a quick note on the versioned/unversioned library discussion: The idea behind the guideline is to keep the runtime dependencies in the non-devel package and move the devel ones to there. Usually this means package the versioned one(s) including the real lib and versioned symlinks to main or libs packages and keep the unversioned symlink to the devel package.

From the discussion it looks like in this case the library is just dumped into an unversioned file, so it is OK to keep it in the main package. The guidelines kindly ask for the packager to also kindly ask of upstream to start versioning their libs in the next release. :)

Welcome to Fedora, Andreas!!!
Comment 8 Axel Thimm 2012-12-22 05:02:21 EST
The FE-NEEDSPONSOR blocker outlines it itself - remove the blocker once sponsored.
Comment 9 Andreas Muehlemann 2012-12-23 06:15:00 EST
I've removed the static libraries, changed the man files and added macros where possible.

I think it makes sense to also include this package in the EPEL repositories, as I expect people using this packages in their company environment.

Updated SPEC and SRPM are on my server

Spec URL: http://www.muehlemann.net/rpm/sstp-client.spec
SRPM URL: http://www.muehlemann.net/rpm/sstp-client-1.0.8-1.fc17.src.rpm
Comment 10 Christopher Meng 2013-06-21 08:09:27 EDT

*** This bug has been marked as a duplicate of bug 976770 ***