Bug#683331: bug in FHS compliance?
Dear bertagaz: What is this problem in Tahoe-LAFS FHS compliance? Thanks! Regards, Zooko
Bug#652003: Fwd: Ticket #961
Dear bertagaz: Thanks again for working on this! Here are a few comments on this version of tahoe-lafs.init ¹ ¹ http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/tahoe-lafs.init;h=352ee7581a9a1f52ada75e791f542fda4f68ea59 . • Re: line 50 ² this reminds me of Tahoe-LAFS ticket #725 (“We should whine if we're running as root.”) ³. I don't suggest any change to the tahoe-lafs.init script, but it makes me wonder if #725 should be hardened to exit with an error if we're running as root. ² http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/tahoe-lafs.init;h=352ee7581a9a1f52ada75e791f542fda4f68ea59#l50 ³ https://tahoe-lafs.org/trac/tahoe-lafs/ticket/725# We should whine if we're running as root. • The issue that I raised in my previous review ⁴ about how to handle one or multiple failures during a multi-node operation was fixed in a way that I like. The current version of tahoe-lafs.init ¹ tries to perform its requested operation (whether it is start, restart, or stop) on each of the requested nodes, and if the exit value from the attempt is non-zero then it sets STATUS to that exit value. This means that the final exit value of tahoe-lafs.init is 0 only if all of the attempts succeeded, else it is the exit value of the most recent attempt. This also means that if tahoe-lafs.init attempts to stop multiple nodes, and one or more of those nodes was already not-running, then the exit code from tahoe-lafs.init will be non-zero, since attempting to stop a not-running node results in a non-zero exit code. All of this sounds fine to me! The only change I would suggest is to put a comment at the top of the file stating this. ☺ Also, would it be appropriate to have the usage printout include a statement of this behavior? ⁴ http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=652003 • Other than that suggestion to add documentation (in the form of a comment and/or usage printout), I see no problem in this script. It is much shorter than earlier versions, thanks to the refactoring of start, restart, and stop into the same code and the delegation of more functionality to the underlying tahoe script, so it was much faster to read through it, which hopefully means it will get better review/auditing/debugging in the future. Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#652003: this issue might be blocking Tails
This issue might be blocking a project to include Tahoe-LAFS as a tool in the Tails system: https://labs.riseup.net/code/issues/6227 It isn't clear to me if this is really a blocker — maybe the Tails people can use Tahoe-LAFS even without the improved init scripts, or even without any init scripts at all! But I'm motivated to push this issue (debian #652003) forward in part because I want to push Tails #6227 forward. Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#652003: Fwd: Ticket #961
Okay, here I'm reviewing
http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/tahoe-lafs.init;h=dda97b498f0f47b3d284fd4228e8e0a1cd2b;hb=refs/heads/feature/sysvinit
.
Thank you for working on this patch! You seem to have addressed all of
the issues that I raised in my previous review.
* It is normally capitalized Tahoe-LAFS instead of Tahoe-lafs, but
if that is a problem for some reason then I don't care.
* The tahoe command and the start-stop-daemon are both going
simultaneously write the PID into the file named twistd.pid, if I
understand correctly. That seems like it might cause trouble, and also
seems unnecessary. Suggest removing the configuration which tells
start-stop-daemon to do that part (--pidfile).
* In fact, what is start-stop-daemon needed for, if we remove the
--pidfile? Maybe replace::
47 start-stop-daemon --start --quiet --oknodo \
48 --pidfile $CONFIG_DIR/${node_name}/twistd.pid \
49 --exec $DAEMON --chuid $node_uid -- \
50 start $DAEMONARGS
$CONFIG_DIR/${node_name} /dev/null || STATUS=1
with::
47 su -s /bin/sh -c $DAEMON start
$CONFIG_DIR/${node_name} $node_uid /dev/null
48 STATUS=$?
This would also mean that STATUS gets updated to reflect the exit
code from tahoe, instead of being set to 1 iff start-stop-daemon
exited with non-zero. (More about STATUS, below.)
* On Line 52, when it refuses to start a node owned by root, should
probably set STATUS (to 1). (More on STATUS below.)
* Line 71 checks if $AUTOSTART is the zero-length string, and if so
exits, but then line 75 checks if $AUTOSTART is the length-zero
string, which it never will be. I suggest to change line 75 from::
if test -z $AUTOSTART -o $AUTOSTART = all ; then
to::
if test $AUTOSTART = all ; then
* AUtOSTART → AUTOSTART
* Lines 83 and 95 test whether a directory exists before running
tahoe start on it, and if it doesn't exist it, they print out an
error message ( No such node configured: $name). However, if this
code were removed from the init script, and instead the init script
tried to run tahoe start on the directory without checking, then
tahoe would print out a similar error message, like this::
$ tahoe start blahblahblah
STARTING '/home/zooko/blahblahblah'
'/home/zooko/blahblahblah' does not look like a directory at all
I would recommend omitting the test within the init script for the
following reasons:
1. The tests performed by tahoe can be more specific and informative
than the one performed by the init script, for example, if the
directory exists but is empty::
$ mkdir blahblahblah
$ tahoe start blahblahblah
STARTING '/home/zooko/blahblahblah'
'/home/zooko/blahblahblah' does not look like a node directory (no
.tac file)
2. If all such error messages come from the same source, they will
be easier for users to understand.
3. The more code there is, the more places a bug-hunter has to
search when looking for a bug (even if the bug turns out not to be in
that code), so if we can optimize-out code we should! This is
especially true of code that is potentially redundant. For example, if
a bug-hunter or security auditor is investigating something involving
erroring-out when a directory doesn't exist, and they see the code for
that in tahoe but not in the init script, or vice versa, then they
might mistakenly think that they finished examining that
functionality, and not realize that there is a semi-redundant
implementation of that functionality elsewhere, that could be
interacting with the bug.
* This same reasoning applies to the detection of whether the
twistd.pid file is missing, on line 117. If that test is omitted, then
instead of the init script saying No such node running: $name, tahoe
will say::
$ tahoe stop gateway/
STOPPING '/home/zooko/blahblahblah'
No such process
* What should the init script do with STATUS when there are multiple
directories and some of them fail but others don't? Currently it
overwrites STATUS with the result of each directory, so at the end the
STATUS variable will reflect the result of the final directory. I
think it would be better for STATUS to be 0 only when *all* of the
directories that it tried succeeded. If there are any errors, then
STATUS should be non-zero at the end.
* Shouldn't handling of AUTOSTART during /etc/init.d/tahoe-lafs
start be done likewise during /etc/init.d/tahoe-lafs stop?
Currently if AUTOSTART is none or the zero-length string then
running /etc/init.d/tahoe-lafs start will result in an error message
saying 'Autostart disabled' (line 71), but running
/etc/init.d/tahoe-lafs stop will result in stopping all of the
running nodes associated with the directories. I suggest to copy the
code from line 71 into the stop) and restart) blocks so that the
behavior will be more consistent, i.e. as determined by the arguments
on the command-line,
Bug#652003: Fwd: Ticket #961
-- Forwarded message -- From: Zooko O'Whielacronx zoo...@gmail.com Date: Mon, Sep 9, 2013 at 10:41 PM Subject: Re: Ticket #961 To: berta...@ptitcanardnoir.org Cc: Jacob Appelbaum ja...@appelbaum.net, Daira Hopwood da...@leastauthority.com Okay, now that I've reviewed http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/README.Debian;h=14ea2a7c1b38d41df36dd052c44cdf22603fd775;hb=87f7666c2c3a5059dc28ea95c336b9de7f08ae47 and http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/tahoe-lafs.init;h=13b505fb4b2d4be959e3df7edef02a369a48fc7c;hb=8428876521454b5fd2b0719048caf909c0ab68ee are there any more patches I should review? Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#652003: Fwd: Ticket #961
-- Forwarded message -- From: Zooko O'Whielacronx zoo...@gmail.com Date: Mon, Sep 9, 2013 at 3:27 PM Subject: Re: Ticket #961 To: berta...@ptitcanardnoir.org Cc: Jacob Appelbaum ja...@appelbaum.net, Daira Hopwood da...@leastauthority.com Dear bertagaz: Thank you for working on this! I just reviewed http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/README.Debian;h=14ea2a7c1b38d41df36dd052c44cdf22603fd775;hb=87f7666c2c3a5059dc28ea95c336b9de7f08ae47 . It is interesting that this is using the username within the local operating system as the nick. That isn't what we intended the nickname to be for, but I don't see anything obviously wrong with it. It means that the node will announce to everyone who connects to it what the local username is. Is that what was expected? Hm, actually now that I think this through, I think the human reading that doc needs a warning about this! I think you should add a sentence that says This nickname will be announced to all participants in the grid. or something like that. Or, change the suggested command-lines so that it no longer fills in the nick field from the username. Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#652003: Fwd: Ticket #961
-- Forwarded message -- From: Zooko O'Whielacronx zoo...@gmail.com Date: Mon, Sep 9, 2013 at 10:33 PM Subject: Re: Ticket #961 To: berta...@ptitcanardnoir.org Cc: Jacob Appelbaum ja...@appelbaum.net, Daira Hopwood da...@leastauthority.com Okay, I just reviewed http://anonscm.debian.org/gitweb/?p=tahoe/tahoe.git;a=blob;f=debian/tahoe-lafs.init;h=13b505fb4b2d4be959e3df7edef02a369a48fc7c;hb=8428876521454b5fd2b0719048caf909c0ab68ee . Here are my comments. Thank you for working on this patch! I'm excited about making Tahoe-LAFS be a more first-class citizen of the Debian universe. A lot of good can come of this. Thank you for your contribution. I don't understand why /etc/init.d/tahoe-lafs restart does tahoe stop ; sleep 1 ; tahoe start and /etc/init.d/tahoe-lafs force-reload does restart. I think both of those should do tahoe restart. I think /etc/init.d/tahoe-lafs stop should do tahoe stop instead of kill `cat twistd.pid`. That's because tahoe stop has a couple of features such as warning the user if the daemon doesn't stop after SIGKILL. See the code here: ¹. ¹ https://tahoe-lafs.org/trac/tahoe-lafs/browser/trunk/src/allmydata/scripts/startstop_node.py?annotate=blamerev=3ee950f09ed8b7f6cc72a98c26eefe9e02c11d85#L78 Other than those two things, I didn't see anything strange or objectionable in this patch. Thanks again! Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#631163: tahoe-lafs: reduce twisted dependencies
On Sun, Jul 3, 2011 at 5:17 AM, berta...@ptitcanardnoir.org wrote: As far as upstream Tahoe-LAFS goes, we decided to have an unconditional dependency on the SFTP-related packages: PyCrypto and pyasn1, and would also have included an unconditional dependency on Conch if it were not already bundled into the Twisted package. This is because we don't want our users to have to manually install dependencies. So I guess that the python-twisted-conch package dependency should be raised from Recommends: to Depends: to follow upstream's choice. It's little overhead for an interesting feature. I wasn't sure it was that important to make it only a recommends anyway. Yes, please. In particular the upstream Tahoe-LAFS docs assume that any user of Tahoe-LAFS also has Conch and PyCrypto installed. Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#631163: tahoe-lafs: reduce twisted dependencies
Hi there Julian and Bert: Thank you for packaging Tahoe-LAFS! This note is mostly just for your information--it doesn't necessarily mean that the current proposed patch needs to be changed. This situation is complicated by the fact that the packaging of Twisted upstream has only a single package (Twisted) but the packaging in Debian (and some other distributions) has several packages (Twisted-Core, Twisted-Web, etc.). (As an historical note, this started when the upstream Twisted project broke itself up into multiple packages and Debian packaged them separately. Then the upstream Twisted project abandoned that idea and went back to a single monolithic package, but Debian did not follow suit.) Now there are three layers of dependency management that we have to keep in mind in order to reduce twisted dependencies successfully. First is, of course, the Debian layer -- the Depends: line in debian/control. That one currently says that it depends on python-twisted. This could (if the other two layers are also changed) be reduced to depending on python-twisted-core, python-twisted-web, and optionally python-twisted-conch. The optional character of the dependency on python-twisted-conch is that it is needed only if the user is going to run an SFTP server. (Note that SFTP server plus sshfs is the way to have FUSE integration, which many users want.) There are two other dependencies that are present only for the SFTP server: PyCrypto and pyasn1. Second layer of dependency management is the Python packaging system. Tahoe-LAFS has a file of Python packaging metadata named allmydata_tahoe.egg-info/requires.txt which contains a list of other Python dependencies. This file is built automatically at build-time, based on the contents of http://tahoe-lafs.org/trac/tahoe-lafs/browser/trunk/src/allmydata/_auto_deps.py?annotate=blamerev=4976 . This sort of .egg-info file is the standard Python way to express dependencies in a machine-parseable format. There may be other tools which parse that file for their own purposes, and certainly Tahoe-LAFS itself does for the third layer of dependency management: Third layer of dependency management is Tahoe-LAFS itself. At startup time it asserts that the dependencies from the Python packaging metadata are satisfied (using the pkg_resources tool) as well as imports each Python module that it depends on and checks the __version__ attribute of each module: http://tahoe-lafs.org/trac/tahoe-lafs/browser/trunk/src/allmydata/__init__.py?annotate=blamerev=5042#L346 . As far as upstream Tahoe-LAFS goes we cannot depend on the specific Twisted sub-packages like Twisted-Web because they are not distributed individually by upstream Twisted and therefore in many installations there is no Twisted-Web.egg-info file for the pkg_resources to use to determine that the dependency on Twisted-Web is satisfied. As far as upstream Tahoe-LAFS goes, we decided to have an unconditional dependency on the SFTP-related packages: PyCrypto and pyasn1, and would also have included an unconditional dependency on Conch if it were not already bundled into the Twisted package. This is because we don't want our users to have to manually install dependencies. Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#532564: report on using stdeb to generate dsc for argparse
I'm working on a package which depends on argparse (zfec: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=503976 ), so to test out how it behaves when there is an argparse Debian package present I used stdeb which produced this dsc directory: http://zooko.com/argparse-sdist_dsc.tar.bz2 When I run dpkg-buildpackage it produces a python-argparse .deb: http://zooko.com/python-argparse_0.9.1-1_all.deb . This .deb works for me. Regards, Zooko -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
