On Fri, Apr 03, 2015 at 05:08:57PM +0700, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
> index 6269117..c61afbf 100755
> --- a/t/t1509/prepare-chroot.sh
> +++ b/t/t1509/prepare-chroot.sh
> @@ -14,25 +14,42 @@ xmkdir() {
>  
>  R="$1"
>  
> +[ "$UID" -eq 0 ] && die "This script should not be run as root, what if it 
> does rm -rf /?"

This line complains for me. $UID is set in my bash login shell, but not
in the dash shell for this script. Maybe "id -u" instead?

>  xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
> -[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 
> 3 && chmod 666 $R/dev/null"
> +touch "$R/dev/null"

It is nice not to mknod here, as it requires root. But making /dev/null
a regular file seems a bit flaky. We will constantly overwrite it with
the output of each test, and then pipe that output back into the next
test. The current set of tests in t1509 does not seem to mind, though.

> +# Fake perl to reduce dependency, t1509 does not use perl, but some
> +# env might slip through, see test-lib.sh, unset.*PERL_PATH
> +sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > 
> "$R$(pwd)/GIT-BUILD-OPTIONS"

Re-preparing an already-made chroot makes this:

  PERL_PATH=/bin/true'/usr/bin/perl'

Did you want "PERL_PATH=.*" as your regex?

> -echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
> +cat <<EOF
> +Execute this in root:
> +chroot $R /bin/su - $(id -nu)
> +IKNOWWHATIAMDOING=YES ./t1509-root-worktree.sh -v -i
> +EOF

I found the "in root" here (and in the original) confusing. Do you mean
"as root"? I wonder if it would make sense to just show:

  sudo chroot $R /bin/su - $(id -nu)

as the sample command.

Aside from the nits above, I did get it to run t1509 with this. I can't
say I'm incredibly excited about the whole thing, if only because it is
clear that nobody is going to run it regularly (so regressions will
likely go unnoticed, which is the whole point of the test script).  But
perhaps it is better than nothing, and it is not hurting anyone to sit
there and bitrot again. ;)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to