Bug 868931

Marcel: what's your FAS username and I'll sponsor you.

Fedora Account System Username: maci

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

Don't use macros for rm, install, chmod and cp.

Remove defattr, if you're not going for EPEL4.

Remove the clean section if you're not going for EPEL5.

Fixed those issues:

Spec URL: http://maci.satgnu.net/rpmbuild/SPECS/sshuttle.spec
SRPM URL: http://maci.satgnu.net/rpmbuild/SRPMS/sshuttle-20121019-2.gitg9ce2fa0.fc17.src.rpm

woops

SRPM URL: http://maci.satgnu.net/rpmbuild/SRPMS/sshuttle-20121019-3.gitg9ce2fa0.fc17.src.rpm

Another update: added some more dependencies

Spec URL: http://maci.satgnu.net/rpmbuild/SPECS/sshuttle.spec
SRPM URL: http://maci.satgnu.net/rpmbuild/SRPMS/sshuttle-20121019-4.gitg9ce2fa0.fc17.src.rpm

fixed manpage generation
minor cleanups


Spec URL: http://maci.satgnu.net/rpmbuild/SPECS/sshuttle.spec
SRPM URL: http://maci.satgnu.net/rpmbuild/SRPMS/sshuttle-20121019-4.gitg9ce2fa0.fc17.src.rpm
Koji URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=4643225

can someone please review this ?

The files you install to /usr/lib should go to %{_datadir} instead.

Please inform upstream of the (long) outdated FSF postal address in LICENSE, if you haven't done so yet.

You might be better off, making the version 0 and add the date in front of the git hash. Otherwise, if a version 0.5, 1.0 or whatever comes out, you'd have to use epochs to make it newer than 20121019.

The Documentation directory contains a manpage. Rather install that instead of including Documentation/sshuttle.md.

Please include the complete commands to create the tarball.

You don't have to BR "make". On the one hand, I think you're not using it; on the other hand it's an exception: http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

- fixed hash ( woops, where did that g come from )
- fixed date to be commit date and not clone date
- use version 0 
- use datadir instead of /usr/local
- remove sshuttle.md in favor of the manpage
- remove make from BR
- added comment on how to create the source tarball


one thing i dont know how to solve is that rpmlint complains about the comment containing a macro bcs of this line:
# checkout=`git log --pretty=format:"%adgit%h" -n1 --date=short|sed 's@-@@g'`

Update:

Spec URL: http://maci.satgnu.net/rpmbuild/SPECS/sshuttle.spec
SRPM URL: http://maci.satgnu.net/rpmbuild/SRPMS/sshuttle-0-6.20120810git9ce2fa0.fc17.src.rpm
Koji URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=4720613

i also upstream about the outdated fsf address

.. woops "informed upstream", sorry for the spam

= Review =

Good:

- rpmlint checks return:
sshuttle.src:2: W: macro-in-comment %adgit
sshuttle.src: W: invalid-url Source0: sshuttle-0-20120810git9ce2fa0.tar.gz
sshuttle.noarch: E: incorrect-fsf-address /usr/share/doc/sshuttle-0/LICENSE
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

Since you've informed upstream about the FSF address issue, and the Source0 is properly documented in comments, and the macro-in-comment warning is a false positive, all of these are safe to ignore.

- package meets naming guidelines
- package meets packaging guidelines
- license (LGPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (manual diff matches upstream checkout)
- package compiles on devel (noarch)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

Minor:

You have %doc %{_mandir}/man8/sshuttle.8.gz. The %doc here is redundant, everything in %{_mandir} is always automatically %doc. Just drop that before you do builds. :)

I also usually prefer that files which end up in the package are not generated within the spec, but rather, included as separate Source files. This isn't strictly against the guidelines, since there are cases where you need to define paths at buildtime (usually involving %{_libdir}), but in this case it really isn't necessary. That said, I won't make you change this behavior, it is up to you how you wish to do it.

Since you seem to already be sponsored, this package is APPROVED.

New Package SCM Request
=======================
Package Name: sshuttle
Short Description: Transparent Proxy VPN
Owners: maci
Branches: f17 f18 f19 el6
InitialCC:

Git done (by process-git-requests).

sshuttle-0-7.20120810git9ce2fa0.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/sshuttle-0-7.20120810git9ce2fa0.el6

sshuttle-0-7.20120810git9ce2fa0.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/sshuttle-0-7.20120810git9ce2fa0.fc18

sshuttle-0-7.20120810git9ce2fa0.el6 has been pushed to the Fedora EPEL 6 testing repository.

sshuttle-0-7.20120810git9ce2fa0.fc18 has been pushed to the Fedora 18 stable repository.

sshuttle-0-7.20120810git9ce2fa0.el6 has been pushed to the Fedora EPEL 6 stable repository.