Bug#721845: ITP: flashproxy -- ephemeral browser-based pluggable transport for Tor
Hi,
here's a quick initial review of the 1.5-1 packaging. Note that I've
not looked at the upstream code, assuming some people better skilled
than me in this area, in the Tor Project, have done this already.
If this is not the case, please tell me.
First of all, if this is your first serious packaging task, this is
a high-quality result, and I'm impressed.
1. debian/copyright gives copyright 2011-2013 to David Fifield, while
the upstream source only mentions 2012. The correct place to fix
upstream copyright information, if that was the intent, is upstream
sources: we're merely relaying it in debian/copyright.
2. Please upgrade to the latest available standards-version.
3. Please make the package Lintian -clean, if possible in pedantic
mode, e.g. like this:
lintian --info --display-info --pedantic --color auto *.changes
Lintian has good advices for you, e.g. using DEP-3 for the
Debian-specific patch to indicate it should not be forwarded
upstream :)
4. Is the flashproxy-facilitator's dependency on python really needed?
I have nearly no experience in Python software packaging, but isn't
the role of ${python:Depends} to do just that?
5. We don't ship software in Debian that has dependencies outside
Debian to be useful at all. So, in the current state of things, the
node-flashproxy binary package (and its README.Debian) is a no-no.
Time to ping Mike Gabriel on #721558 (and offer your help if you
wish), perhaps.
6. A the facilitator's postinst depends on /usr/share/doc to be
available. This is a violation of Policy 12.3: "Packages must not
require the existence of any files in /usr/share/doc/ in order to
function." These files must be moved to /usr/share/$PACKAGE and may
be symlinked from /usr/share/doc/$PACKAGE.
7. In the facilator's postinst, instead of install + cat, you could
just use install to copy reg-email.pass where it belongs.
8. Why install symlinks in the facilitator's postinst, instead of ship
them in the package (e.g. with dh_link(1))? This would avoid the
need to remove them in postrm (and having to handle the case when
the administrator changed these files).
9. This comment leaves me thinking:
# NOTE: debian/rules build (which runs tests) must be run outside of fakeroot
# since the tests open sockets, which fakeroot stubs out.
It could be worth supporting fakeroot environments, no?
Can't you detect fakeroot and (loudly) skip tests if applicable?
10. What does this mean, exactly?
# TODO(infinity0): handle setup.py better
Any potential issue you foresee?
11. I find it a bit scary not to install the upstream LICENSE file,
and to rely on Debian's copyright instead. As soon at the Debian
packaging misses an upstream update (and they are *already* in
mismatch, actually), then we're violating the upstream license.
I recommend installing the upstream LICENSE file.
12. I have not tested the resulting binary packages with piuparts.
I suggest you do it if you can afford it: discovering issues
before the QA team knocks at the door is nicer both for them, and
for you :)
Great work!
Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
Reply to: