On 12-04-18 10:05, Simon McVittie wrote:
On Wed, 11 Apr 2018 at 22:14:21 +0200, Olliver Schinagl wrote:
Tags: newcomer
The newcomer tag is for use by the package's maintainer. It indicates a
bug that is particularly suitable for new contributors to work on as an
introduction to contributing to this package (I don't think this is one
of those). It does not indicate bugs that were encountered by new users,
or bugs that particularly affect new users.
Sorry about that, I just wanted to get the mailbot to accept the issue ;)


While running debootstrap on a non-native debian system, debootstrap keeps
failing with

debootstrap: line 55: apt-config: command not found

The change causing the issue seems to be 
https://anonscm.debian.org/cgit/d-i/debootstrap.git/commit/?id=98858a907a9f69e
which always seems to pass the if check (even though it's not installed) and
then fails on the eval.
Recent versions (I'm looking at the git repository here) seem to have
introduced seversal misunderstood conditionals. Were these tested or
reviewed?

The important thing to remember in shell conditions is that the square
brackets in "if [ ... ]; then" are not special shell syntax: the
opening square bracket is another name for test(1), and the closing
square bracket is required and ignored when test(1) is invoked as
[(1). The square brackets should be used if and only if you want to
check a condition that is most easily checked via test(1).

The line mentioned in this bug is:

     if [ "apt-config -v > /dev/null" ]; then

which is equivalent to "if true; then" because "apt-config -v > /dev/null"
is a non-empty string. I think this was meant to be either

     if apt-config -v > /dev/null 2>&1; then

which enters the "then" block whenever apt-config -v exits 0 (success),
or

     if [ -n "$(apt-config -v 2>/dev/null)" ]; then

which enters the "then" block whenever apt-config -v produces output.
Either way, stderr should probably be redirected to /dev/null (as shown)
to silence "apt-config: not found" messages on non-Debian systems.

This is not the only problematic conditional. There's also:

     +if [ ! "$VARIANT" = fakechroot ] && [ "$(apt-config -v > /dev/null 2>&1)" 
]; then

The second part of this condition will always evaluate to false, because
apt-config -v > /dev/null 2>&1 never produces output (it has all been
redirected to /dev/null), so the $() operator produces no output, and
the test is [ "" ] which returns false. I think this was meant to be:

     if [ ! "$VARIANT" = fakechroot ] && apt-config -v > /dev/null 2>&1; then

in which the second part becomes a check for "did apt-config -v exit with
status 0?", or possibly

     if [ ! "$VARIANT" = fakechroot ] && [ -n "$(apt-config -v 2>/dev/null)" ]; 
then

in which the second part checks whether apt-config produced output.

Similarly:

     +if [ "$(ls -A "$TARGET" > /dev/null 2>&1)" ]; then

doesn't do anything useful, because the output of ls is discarded. Either
the intention was to check whether the output of ls was non-empty (in
which case remove the "> /dev/null" redirection and use 2>/dev/null),
or the intention was to check the exit status (in which case remove []
and "$()").

When testing whether a string is (non-)empty, I would recommend using the
[ -n "$foo" ] or [ -z "$foo" ] form instead of just [ "$foo" ] or
! [ "$foo" ] - it makes it a bit clearer what the intention was.

     smcv

Reply via email to