[Bug 175433] Review Request: tor - Anonymizing overlay network for TCP (The onion router)

Mon Sep 25 05:13:17 UTC 2006

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

Summary: Review Request: tor - Anonymizing overlay network for TCP (The onion router)

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

------- Additional Comments From toshio at tiki-lounge.com  2006-09-25 01:13 EST -------
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 at 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.

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