On Wed, Jul 14, 2010 at 09:55:41PM +0200, Lorenzo De Liso wrote: > In ubuntu we applied the following changes:
Hi Lorenzo,
I'm going to apply some of the patches in the Debian package, but let me
commment on some:
> - scripts/check_rootdir: Check for inode 2 on ext4 as well as ext2/3.
> - debian/postrm: Clean directories correctly when purging.
These two are OK.
> - config:
> + Auto-create the work directory if it's in /var/run, /tmp,
> or /var/tmp
This patch I will not accept (see below)
> + Use temporary file instead of hardcoded rcfile for temporary work
This patch I will not accept (see below)
> --- tiger-3.2.3.orig/config
> +++ tiger-3.2.3/config
> @@ -287,10 +287,15 @@
> # TODO: WORKDIR should be removed on exit if it is located in a temporary
> # directory
> if [ ! -d "$WORKDIR" ] ; then
> - echo "Configured working directory $WORKDIR does not exist" >&2
> - if [ "$QUIET" != "Y" ] ; then
> - echo "Creating working directory $WORKDIR"
> - echo
> + case "$WORKDIR" in
> + /var/run/* | /tmp* | /var/tmp/* )
> + mkdir -p "$WORKDIR"
> + ;;
> + *)
> + echo "Configured working directory $WORKDIR does not exist" >&2
> + exit 1
^^^^^^^
Why exit? The goal of this message is to warn the user that
the directory is going to be created.
> + esac
> +
> fi
> # TODO: -p switch is not portable
> if ! mkdir -p "$WORKDIR" >/dev/null 2>&1; then
This change does not make sense to me as this 'case' statement calls mkdir
when, actually, mkdir is already called (within the 'if' statement at the
end). I understand that you might not want Tiger to be complaining in stderr
everytime the working directory is created when it points to /var/run/* |
/tmp* | /var/tmp/* so I'm adjusting the patch to not complain in this
situation. But the directory creating is done afterwards.
> @@ -395,11 +400,11 @@
> #
>
> [ -n "$RCFILE" -a -r $RCFILE ] && {
> - tempfile=`tempfile -d $WORKDIR` || tempfile=$WORKDIR/rcfile.$$
> + tmpf=$(tempfile)
> $GREP -v '^#' $RCFILE |
> - $SED -e 's/^\(.*\)=/export \1; \1=/' > $tempfile
> - . $tempfile
> - $RM -f $tempfile
> + $SED -e 's/^\(.*\)=/export \1; \1=/' > $tmpf
> + . $tmpf
> + $RM -f $tmpf
> }
This patch I cannot agree with:
- the result of tempfile is not checked with your patch. The tempfile command
does not actually exist across all UNIX systems Tiger runs in.
With the above, Tiger tries to use tempfile and, if it fails, it goes
to a sane default by using a file in WORKDIR. As WORKDIR should be
restricted only to the admin user, the fact that this is not a "perfect"
temporary file is of no concern to us.
- there is no reason to change the variable from tempfile to tmpf (it just
obscures the patch)
Regards
Javier
signature.asc
Description: Digital signature
