Hi again,
I was looking into applying this (sorry to take so long to do so).
More questions.
Daniel Baumann wrote:
> On 10/20/2010 10:35 PM, Jonathan Nieder wrote:
>> install git
>> install git-daemon-sysv
>> remove git-daemon-sysv
>>
>> Then git will be installed but git-daemon-sysv will be in the conffiles
>> state. Although the daemon is present it should not be run imho.
>
> fixed in updated patch.
The fix was to make /etc/init.d/git-daemon a non-conffile ---
customizations go in /etc/default/git-daemon. Okay. This means
that either:
- the init script should prominently document that inline
customizations will be overwritten on upgrade
or
- we could use another sentinel file instead of /usr/bin/git-daemon
to signal that git-daemon-sysv is installed --- for example,
/usr/share/git-daemon-sysv/installed.
What do you think? Does one of those sound like a good idea?
> --- a/debian/control
> +++ b/debian/control
> @@ -165,6 +166,25 @@ Description: fast, scalable, distributed revision
> control system (git-daemon ser
> repositories through the network. This package provides a runit service
> for running git-daemon permanently.
>
> +Package: git-daemon-sysv
git-daemon-sysvinit might be clearer.
> +++ b/debian/git-daemon-sysv.README.Debian
> @@ -0,0 +1,31 @@
[...]
> +This makes 'git-clone git://git.example.org/git/foo' to clone the foo
> +repository on remote machines work.
"git clone" (the dashed forms don't work in the default setup
nowadays).
> --- /dev/null
> +++ b/debian/git-daemon-sysv.postinst
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +set -e
> +
> +test "$1" = 'configure' || exit 0
> +
> +getent passwd gitdaemon >/dev/null || \
> + adduser --system --home /nonexistent --no-create-home gitdaemon
The backslash at the end of the line is not needed.
Shouldn't the gitdaemon user be removed on package removal?
> +
> +if [ -x "/etc/init.d/git-daemon" ]; then
> + # enable git-daemon service
> + update-rc.d git-daemon defaults >/dev/null
> +
> + # restart git-daemon service if it was running
> + if [ -x "`which invoke-rc.d 2>/dev/null`" ]; then
> + invoke-rc.d git-daemon start || exit $?
Rather than "test -x $(which ...)", it is easier to write
if which invoke-rc.d >/dev/null 2>&1; then
which has the nice side-benefit of being the form used in the policy
manual.
The "|| exit" is not needed here, since "set -e" is in use.
To restart the git-daemon service, shouldn't this be
invoke-rc.d git-daemon restart
?
> --- /dev/null
> +++ b/debian/git-daemon/sysv
[...]
> +DAEMON_ARGS="--user=$GIT_DAEMON_USER --reuseaddr --syslog --verbose
> $GIT_DAEMON_OPTIONS --base-path=$GIT_DAEMON_DIRECTORY $GIT_DAEMON_DIRECTORY"
Using --reuseaddr means the SO_REUSEADDR flags is set. For the
benefit of other readers, ip(7) explains:
A TCP local socket address that has been bound is unavailable
for some time after closing, unless the SO_REUSEADDR flag
has been set. Care should be taken when using this flag as it
makes TCP less reliable.
More precisely, if I understand correctly then this defeats the
TIME-WAIT facility for a short window when the daemon is restarted.
The git daemon does not implement any authentication on its own so
this should be harmless from a security point of view, but it is
unfortunate.
I think this is the better of two evils. Just mentioning it for
posterity.
> + start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON
> --test > /dev/null \
> + || return 1
> + start-stop-daemon --start --quiet --pidfile $PIDFILE --exec $DAEMON
> --background --make-pidfile -- \
> + $DAEMON_ARGS \
> + || return 2
git-daemon is capable of writing its own pidfile (see the --pid-file
option). Why not use that facility?
> + start-stop-daemon --stop --quiet --retry=TERM/30/KILL/5 --pidfile
> $PIDFILE --name $NAME
> + RETVAL="$?"
> + [ "$RETVAL" = 2 ] && return 2
> + # Wait for children to finish too if this is a daemon that forks
> + # and if the daemon is only ever run from this initscript.
> + # If the above conditions are not satisfied then add some other code
> + # that waits for the process to drop all resources that could be
> + # needed by services started subsequently. A last resort is to
> + # sleep for some time.
> + start-stop-daemon --stop --quiet --oknodo --retry=0/30/KILL/5 --exec
> $DAEMON
git daemon's children are not named git-daemon so this won't kill
them. Is it important to terminate active connections (git-daemon
deliberately does not do so when it is killed)?
The rest is in good shape. Thanks for your work.
Jonathan
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
