[Bug 787561] Review Request: torsocks - A transparent socks proxy for use with tor

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=787561

--- Comment #3 from Petr Šabata <psabata at redhat.com> 2012-02-08 05:17:29 EST ---
> FIX: Don't package static libraries and libtool archives.  Put them into a
> static subpackage if you really have a reason to ship those.  (I guess you'll
> just correct line 34 to remove the rest :) )

Ah, not like this.  See the my SPEC patch below for suggestions.

> TODO: BuildRoot tag, buildroot cleaning in %install and the $clean section
> aren't needed anymore.  Remove them unless you plan to use this package in
> EPELs.
> TODO: The same applies to %defattr in your $files sections.

Ok, EPEL5.

> FIX: Remove the extra '- ' before your name in the %changelog.

The dash between the date and your name is still there.
https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

> FIX: The devel subpackage contains the same files as the base package.  RPM
> possible handles that but it would be better if you shipped them just once.

Again, see the patch below.

> FIX: The %doc macro handles files in the current build directory;  don't list
> bogus absolute paths.  Consider the following simple replacement:
> %doc ChangeLog COPYING README doc/socks
> And possibly this for the devel subpackage:
> %doc doc/notes/DEBUG

Still needs to be fixed.

> FIX: Include the license in the %doc (see above)

Still needs to be fixed.

> FIX: Correct the License tag; it should be GPLv2+; see the source files.

Ok.

> FIX: Don't use hardcoded paths in %files.  Change /etc to %{_sysconfdir}

Ok.

> FIX: The devel subpackage should have an arch-specific base package dependency,
> e.g. Requires: %{name}%{?_isa} = %{version}-%{release}

Ok.

> FIX: Drop the pkgconfig dependency from the devel subpackage.  There are no
> pkgconfig files.

Ok.

> TIP: Use the %{name} macro in URL and Source.

Ok.

And now the promised tip/patch.  This puts libtorsocks.so.1 and
libtorsocks.so.1.0.0 in the base package and the unversioned
libtorsocks.so in the devel subpackage.  That's the way to go.

Note you still need to fix the changelog and your %doc macros.
Those are MUST items.

diff --git a/torsocks.spec b/torsocks.spec
index 3b53fc4..7d59edc 100644
--- a/torsocks.spec
+++ b/torsocks.spec
@@ -30,8 +30,7 @@ rm -rf $RPM_BUILD_ROOT
 make install DESTDIR=$RPM_BUILD_ROOT

 # remove static libraries and libtool droppings
-rm -f $RPM_BUILD_ROOT/%{_libdir}/torsocks/lib%{name}.{a,la,so}
-rm -f $RPM_BUILD_ROOT/%{_libdir}/torsocks/lib%{name}.{so}
+rm -f $RPM_BUILD_ROOT/%{_libdir}/torsocks/lib%{name}.{a,la}*
 rm -f $RPM_BUILD_ROOT/usr/share/*.patch

 %clean
@@ -54,14 +53,13 @@ rm -rf $RPM_BUILD_ROOT
 %doc /usr/share/expectedresults.txt
 %doc /usr/share/run_tests.sh
 %doc /usr/share/socks-extensions.txt
-#%{_libdir}/torsocks/lib%{name}.so.*
+%{_libdir}/torsocks/lib%{name}.so.*
 %config(noreplace) %{_sysconfdir}/torsocks.conf

 %package devel
 Group:         Development/Libraries
 Summary:       The libtsocks library
 Requires:      %{name}%{?_isa} = %{version}-%{release}
-Provides:      torsocks-static = %{version}-%{release}

 %description devel
 Torsocks allows you to use most socks-friendly applications in a safe way with
@@ -71,7 +69,7 @@ These are the files used for development.

 %files devel
 %defattr(-,root,root,-)
-%{_libdir}/torsocks/lib*
+%{_libdir}/torsocks/lib*.so

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.