Bug 175433 - Review Request: tor - Anonymizing overlay network for TCP (The onion router)
Review Request: tor - Anonymizing overlay network for TCP (The onion router)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Toshio Kuratomi
David Lawrence
: Reopened
: 175799 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-12-10 07:12 EST by Enrico Scholz
Modified: 2009-05-31 16:17 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-10-01 07:32:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Enrico Scholz 2005-12-10 07:12:31 EST
Spec Name or Url: http://ensc.de/fedora/tor.spec
SRPM Name or Url: http://ensc.de/fedora/tor-0.1.0.15-1.src.rpm
Description:

Tor is a connection-based low-latency anonymous communication system.

Applications connect to the local Tor proxy using the SOCKS protocol. The
local proxy chooses a path through a set of relays, in which each relay
knows its predecessor and successor, but no others. Traffic flowing down
the circuit is unwrapped by a symmetric key at each relay, which reveals
the downstream relay.

Warnings: Tor does no protocol cleaning.  That means there is a danger
that application protocols and associated programs can be induced to
reveal information about the initiator. Tor depends on Privoxy and
similar protocol cleaners to solve this problem. This is alpha code,
and is even more likely than released code to have anonymity-spoiling
bugs. The present network is very small -- this further reduces the
strength of the anonymity provided. Tor is not presently suitable for
high-stakes anonymity.

This package provides the "tor" program, which serves as both a client
and a relay node.
Comment 1 Enrico Scholz 2005-12-15 01:49:34 EST
* Wed Dec 14 2005 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> -
0.1.0.15-1.1
- added -minit subpackage


http://ensc.de/fedora/tor.spec
http://ensc.de/fedora/tor-0.1.0.15-1.1.src.rpm
Comment 2 Enrico Scholz 2005-12-15 14:53:05 EST
* Thu Dec 15 2005 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.0.15-1.2
- use relative UID of 19 instead of 18 due to conflicts with the
  'munin' package


http://ensc.de/fedora/tor.spec
http://ensc.de/fedora/tor-0.1.0.15-1.2.src.rpm
GNU Arch: ensc@ensc.de--fedora (http://ensc.de/tla/{archives}/fedora)
          tor--review--0
Comment 3 Ville Skyttä 2005-12-15 15:09:18 EST
*** Bug 175799 has been marked as a duplicate of this bug. ***
Comment 4 Kevin Fenzi 2005-12-22 23:04:54 EST
Good: 
- sources match up with upstream. 
- builds on fc4/devel
- license is good and included (BSD)
- names good

Nits:
- why use the 'release_func'? and why a 1.2 release? integers are more standard
and not having a release_func would make things more readable (IMHO). 
- minit and initng aren't in extras/core yet, might leave that out until they
are added and then add support for those in? 
- Might fold the 'lsb' package into the main package? I would expect a package
to have a init script in the main package. 
- Changelog might be included in docs?

Needs work:
- rpmlint output: 
these can be ignored: 

E: tor non-standard-gid /var/log/tor toranon
E: tor non-standard-uid /var/lib/tor toranon

Do these need to have these permissions? 
E: tor non-standard-dir-perm /var/log/tor 0730
E: tor non-standard-dir-perm /var/lib/tor 0700

Can you add a logrotate file?
W: tor log-files-without-logrotate /var/log/tor

Perhaps fold this into the main package to avoid this:
W: tor-lsb no-documentation

Looks like init script isn't right according to rpmlint: 
W: tor-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/tor
E: tor-lsb executable-marked-as-config-file /etc/rc.d/init.d/tor
E: tor-lsb postin-without-chkconfig /etc/rc.d/init.d/tor
E: tor-lsb preun-without-chkconfig /etc/rc.d/init.d/tor
W: tor-lsb incoherent-init-script-name tor

- Doesn't build in mock: missing BuildRequires of 'ghostscript' ?
make: Entering directory `/builddir/build/BUILD/tor-0.1.0.15/doc/design-paper'
fig2dev -L pdf cell-struct.fig cell-struct.pdf
sh: gs: command not found
fig2dev: broken pipe (GhostScript aborted?)
command was: gs -q -dNOPAUSE -sAutoRotatePages=None -sDEVICE=pdfwrite
-sOutputFile=cell-struct.pdf - -c quit
make: *** [cell-struct.pdf] Error 1
make: Leaving directory `/builddir/build/BUILD/tor-0.1.0.15/doc/design-paper'
error: Bad exit status from /var/tmp/rpm-tmp.66579 (%build)
Comment 5 Enrico Scholz 2005-12-23 05:26:15 EST
* Fri Dec 23 2005 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.0.15-1.8
- added ChangeLog to %doc
- made torrc not world-readable
- added logrotate script

* Thu Dec 22 2005 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.0.15-1.4
- updated initng scripts to initng-0.4.8 syntax
- tweaked some Requires(...):
- added ghostscript BuildRequires:
- install initng scripts into the correct dir

http://ensc.de/fedora/tor.spec
http://ensc.de/fedora/tor-0.1.0.15-1.8.src.rpm


=====================


> - why use the 'release_func'?

there is no big difference between the '%{?dist}' annotation and my
%release_func. But the %release_func gives some more control over the
resulting Release: and all of my packages are using it.


>   and why a 1.2 release?

Will become '2' at first CVS checkin. I just want to avoid
release-inflation during the review.


> - minit and initng aren't in extras/core yet, might leave that out
>   until they are added and then add support for those in?

Both initstyles are disabled by default. But they are both used on my
systems and 'initng' is under review (bug #173459) so it may appear
soon at least in Extras.


> - Might fold the 'lsb' package into the main package? 

I would like to avoid it. It brings lot of huge dependencies which are
useless when you want to use 'tor' with 'initng' or 'minit'.


> - Changelog might be included in docs?

done


> Do these need to have these permissions? 
> E: tor non-standard-dir-perm /var/log/tor 0730
> E: tor non-standard-dir-perm /var/lib/tor 0700

can be ignored IMO.


> Can you add a logrotate file?
> W: tor log-files-without-logrotate /var/log/tor

done


> Looks like init script isn't right according to rpmlint: 
> W: tor-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/tor
> E: tor-lsb executable-marked-as-config-file /etc/rc.d/init.d/tor

initscripts ARE %config but not %config(noreplace) usually


> E: tor-lsb postin-without-chkconfig /etc/rc.d/init.d/tor
> E: tor-lsb preun-without-chkconfig /etc/rc.d/init.d/tor
> W: tor-lsb incoherent-init-script-name tor

looks like bugs in rpmlint; the lsb initscript will be (un)registered
properly and it is named 'tor'.


> - Doesn't build in mock: missing BuildRequires of 'ghostscript' ?

fixed
Comment 6 Paul Howarth 2005-12-23 05:31:02 EST
(In reply to comment #5)
> * Fri Dec 23 2005 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - >
> Looks like init script isn't right according to rpmlint: 
> > W: tor-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/tor
> > E: tor-lsb executable-marked-as-config-file /etc/rc.d/init.d/tor
> 
> initscripts ARE %config but not %config(noreplace) usually

Really? I thought usually they weren't %config and shouldn't be edited, any
config changes being managed using entries in /etc/sysconfig/somefile


Comment 7 Enrico Scholz 2005-12-23 06:47:14 EST
> > initscripts ARE %config but not %config(noreplace) usually
> 
> Really? I thought usually they weren't %config and shouldn't be edited,
> any config changes being managed using entries in /etc/sysconfig/somefile

I could not find an explicit rule, but afais, most initscripts in Core
are plain %config. This seems reasonable because:

* you can make changes which remain across package updates (when the
  initscript did not changed between the versions)
* changes will be backuped in .rpmsave files
Comment 8 Kevin Fenzi 2005-12-23 14:08:34 EST
>there is no big difference between the '%{?dist}' annotation and my
>%release_func. But the %release_func gives some more control over the
>resulting Release: and all of my packages are using it.

ok. No big deal there. ;) 

>>   and why a 1.2 release?

>Will become '2' at first CVS checkin. I just want to avoid
>release-inflation during the review.

ok. Reasonable. 

>> - minit and initng aren't in extras/core yet, might leave that out
>>   until they are added and then add support for those in?

>Both initstyles are disabled by default. But they are both used on my
>systems and 'initng' is under review (bug #173459) so it may appear
>soon at least in Extras.

Indeed, but they aren't currently in, so why add them now? You can re-add those
spec file bits and rebuild when/if they are included. Until then they are just
dead weight in the spec file. 

>> - Might fold the 'lsb' package into the main package? 

>I would like to avoid it. It brings lot of huge dependencies which are
>useless when you want to use 'tor' with 'initng' or 'minit'.

Yeah, but doesn't every other package in core and extras with an init.d script
already do that? I can't think of any other packages that have an init.d script
that package it seperately. Or am I just missing some dependency there? 

>> E: tor-lsb postin-without-chkconfig /etc/rc.d/init.d/tor
>> E: tor-lsb preun-without-chkconfig /etc/rc.d/init.d/tor
>> W: tor-lsb incoherent-init-script-name tor

>looks like bugs in rpmlint; the lsb initscript will be (un)registered
>properly and it is named 'tor'.

I think it's just mad because the package is 'tor-lsb' and the init.d file is
'tor'... 

Instead of the /etc/tor/.have-lsb, /etc/tor/.have-initng, /etc/tor/.have-minit, 
how about a '/bin/kill -HUP `cat /var/run/tor.pid 2>/dev/null` 2> /dev/null ||
true' (like httpd uses). That would work under any of those init schemes I would
think. 

With the ghostscript BR added, builds fine nuder mock. 

Works fine on my test machine. It's even not very slow for web browsing. :)

Thanks for working on this package... will be a good one to have. 
Comment 9 Enrico Scholz 2005-12-23 15:36:04 EST
* Fri Dec 23 2005 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.0.15-1.11
- reworked the 'setgroups' patch so that 'tor' survives a SIGHUP
- (re)added the 'reload' functionality to the lsb initscript and use
  it in logrotate

http://ensc.de/fedora/tor.spec
http://ensc.de/fedora/tor-0.1.0.15-1.11.src.rpm


===========


> >Both initstyles are disabled by default. But they are both used on my
> >systems and 'initng' is under review (bug #173459) so it may appear
> >soon at least in Extras.
> 
> Indeed, but they aren't currently in, so why add them now? You
> can re-add those spec file bits and rebuild when/if they are
> included. Until then they are just dead weight in the spec file.

When I would remove the -initng/-minit subpackages somebody would
probably ask why I have a single -lsb subpackage while no other init
methods are provided ;)


> >I would like to avoid it. It brings lot of huge dependencies which
> >are useless when you want to use 'tor' with 'initng' or 'minit'.
> 
> Yeah, but doesn't every other package in core and extras with an
> init.d script already do that? I can't think of any other packages
> that have an init.d script that package it seperately. Or am I just
> missing some dependency there?

I really want to avoid the bloaty initscripts/lsb; they add lot of
stuff which is unneeded and there is no chance to modularize this
package.

'initscripts' itself is not tight very much into the system; e.g. my
tor-vserver (with 'tor-minit') has it only because of

| $ LANG=C vrpm cheese -- -e --test initscripts 
| error: Failed dependencies:
|         initscripts >= 3.94 is needed by (installed) pam-0.79-9.6.i386
|         initscripts >= 8.11-1 is needed by (installed) hotplug-2004_09_23-7.i386

'hotplug' is brought in by 'initscripts' and I just submitted a bug
#176508 to remove the dependency in 'pam'.


> Instead of the /etc/tor/.have-lsb, /etc/tor/.have-initng,
> /etc/tor/.have-minit, how about a '/bin/kill -HUP `cat
> /var/run/tor.pid 2>/dev/null` 2> /dev/null || true' (like httpd
> uses).

Does not work because 'initng' and 'minit' do not need/provide a
pidfile. I find it also a little bit ... mmmh ... risky to send
signals to a foreign process just because its pid appears in some
file.
Comment 10 Enrico Scholz 2006-01-04 14:38:37 EST
* Mi Jan 04 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.0.16-0
- updated to 0.1.0.16

http://ensc.de/fedora/tor.spec
http://ensc.de/fedora/tor-0.1.0.16-0.src.rpm
Comment 11 Kevin Fenzi 2006-01-05 21:53:06 EST
Sorry for the delay in continuing the review. :( 

>I really want to avoid the bloaty initscripts/lsb; they add lot of
>stuff which is unneeded and there is no chance to modularize this
>package.
>
>'initscripts' itself is not tight very much into the system; e.g. my
>tor-vserver (with 'tor-minit') has it only because of
>
>| $ LANG=C vrpm cheese -- -e --test initscripts 
>| error: Failed dependencies:
>|         initscripts >= 3.94 is needed by (installed) pam-0.79-9.6.i386
>|         initscripts >= 8.11-1 is needed by (installed) >hotplug-2004_09_23-7.i386
>
>'hotplug' is brought in by 'initscripts' and I just submitted a bug
>#176508 to remove the dependency in 'pam'.

Well, thats your system... on my pretty much default fedora devel test box I get: 

[root@testbox ~]# rpm -e --test initscripts
error: Failed dependencies:
        initscripts >= 5.83 is needed by (installed) glibc-kernheaders-3.0-2.i386
        initscripts >= 7.22-1 is needed by (installed) rhgb-0.16.2-12.i386
        initscripts >= 8.04-1 is needed by (installed) hal-0.5.5.1-2.1.i386
        initscripts >= 5.86-1 is needed by (installed) kbd-1.12-12.1.i386
        initscripts >= 8.11-1 is needed by (installed) hotplug-2004_09_23-10.1.i386
        initscripts >= 7.31.11.EL-1 is needed by (installed)
ipsec-tools-0.6.3-1.1.i386
        initscripts is needed by (installed) bluez-utils-2.22-2.1.i386
        initscripts >= 5.54 is needed by (installed) portmap-4.0-65.1.i386
        initscripts is needed by (installed) xorg-x11-xfs-1.0.0-1.i386
        initscripts is needed by (installed) xinetd-2.3.13-6.1.i386
        initscripts >= 0:5.54-1 is needed by (installed) samba-3.0.20b-2.1.i386
        initscripts >= 0:5.99 is needed by (installed)
system-config-network-tui-1.3.30-2.1.noarch
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1771_FC5.i686
        initscripts >= 6.38 is needed by (installed) quota-3.13-1.1.i386
        initscripts >= 0:5.99 is needed by (installed)
system-config-packages-1.2.25-1.1.noarch
        initscripts >= 7.73 is needed by (installed) dhcpv6_client-0.10-15.2.i386
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1773_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1776_FC5.i686
        initscripts >= 5.20 is needed by (installed) openssh-4.2p1-10.i386
        initscripts is needed by (installed) system-config-printer-0.6.146-1.i386
        initscripts >= 6.75 is needed by (installed) dhclient-3.0.3-18.i386
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1777_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1783_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1800_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1805_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1806_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1807_FC5.i686
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.14-1.1808_FC5.i686
        initscripts is needed by (installed) foomatic-3.0.2-30.i386
        initscripts >= 8.11.1-1 is needed by (installed)
kernel-2.6.15-1.1819_FC5.i686
        /bin/usleep is needed by (installed) bind-9.3.2-1.i386
        /sbin/service is needed by (installed) athcool-0.3.11-3.fc5.i386
        /sbin/service is needed by (installed) distcache-1.4.5-12.1.i386
        /sbin/service is needed by (installed) bluez-utils-2.22-2.1.i386
        /sbin/service is needed by (installed) acpid-1.0.4-1.1.i386
        /sbin/service is needed by (installed) vixie-cron-4.1-42.FC5.i386
        /sbin/service is needed by (installed) smartmontools-5.33-4.i386
        /sbin/service is needed by (installed) xorg-x11-xfs-1.0.0-1.i386
        /sbin/service is needed by (installed) xinetd-2.3.13-6.1.i386
        /sbin/service is needed by (installed) spamassassin-3.1.0-3.fc5.1.i386
        /sbin/service is needed by (installed) rng-utils-2.0-1.9.1.i386
        /sbin/service is needed by (installed) irqbalance-1.12-1.21.1.i386
        /sbin/service is needed by (installed) cpuspeed-1.2.1-1.28.i386
        /sbin/service is needed by (installed) microcode_ctl-1.12-1.27.1.i386
        /sbin/service is needed by (installed) cups-1.1.23-27.i386
        /sbin/service is needed by (installed) cyrus-sasl-2.1.21-9.i386
        /sbin/service is needed by (installed) yum-2.4.1-1.fc4.noarch
        /sbin/service is needed by (installed) munin-node-1.2.4-5.fc5.noarch

If you have a need to remove that, wouldn't you just keep a local version of the
package that doesn't depend on initscripts/lsb? No need to do that for every
fedora-extras using box, is there?

>Does not work because 'initng' and 'minit' do not need/provide a
>pidfile. I find it also a little bit ... mmmh ... risky to send
>signals to a foreign process just because its pid appears in some
>file.

Yeah, I got that from the httpd package, but I agree it's kinda odd. 
The reload in logrotate sounds like a fine way to go. 

The 0.1.0.16-0 version builds fine under mock and works great on my test machine. 

Anyone else want to give thoughts on the tor-lsb subpackage?
Thats the only issue left I see on this package.
Comment 12 Enrico Scholz 2006-01-06 20:25:06 EST
> >'initscripts' itself is not tight very much into the system; e.g. my
> >tor-vserver (with 'tor-minit') has it only because of
> 
> Well, thats your system... on my pretty much default fedora devel
> test box I get:

ok, devel test boxes and tor servers are having completely different
requirements. E.g. the devel box needs gcc, editors, perhaps cups and
a complete networking.

The tor server does not need these programs (which add complexity and
additional potential points of failures). tor server and clients should 
not have a complete networking (e.g. no DNS, fake hostname) to increase
anonymity.


> If you have a need to remove that, wouldn't you just keep a local
> version of the package that doesn't depend on initscripts/lsb?

Current packaging (with split lsb and initng initmethod) satisfies both
needs: Those who want to use tor on a devel test box can install the bloaty
lsb subpackage, while those who want to use it productivily (whatever that
means for "tor") can install the minit/ining subpackages with minimal deps.
Comment 13 Paul Wouters 2006-01-08 15:14:27 EST
You are proposing a package for FE, not "my devel box". There are also no
provisions for Suse or Mandrake type startup sequences. IMHO, splitting tor in
tor-lsb is just going to confuse users who are running this on FE, and not some
devel box who want to install "tor" and not "tor-lsb". This is not "bloat" on FE
systems, for which you are submitting a package. 

other comments:

cat <<EOF >>src/config/torrc.sample.in

I personally dislike creating files from the spec file. They should be included
as  a SOURCE file. (some of these constucts fail in a non-interactive shell as well)

tor is a very common name and often already exists on Scandinavian boxes. This
will cause tor to fail during install. This is why my tor rpm/spec uses _tor
instead of tor as user/group. Alternative torproxy or tordaemon could be used.

tor uses libevent, but (wrongly) staticly links this. It should not do this and
depend on the libevent package, which is part of FE.

SOURCE1 is defined but not used. It should be removed.
Comment 14 Enrico Scholz 2006-01-08 15:35:03 EST
Sorry, I do not see about which spec file you are speaking. There will neither a
file created in the described way, nor libevent be linked statically nor a user
'tor' be created.

When SuSE and Mandriva are LSB compliant there is a startup sequence for them in
the -lsb subpackage.

It is a good practice to include gpg signatures into the package to easy
verification of the source tarballs.
Comment 15 Kevin Fenzi 2006-01-16 14:43:39 EST
I don't see any blockers left. The comments in #13 seem to be based on a
diffrent .spec file. Paul, if you could look again based on the spec in comment
#10, that would be great. 

I don't like the lsb subpackage, but I don't see any technical grounds for that
being a blocker. I wouldn't do things that way, but the package does work and
meet the package guidelines. 

I'll give a day or two for any additional comments, and then approve if no
blockers come up. 

Comment 16 Paul Wouters 2006-01-17 20:47:04 EST
I still see a file being created in the spec file:

%prep
%setup -q
%patch0 -p1 -b .setgroups

sed -i -e 's!^\# *\(Log notice file \)!\1!;
           s!^\(\# *\)\?DataDirectory .*!DataDirectory %homedir/.tor!'
src/config/torrc.sample.in
cat <<EOF >>src/config/torrc.sample.in
Group %username
User  %username
EOF

I think that should be a separate SOURCE file.

It uses a harcoded user/group id of 19. I am not sure what the official policy
is for creating users, but I don't think it is needed to create them with a
globally set userid, since tor does not span its files over multiple servers.
Just giving the toranon user /sbin/nologin, as already done, should be enough.
Perhaps there is some "fedora registry" and some policy somewhere for requesting
static userids? 

BuildRequires has libevent-devel, but Requires does not (because it is using
libevent statically linked, which is against fedora policies. (I did inform
Roger, the main tor developer, of this as well. they should not do this). This
needs to be fixed. see further:
https://www.redhat.com/archives/fedora-extras-list/2005-November/msg00386.html

I dont think circular dependancies are cool. Currently "tor" requires "tor-lsb"
and "tor-lsb" requires "tor". I thnk as an FE package, the initscripts should
just come with the tor package. If FC/FE migrates to another system, it can be
changed later with everything else.

source1 (the gpg signature) is defined but not used. It should probably either
not be defined, or it should be used to actualy gpg check the source file in the
prep stage.

Missing chkconfig --add / --delete in %post, %prun, %postun
Missing: Requires(post): /sbin/chkconfig
Missing: Requires(preun): /sbin/chkconfig, /sbin/service
Missing: Requires(postun): /sbin/service


The username/group toranon is good and will prevent conflicts with the common
name tor.

Daemon correctly ony listens on localhost in the default configuration.


  
Comment 17 Paul Wouters 2006-01-17 23:16:39 EST
Actually, libevent is no longer statically linked. But it still needs a Requires:
Comment 18 Enrico Scholz 2006-01-18 12:18:23 EST
> I still see a file being created in the spec file:
> ...
> cat <<EOF >>src/config/torrc.sample.in

There won't be a file *created* but an existing one be modified. I do
not see how this can be done shorter or clearer with a separate SOURCE

> It uses a harcoded user/group id of 19...

Regarding the uid, please see
	  http://fedoraproject.org/wiki/PackageUserRegistry
	  http://fedoraproject.org/wiki/PackageUserCreation


> Actually, libevent is no longer statically linked. But it still
> needs a Requires:

An explicit  Requires: for libevent is not needed because of:
| $ rpm -qR tor
| ...
| libevent-1.1a.so.1  


> I dont think circular dependancies are cool...

Circular dependencies can not be avoided, Fedora Core is full of them
and rpm/smart/yum works fine with them. They are needed because:

a) tor needs some init-scripts
b) the init-scripts need the tor daemon

Because different init-methods are possible which bring in non-trivial
dependencies the init-scripts are in separate subpackages.


> source1 (the gpg signature) is defined but not used. it should
> probably either not be defined, or it should be used to actualy gpg
> check the source file in the prep stage.

It makes no sense to check the GPG signature in %prep. The buildsystem does
know neither the associated gpg key, nor does it define the trust. The
shipped GPG signature is for reviewers only who want to verify the tarball.


> Missing chkconfig --add / --delete in %post, %prun, %postun

The -lsb subpackage has the needed Requires(...): to register/unregister
the lsb initscript.
Comment 19 Kevin Fenzi 2006-01-28 14:58:25 EST
Sorry for the delay on this Enrico. :(

I think you addressed all of Paul's comments. 
I will give another day or two to solicit any additional comments before I
approve the package. 

In particular one of the tor developers mailed me asking if the package could be
closer to the upstream spec (I think he was refering to not having the -lsb
subpackage). I told him to comment here with any concerns he had. 

I could see the -lsb subpackage causing problems for upstream. If they link to
the extras rpms from their web page and someone downloads the main rpm, they
could easily miss the tor-lsb link and get confused trying to install it. If
they build from the src.rpm and don't make sure and also install the tor-lsb
package they could get confused. 
The message you get when trying to install just the main tor rpm is not very
helpfull to neophyte user: 

error: Failed dependencies:
        init(tor) is needed by tor-0.1.0.16-0.fc5.i386

Would you reconsider again the tor-lsb subpackage? While it might be useful for
your local env, it's not what most fedora users would expect. NO other package
that I can see does things this way either. Perhaps once init-ng gets accepted
we should start looking at moving packages to a setup like this, but right now
it just makes this package not work as people expect. 
Comment 20 Roger Dingledine 2006-01-28 18:07:05 EST
Hi folks,

I'm the Tor project leader, Roger Dingledine ("the upstream").

I've been trying to keep an eye on this discussion. What I most want
from this is an rpm that I can point people running Red Hat like systems
to. So anything that confuses people on various RPM-based systems is
bad. (This seems to include the lsb stuff, if what Kevin says is right.)

(Enrico's point about running the RPM on a system without gcc, make, etc
is odd, since most people I know use their computer for many things at
once.)

We currently ship an RPM spec file that seems to work pretty well. The
only problem is that we don't have an official unified maintainer that
we know to maintain it, build new RPMs, and so on. Keeping the Fedora
Extras spec similar to this would help in sending patches upstream
and downstream.

I'm also concerned about dependencies like fedora-groupadd. Is this
going to be an RPM that is only useful for people running a particular
configuration of Fedora, or can we make it more general? If it is too
niche, then we will end up with competing RPMs, which is bad.
Comment 21 Enrico Scholz 2006-02-04 11:35:22 EST
* Mon Jan 30 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.0.16-0.1
- renamed the current main-package into a '-core' subpackage and
  created a new main-package which requires both the 'tor-core'
  subpackage and this with the current default init-method. This
  allows 'yum install tor' to work better; because yum is not very
  smart, the old packaging might install unwanted packages else.

http://ensc.de/fedora/tor.spec
http://ensc.de/fedora/tor-0.1.0.16-0.1.src.rpm

============

An rpm package which is linked on the Tor homepage and which would run on
every (LSB compliant) system is possible. But it would require heavily
discouraged things like static linking of non-LSB deps (e.g. libevent).

So, this review is for a package in the Fedora Extras environment. This
environment provides all the packages required for my packaging.

The tor homepage does not link to Debian, Gentoo or *BSD binaries
either but tells the installation command. For this package, the
corresponding command is 'yum install tor'. The current packaging
will also install the current default init-method but allows still
minimal environments with more effective init methods.
Comment 22 Kevin Fenzi 2006-02-05 14:10:05 EST
>* Mon Jan 30 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> -
0.1.0.16-0.1
>- renamed the current main-package into a '-core' subpackage and
>  created a new main-package which requires both the 'tor-core'
>  subpackage and this with the current default init-method. This
>  allows 'yum install tor' to work better; because yum is not very
>  smart, the old packaging might install unwanted packages else.

Humm.. Can you elobrate on what situation would result in unwanted packages?
I think this is a regression from the package in comment #10.
I would prefer just one tor package (with the lsb stuff), but I don't
see how adding additional packages helps anything.

>An rpm package which is linked on the Tor homepage and which would run on
>every (LSB compliant) system is possible. But it would require heavily
>discouraged things like static linking of non-LSB deps (e.g. libevent).

Quite possibly.

>So, this review is for a package in the Fedora Extras environment. This
>environment provides all the packages required for my packaging.

Agreed. This is specifically a Fedora Extras package.
In my mind this would be an argument against having a tor-lsb subpackage
as well, since fedora-extras doesn't have (yet) any other init setups.
This tor-lsb subpackage seems only needed for your setup, not for
Fedora Extras.

>The tor homepage does not link to Debian, Gentoo or *BSD binaries
>either but tells the installation command. For this package, the
>corresponding command is 'yum install tor'. The current packaging
>will also install the current default init-method but allows still
>minimal environments with more effective init methods.

True, but there is currently nothing but the default init-method
in Fedora, so it would seem to me that adding packaging for these
non Fedora cases adds confusion and isn't needed.

In any case I would be willing to approve the package as it was
in comment #10, since it meets all the guidelines. I would prefer
that version over this one with the additional tor-core subpackage,
unless there is some good reason for the package inflation.

If you prefer to get approval for the 0.1.0.16-0.1 version
instead, perhaps I should move this back to FE-NEW and you can
pick up a diffrent reviewer.
Comment 23 Paul Wouters 2006-02-06 12:30:10 EST
in btoh cases there are (i think unwanted) sub-packages. but in the later one,
at least one can "yum install tor" to prevent some confusion, where as the one
in #comment 10 you had to "yum install tor-lsb". So the latter one is better
then the previous one, though I'm still in favour of one package until a new
supported init system makes it into Fedora Core/Extras
Comment 24 David Nielsen 2006-02-24 07:21:01 EST
I'll agree with #23, untill we actually support multiple init systems there's no
special reason to branch it out in seperate packages.
Comment 25 Roozbeh Pournader 2006-05-23 16:42:10 EDT
After installating the RPM resulting from comment #21, "torify" will fail with
the following error message:

/usr/bin/torify: line 7: exec: tsocks: not found

You probably need a Requires: on tsocks.
Comment 26 Enrico Scholz 2006-05-23 19:30:57 EDT
* Wed May 24 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.1.20-0
- updated to 0.1.1.20; adjusted %doc file-list
- added (optional) -tsocks subpackage
- use the more modern %bcond_with* for specifying optional features

-------

'tsocks' requirement will be brought in by the -tsocks subpackage. Because
it is not available in FE, it is disabled by default.

fwiw; initng is making big steps to become part of FE so we will have
an alternative init system soon. When there are still objections
against the -lsb, -initng, ... subpackages, I will hold this package
until 'initng' is really available for FE and we have something to
discuss about.
Comment 27 Enrico Scholz 2006-06-28 02:11:35 EDT
[lost in the last bugzilla crash]

* Tue Jun 13 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.1.21-0
- updated to 0.1.1.21

http://ensc.de/fedora/tor/
http://ensc.de/fedora/tor/tor.spec
http://ensc.de/fedora/tor/tor-0.1.1.21-0.src.rpm
Comment 28 Enrico Scholz 2006-07-08 05:21:54 EDT
* Sat Jul 08 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.1.22-0
- updated to 0.1.1.22

http://ensc.de/fedora/tor/
http://ensc.de/fedora/tor/tor.spec
http://ensc.de/fedora/tor/tor-0.1.1.22-0.src.rpm
Comment 29 Enrico Scholz 2006-07-08 05:38:22 EDT
regarding comment #22 (sorry, I forget it)
------------------------------------------

> >- renamed the current main-package into a '-core' subpackage and
> >  created a new main-package which requires both the 'tor-core'
> >  subpackage and this with the current default init-method. This
> >  allows 'yum install tor' to work better; because yum is not very
> >  smart, the old packaging might install unwanted packages else.
> 
> Humm.. Can you elobrate on what situation would result in unwanted
> packages?

We had the following situation with the old packaging:

| Name:     tor
| Requires: init(tor)
| 
| %package lsb
| Provides: init(tor) = lsb
| Requires: lsb
| 
| %package initng
| Provides: init(tor) = initng
| Requires: initng

Assuming you have a minimal system with the 'initng' initsystem but
without 'tor' packages. Now


When you install 'tor' now, two possibilities exist for the resulting
package combination :

* 'tor' + 'tor-initng'; this is the probably wanted result and will
  not bring unwanted packages in

* 'tor' + 'tor-lsb'; this will install 'tor-lsb' (which will not work
  with the initng system) with its huge dependency chain. This option
  is probably unwanted.


yum has a lousy depsolver and will use the second, unwanted option due
to the shortest-packagename-wins rule.


Therefore, I moved 'tor' into the -core subpackage and made 'tor' a
metapackage requiring 'tor-core' plus the subpackage for the current
initsystem. This might be tor-lsb for FC4-FC6 and tor-initng for FC7.

This eases package installation for people with standard installation
doing

| # yum install tor

and those with initng who can do

| # yum install tor-core tor-initng
Comment 30 Kevin Fenzi 2006-07-25 15:42:51 EDT
Sorry for sitting on this review for so long... I am going to set this back to 
FE_NEW and see if someone else would like to move it forward. (I thought I did 
this a while back, but it fell through the cracks. Sorry). 
Comment 31 Enrico Scholz 2006-08-13 12:23:11 EDT
* Sun Aug 13 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> -
0.1.1.23-0
- updated to 0.1.1.23

http://ensc.de/fedora/tor/
Comment 32 Jason Tibbitts 2006-08-30 02:08:03 EDT
I took a look at this package, but I find myself questioning the point behind
the lsb, minit and initng subpackages.  We have a defined system in Fedora for
initscripts and such; I guess I don't see the point in splitting what's
certainly going to be required out to a subpackage.  Is there any existing
daemon that has its initscripts in a subpackage?
Comment 33 Enrico Scholz 2006-09-10 14:44:18 EDT
> I guess I don't see the point in splitting what's certainly going to
> be required out to a subpackage.

ok; package was written when it seemed that 'initng' could replace
'initscripts' in near future. But current development and codebaes
shows that they are still in the experimenting phase (e.g. they try
crazy things like garbage collector in init).

But: I do not see a reason why 'distcc' needs lvm2, udev or e2fsprogs
(which would be the case when SysV initscript would be in the main
package). So I will keep core functionality and initscripts in separate
packages.

Things are special for this package because it supports startup with
SysV, inetd and ssh.


> Is there any existing daemon that has its initscripts in a subpackage?

ip-sentinel, dhcp-forwarder, milter-greylist
Comment 34 Enrico Scholz 2006-09-10 14:50:10 EDT
sorry, comments where made with the wrong package in mind... 

So, please remove the third paragraph
Comment 35 Denis Leroy 2006-09-20 06:14:27 EDT
I would like to see tor make it into Fedora Extras, and this being one of the
oldest review in bugzilla I would like to review this and get it over with.

Is there anyway you would agree to simplify your spec file, merge the core and
lsb subpackages into the main one and remove the other subpackages that aren't
directly relevent to Fedora (at this point). I would recommend a simpler
approach for now, to speed up the review, and add support for newer
init-replacement only when they make it into FE.
Comment 36 Enrico Scholz 2006-09-20 07:08:19 EDT
no; the part which requires lvm2, udev or e2fsprogs will be always in another
(sub)package than the tor-server which works perfectly without them.
Comment 37 Denis Leroy 2006-09-20 07:52:09 EDT
Hmm, this seems overkill, especially since lvm2, udev and e2fsprogs are pretty
basic packages. Tor is a pretty small and simple package to start with. All
other distributions ship it as a single package, including the RHEL43 packages
directly provided by upstream.

Also, torify requires tsocks which is not currently available. Were you planning
to submit tsocks as well ? If not, we need to close this bug with a WONTFIX or
DEFERRED.

Comment 38 Denis Leroy 2006-09-20 07:53:51 EDT
Sorry, closing the bug was accidental.
Comment 39 Enrico Scholz 2006-09-20 08:18:31 EDT
I will merge packages when you show me that tor requires lvm2, udev or
e2fsprogs. Else, I see at me own systems that 'tor' works perfectly without them
so I assume they are optional.

The '-tsocks' subpackage won't be built be default; when you really want it, I
can remove any traces of it out of the spec-file.
Comment 40 Paul Wouters 2006-09-20 13:00:11 EDT
It seems this package is dead locked, and no one wants to approve this package
due to the separate sub packages. 

I submitted a package, which had the approval of the tor developers upstream,
but it was submitted a day after this package, so was resolved as duplicate.

This issue should be discussed on the fedora-extras list and resolved one way or
another. The result of the current deadlock is that there is no tor pacakge
whatsoever in Fedora, which is not right.
Comment 41 Denis Leroy 2006-09-20 13:19:21 EDT
One more question: does it make sense to package tor without the torify wrapper
script ? (which requires tsocks, http://tsocks.sourceforge.net/, which we don't
have). Wouldn't it make sense to first submit tsocks in a seperate review, THEN
submit tor when that's available ?
Comment 42 Till Maas 2006-09-20 13:44:25 EDT
I would prefer having tsocks and torify in Extras.
Comment 43 Enrico Scholz 2006-09-20 13:46:19 EDT
I do not think that it is deadlocked; 'initng' is probably going soon
into FE (AMD64 issues were main blocker and packager has AMD64 machine
now), so we have the multi-initsystem soon.

It would be good too, when reviewers do not insist on personal views
and *requesting* their will. Statements like "others are doing it so"
are valid comments, but without reasons they are just comments, and
not a blocker.


And yes; it makes sense to package tor without tsocks support. Even
when tsocks would be available, the corresponding tor-wrapper must be
in an own subpackage due to the additional dependencies.

The 'tsocks' wrapper might be a nice feature but not required so it
can be ommitted for now.
Comment 44 Paul Wouters 2006-09-20 15:10:09 EDT
As for "requesting your will", not a single person but you is insisting on not
having tor included without different init methods. I only see people who either
want tor in and don't care, or people who are negative about the split. If we
would be doing a concensus here, it would be to have a single tor package in FE
now, and in the future when initng IS in FE to do the split.

Furthermore, you expected initng to get here soonbut reality is, I still don't
see it in the upcoming FC6 (as shown in 5.92). Are we going to wait having a tor
package until FC7?

It is you who does not want to depend on package that are considered 'always
present', such as lvm2, thereby adding non fedora-extra issues to this package.

Let's not drag the tsocks argument into this. One can trivially grab
http://dag.wieers.com/packages/tsocks/ and put in in FE. I'll gladly either
propose it or approve it if this tsocks issue is considered a tor blocker.

The sub package issue is the only real issue here. It caused this package to not
be approved on 2005-12-22 and 10 months later is still blocking it. That's why I
think this should be discussed. This package either needs to get approved with
the subpackage structure, or should be declined in favour of another packager. 

It should not remain in political limbo.

Comment 45 Enrico Scholz 2006-09-20 16:36:44 EDT
Reviewers want the one-package structure but do not give a single argument why
this should be done or why multiple packages are bad.

I gave arguments why I chose the multi-package structure and nobody responded to
these arguments so far.


>  initng ... I still don't see it in the upcoming FC6

initng is an Extras candidate and can be added after FC6 release


> ... package that are considered 'always present', such as lvm2 ...

ok; this might be a response to my arguments. But I do not think that this is a
valid one. 'lvm2' is always present due to packaging bugs only (mixed
initscripts and core-functionality; bloated 'initscripts' package).

I can not fix the other packages because I would have to make this discussion at
lot of other packages. RH developers are usually ignorant regarding dependency
issues (e.g. look at aspell -> perl dep, sendmail -> cyrus-sasl, initscripts ->
low-level stuff) so this would be a lost battle at least in Core.

What I can do, is to package my packages properly and to separate
core-functionality and unneeded/big dependencies which is giving users with e.g.
chroots or non-SysV init a chance for a small system.
Comment 46 Denis Leroy 2006-09-21 06:31:56 EDT
> Reviewers want the one-package structure but do not give a single
> argument why this should be done or why multiple packages are bad.

Sure, let's give it a shot.

- We've never had a policy for systematically splitting packages strictly based
on a couple of fairly common packages. The policy is mostly based on
common-sense, i.e. when the subpackage has a real dependency bloat issue such as
bringing in the entire java stack, or gstreamer, or 25 perl packages.

- If we were to use your strict splitting policy on all Fedora packages, the
total number of packages in Fedora would be multiplied by 3 or 4. There's an
inherent cost associated with increasing the number of packages at the yum/rpm
level. Yum is improving all the time but it has enough work to do as it is.

- Simplicity. Keep It Simple. I'm looking at the tor tarball, and it's
dreadfully simple. No complicated dependencies, very small number of installed
files. Not even 2M in size. So the complexity you're introducing in the spec
file doesn't match the complexity of the upstream project.

- Consistency to me is an important issue. Consistency across Fedora for one. To
use more or less similar guidelines for packages split. Consistency across other
distributions for second. 

- Your refusal to collaborate with reviewers is hurting Fedora. You're
essentially blackballing a number of useful packages from entering Fedora, since
 you're holding a temporary monopoly on those particular package reviews.

- Enrico, nobody is doubting your technical expertise, but I just think your
reasoning doesn't fall within the scope of what Fedora is. Fedora is not a
distro targetted at the embedded world, and mock seems to work pretty well is it
now, so I don't understand the quest for the smallest system possible. The SysV
init is the default and only init system available right now, so isolating that
dependency right now doesn't make sense. Especially since we'll end up with a
subpackage containing a single 1.8 Kbytes shell script.

The fact that the entire community doens't support your splitting proposal, and
the fact that no other distros does it should *at least* give you a hint that
something is wrong with your reasoning. You can't be serious if you think you're
right and everyone else is wrong.

So please, either

- fold the package into one, so I can review it and let's get this over with

- close this bug and withdraw your review to give someone else the opportunity
to submit it.

Comment 47 Paul Howarth 2006-09-21 07:11:29 EDT
(In reply to comment #46)
> - Enrico, nobody is doubting your technical expertise, but I just think your
> reasoning doesn't fall within the scope of what Fedora is. Fedora is not a
> distro targetted at the embedded world, and mock seems to work pretty well is it
> now, so I don't understand the quest for the smallest system possible.

Given that Fedora is heavily involved in the OLPC project, I suspect that
bloated dependency chains are likely to become more of an issue and get more
attention in the FC7 timeframe. Enrico is a little "ahead of the game" here but
I can see the approach of splitting packages up to fine-tune dependencies
becoming more common in the near future.
Comment 48 Patrice Dumas 2006-09-21 08:24:57 EDT
(In reply to comment #46)

> - Your refusal to collaborate with reviewers is hurting Fedora. You're
> essentially blackballing a number of useful packages from entering Fedora, since
>  you're holding a temporary monopoly on those particular package reviews.

That's the rule of the game. Maybe it could be changed, but I don't
think this example call for that change. It is not a refusal to 
cooperate, but a disagreement. I haven't looked deeply at this, but 
I tend to think that both approaches are valid (split and unsplit) 
each with pros and cons. The dependencies are better isolated with
Enrico approach while it is simpler and more generic unsplit.

> The fact that the entire community doens't support your splitting proposal, and
> the fact that no other distros does it should *at least* give you a hint that
> something is wrong with your reasoning. You can't be serious if you think you're
> right and everyone else is wrong.

That's a wrong assumption. The number isn't a proof of 
correctness. Especially when the people having looked at the 
issue is only a subset of the community.

And given the rules, Enrico needs only to find one reviewer 
who backs up his view. Anybody disagreeing might then throw the 
issue on the extras list, but currently 2 people may be against 
all the other packagers (not that I consider that to be a healthy 
situation).
Comment 49 Paul Wouters 2006-09-21 11:06:16 EDT
> Reviewers want the one-package structure but do not give a single argument why
> this should be done or why multiple packages are bad.

No, you are refusing to hear them.

1) most, if not all other packages work like that. When people introduce initng,
if it will happen at all in the future, then that's a good time to redo this
package along with the hundreds of other packages

2) you are insisting on custom non-FE requirements

3) your requires are custom and don't take into account the regular FE base
install (that includes lvm2 etc, you call bloat)

4) spec file is overly complex (and a reason people are not approving it)

5) You are blowing up a simple package into many subpackages which is completely
unneccessary on a FC/FE machine (versus your development box where you need it)

I guess the FE new package submission needs a way to DISapprove a package so
these deadlocks do not occur.
Comment 50 Rex Dieter 2006-09-21 11:16:48 EDT
Re: comment 46 , Dennis said:

> So please, either
> - fold the package into one, so I can review it and let's get this over with
> - close this bug and withdraw your review to give someone else the opportunity
> to submit it.

Also consider the possibility that you (both) can agree to disagree, and remove
yourself as reviewer, and let someone else do it.
Comment 51 Enrico Scholz 2006-09-21 14:12:17 EDT
> - We've never had a policy for systematically splitting packages

Exactly, there is no policy which says when to split a package. Until
then, it is packager's choice whether he splits or does not split.

My choice is, to split.


> - If we were to use your strict splitting policy on all Fedora packages,
>   the total number of packages in Fedora would be multiplied by 3 or
>   4. There's an inherent cost associated with increasing the number of
>   packages at the yum/rpm level.

Is this cost measured in KB, seconds, used lines on display or
bananas? Wouldn't they be outweighted by lesser dependencies and a
smaller system?

E.g. monolithic 'tor' might bring in initscripts, lvm2, udev... while
a splitted tor brings only tor-core. Splitting seems to reduce inherent
costs on yum/rpm level for me...

Splitting will perhaps increase needed blocksize (1-4K) in the repository
by one or two. The Used diskspace on the repository is cheap. Much cheaper
than the bloat introduced by unneeded dependencies.


> - Simplicity. Keep It Simple.

Ok, I can remove the initscript stuff completely and provide single
'tor-lsb' and 'tor-initng' packages. Would just add two more reviews
and people would complain that 'tor' main package does not have an
initscript.

As a compromise: I will keep -lsb in main package (as is) and remove
only the -minit and -initng part. Would you accept this?


> - Consistency to me is an important issue.

What would bring you consistency here? Using 'yum install tor' installs
consistently a 'tor' daemon with the appropriate initscripts; both with
the splitted and bloated variant.


> Consistency across other distributions for second. 

Package is for Fedora Extras; I do not request a review for Debian or
Mandriva.


> Not even 2M in size.

Size of package does not matter for dependencies issues. A 20 byte
perl script can bring in 50 MB of perl.


> - Your refusal to collaborate with reviewers is hurting Fedora.

Come on. Your refusal to accept views of packagers is hurting Fedora.


=========


> 1) most, if not all other packages work like that.

In Germany we have a proverb: "millions of flies can not err: shit
tastes great".

When you are new it might be good idea to follow the masses. But at
some time you should turn on the brain and think yourself.


> 2) you are insisting on custom non-FE requirements

Ok, as written above, I will remove the -initng and -minit subpackage
when this helps.




I really do not want to continue this meta-discussion which consists
only of personal views and unproved statements like "entire community".
Comment 52 Enrico Scholz 2006-09-21 14:38:25 EDT
Ok, you won:

* Do Sep 21 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.1.23-0.1
- simplified spec file and removed -initng and -minit stuff

http://ensc.de/fedora/tor/
Comment 53 David Nielsen 2006-09-21 14:45:26 EDT
it might just be me but does the tsocks package require... tsocks?
Comment 54 Enrico Scholz 2006-09-21 15:09:30 EDT
Although tsocks issues were discussed some comments above already, I
will remove torify for now and add it back when tsocks exists.


* Thu Sep 21 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.1.23-2
- simplified things yet more and removed tsocks/torify too
- build -lsb unconditionally

http://ensc.de/fedora/tor/
Comment 55 Toshio Kuratomi 2006-09-25 01:13:04 EDT
MD5Sum

a1c6efad2d042b7b54da114852687df4  tor-0.1.0.15-setgroups.patch
33ce7155f545c4d30cb846d7017cc6c2  tor-0.1.1.23.tar.gz
e1c9fd2bd8fb03c1f35028fbe7d19585  tor-0.1.1.23.tar.gz.asc
56c122286a73ed67308cf2864a246c7a  tor.logrotate
fa520d134658dc6919af24a1218b3676  tor.lsb
c83c1cb67453e47bf710f899b9e58976  tor.spec
8cef32dff6452c22873846adc6041d86  tor-0.1.1.23-2.fc5x.src.rpm

Cosmetic:
* The gpg file is nice in that it alerts me to its presence on the upstream
  download site but unless I have the signing gpg key in my web of trust I'm
  still going to have to run around the internet verifying that the gpg
  signature comes from upstream and that the key that made it probably belongs
  to the developers by which time I've downloaded the file from the internet
  myself.  So the case for including it is only so-so to me.  (Not a blocker,
  though.)

Rpmlint: *.src.rpm:
W: tor strange-permission tor.lsb 0775
  - Ignorable, this is the initscript for SysVinit.

E: tor hardcoded-library-path in /usr/lib/lsb/install_initd
E: tor hardcoded-library-path in /usr/lib/lsb/remove_initd
  - Ignorable, you're just calling chkconfig via the lsb standard names.

W: tor macro-in-%changelog doc
  - Line 221 has a bare %doc instead of %%doc.

W: tor mixed-use-of-spaces-and-tabs
  - Cosmetic.

Rpmlint: tor:
E: tor no-binary
  - Igorable as this is a meta-package.

Romlint: tor-core:
E: tor-core non-standard-gid /etc/tor/torrc toranon
E: tor-core non-standard-gid /var/log/tor toranon
E: tor-core non-standard-uid /var/lib/tor toranon
E: tor-core non-standard-gid /var/lib/tor toranon
  - toranon is fine so these are ignorable.

E: tor-core non-readable /etc/tor/torrc 0640
E: tor-core non-standard-dir-perm /var/log/tor 0730
E: tor-core non-standard-dir-perm /var/lib/tor 0700
  - Should be fine as well.

E: tor-core incoherent-logrotate-file /etc/logrotate.d/tor
  - rpmlint is confused because the package is named tor-core.  This is
    ignorable.

Rpmlint: tor-lsb:
W: tor-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/tor
E: tor-lsb executable-marked-as-config-file /etc/rc.d/init.d/tor
  - As explained earlier, this is normal for init scripts.

W: tor-lsb no-documentation
  - Documentation is in the main package.  This is ignorable.

E: tor-lsb non-standard-uid /var/run/tor toranon
E: tor-lsb non-standard-gid /var/run/tor toranon
  - This is fine.

W: tor-lsb hidden-file-or-dir /etc/tor/.have-lsb
E: tor-lsb zero-length /etc/tor/.have-lsb
W: tor-lsb non-conffile-in-etc /etc/tor/.have-lsb
  - The .have-lsb file seems to be a marker identifying which set of init
    scripts is installed for things like the logrotate script.  So it's state
    of the system rather than configuration.  So not marking it %config makes
    sense.  But putting it in /var might be better than /etc.  Also, is there
    a reason to make it hidden?  If not, perhaps: /var/lib/tor/have-lsb would
    be better.

E: tor-lsb postin-without-chkconfig /etc/rc.d/init.d/tor
E: tor-lsb preun-without-chkconfig /etc/rc.d/init.d/tor
  - You're calling chkconfig by its lsb name, /usr/lib/lsb/install_initd
    so this is ignorable.

W: tor-lsb incoherent-init-script-name tor
  - Once again, rpmlint is confused by the tor-lsb package name so this is
    ignorable.

Good:
* Source and signature matches upstream
* Signature verified created by: #28988BF5: "Roger Dingledine <arma@mit.edu>"
  and is a valid signature for the source.
* Package meets the Naming guidelines
* License, BSD, is OSI approved and matches what is documented in the spec.
* LICENSE is included in %files.
* BuildRequires are listed.
* Package has no locales; language files in documentation are marked with the
  appropriate languages.
* No shared libraries.
* Not relocatable.
* Package owns all the directories it creates.
* No duplicate files listed.
* Permissions properly set.
* Package has a proper %clean section.
* Macros used consistently.
* Package contains code.
* Documentation fits comfortably into the main package.
* Documentation does not affect package at runtime.
* No libraries.
* Not a GUI application.
* Package owns all files and directories that it creates and no extraneous
ones.* Scriptlets are sane.  They use fedora-usermgmt to create and delete a system
  uid/gid.  They install the tor init scripts but don't start the service.
* Builds in mock on x86_64.

Summary:
Fixing the macro in changelog and moving /etc/tor/.have-lsb to
/var/lib/tor/have_lsb are the only things I see to be fixed here.  If you're
okay with those changes I'll approve.

I've gone through all the previous comments as well and I think there's a bit
of tempest in a teapot going on here.  There are some advantages to keeping the
package unified.  However, Enrico seems to have addressed nearly all of those
with his tor meta package so that `yum install tor` does the expected thing on
a default FC install with the added flexibility that installing tor-core with a
different subpackage can make the package work for other init systems.  There
may be a more elegant way to solve the dependency problem but without any new
suggestions, Enrico's approach seems to be the best we can do with what's
available now.
Comment 56 Enrico Scholz 2006-09-25 02:42:31 EDT
thx

* Mon Sep 25 2006 Enrico Scholz <enrico.scholz@informatik.tu-chemnitz.de> - 0.1.1.23-3
- removed '.have-lsb' and related logic in logrotate script; check for
  existence of the corresponding initscript instead of
- fixed bare '%' in changelog section

http://ensc.de/fedora/tor/


========

> * The gpg file is nice in that it alerts me to its presence on the
>   upstream download site but unless I have the signing gpg key in my
>   web of trust...

it's an habit from old fedora.us days. But I think a good one;
being in somebody's web-trust is not so difficultly as your comment
suggests. E.g. signer of tor is verified in my trustdb



> W: tor mixed-use-of-spaces-and-tabs

rpmlint bug; seems to be triggered by

| sed -i -e '...
| <TAB><WS>+'...

which happens due to indentation reasons


> W: tor-lsb hidden-file-or-dir /etc/tor/.have-lsb
> E: tor-lsb zero-length /etc/tor/.have-lsb
> W: tor-lsb non-conffile-in-etc /etc/tor/.have-lsb
>   - The .have-lsb file seems to be a marker identifying which set of init
>     scripts is installed for things like the logrotate script.  So it's state
>     of the system rather than configuration.  So not marking it %config makes
>     sense.  But putting it in /var might be better than /etc.  Also, is there
>     a reason to make it hidden?  If not, perhaps: /var/lib/tor/have-lsb would
>     be better.

I removed the '.have-XXX' stuff completely. But '/var/lib/tor' would
have been a bad place because the have-XXX files are files used directly
by 'root' while '/var/lib/tor' is owned by 'toranon'. Nevertheless,
should not be an issue anymore.
Comment 57 Toshio Kuratomi 2006-09-25 14:10:54 EDT
MD5Sum
2d861e91e45a709acd921f26214319c1  tor-0.1.1.23-3.fc5x.src.rpm

Both issues raised before have been fixed.
 
APPROVED
Comment 58 Enrico Scholz 2006-10-01 07:32:11 EDT
thx
Comment 59 Simon 2009-05-28 16:59:53 EDT
Package Change Request
======================
Package Name: tor
New Branches: EL-5
Owners: ensc cassmodiah
Comment 60 Jason Tibbitts 2009-05-28 19:20:50 EDT
I found an ack from Enrico on IRC; I include it below:

[15:31] <cassmodiah> ensc you recieved my mail?
[15:33] <cassmodiah> ensc about tor
[15:35] <ensc> cassmodiah: no, did not saw such a mail
[15:35] <cassmodiah> :-(
[15:36] <cassmodiah> ensc http://fpaste.org/paste/13205
[15:39] <ensc> cassmodiah: ah... no, I do not plan to do EPEL packaging.  "Working" with CVS SCM takes too much time... the three FEdora branches are enough and I do not want two additional ones.
[15:39] <cassmodiah> :-p
[15:40] <cassmodiah> you need a co? for epel? I'm just a packager in training, but cwickert and rsc always help me out if i have a problem :-)
[15:43] <cassmodiah> ensc thx. can i have the el5?
[15:47] <ensc> cassmodiah: yes; but you will have to do the necessary steps to add it to epel.  I will approve you then (when required)
[15:48] <cassmodiah> ensc i thought i create a cvs request for el5 with me as co :-p
[15:50] <ensc> cassmodiah: sorry; I do not have time for these steps
[15:51] <rsc> cassmodiah: you don't need any approval from the Fedora maintainer for EPEL branching
[15:53] <cassmodiah> rsc i know, but i think i have to bring the respect to the maintainer and ask him/her first
[15:54] <rsc> cassmodiah: that's okay and good. I just wanted to note that.

However, I see no indication that Enrico wishes to have anything to do with this package; the above would seem to indicate the contrary.  So I won't branch with him as the owner without an explicit ack.

Please reset fedora-cvs to '?' if that ack is received or if another CVS request is made without Enrico as an owner.
Comment 61 Jason Tibbitts 2009-05-28 19:44:41 EDT
Sorry, please correct my above comment as "I see no indication that Enrico wishes to have anything to do with an EPEL branch of this package".  Obviously he still maintains the package in the regular Fedora branches.
Comment 62 Simon 2009-05-31 10:37:39 EDT
okay, then me only

Package Change Request
======================
Package Name: tor
New Branches: EL-5
Owners: cassmodiah
Comment 63 Jason Tibbitts 2009-05-31 16:17:51 EDT
CVS done.

Note You need to log in before you can comment on or make changes to this bug.