[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#517077: FTBFS when SHELL isn't set; also "SHELL ?= =/bin/sh -e" in debian/rules is useless and wrong



Package: glibc
Version: 2.9-3
Severity: important
Tags: patch

        Hi,

 (Sorry if I'm reporting two issues in this single bug, the fix is the
 same for both and it's best to know about both when fixing either.)

 glibc fails to build in an environment where SHELL isn't set -- which
 is the case by default when using debuild (it clears the env).  The
 testsuite fails with:
make[2]: Target `check' not remade because of errors.
make[2]: Leaving directory `/home/lool/scratch/glibc/glibc-2.9'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/lool/scratch/glibc/glibc-2.9/build-tree/amd64-libc'
#
# Testsuite failures, someone should be working towards
# fixing these! They are listed here for the purpose of
# regression testing during builds.
# Format: <Failed test>, Error <Make error code> [(ignored)]
#
tst-tables.out, Error 1
***************
Encountered regressions that don't match expected failures:
tst-tables.out, Error 1

 I could reproduce this on amd64 and armel.

 This is due to an upstream bug: "env -i make check" fails but "env -i
 SHELL=/bin/zsh make check" passes.

 While debugging this issue with SHELL not being set properly, I saw
 this in glibc's debian/rules:
    SHELL                 ?= =/bin/sh -e
   However this statement is never going to be useful: make sets SHELL
 despite whatever value the env might have, so ?= will never set it.
   You can check this with this Makefile:
    SHELL ?= foo

    $(info origin: $(origin SHELL))
    $(info SHELL=$(SHELL))
    $(info env SHELL=$(shell printenv SHELL))

    default:
            $(SHELL) -c printenv SHELL | grep ^SHELL=

 % env -i make
 origin: default
 SHELL=/bin/sh
 env SHELL=
 /bin/sh -c printenv SHELL | grep ^SHELL=
 make: *** [default] Error 1

 % env -i SHELL=bar make
 origin: file
 SHELL=/bin/sh
 env SHELL=bar
 /bin/sh -c printenv SHELL | grep ^SHELL=
 SHELL=bar

 => SHELL was never defined with ?=, neither for env nor for Makefile

 If you change the Makefile to set SHELL as follows:
    SHELL := =/bin/sh -e

 % env -i SHELL=bar make
 origin: file
 SHELL==/bin/sh -e
 /bin/sh: =/bin/sh: No such file or directory
 env SHELL=
 =/bin/sh -e -c printenv SHELL | grep ^SHELL=
 /bin/sh: =/bin/sh: No such file or directory
 make: *** [default] Error 127

 => can't even run; I think the "=" was bogus


 The main bug is that the glibc testsuite relies on SHELL being in the
 env; some tests are called from within upstream's make check with:
    $(SHELL) foo.sh
 which will work even if SHELL isn't exported in the env because it's
 set by make.  However some scripts call other scripts which call other
 scripts, and in that latter case $(SHELL) isn't in the env anymore:
    <original env>: SHELL unset
        debian/rules: SHELL defined by make
            $(MAKE) check: SHELL defined by make (same process as above)
                $(SHELL) tst-tables.sh: SHELL is a regular var, not env
                    $(SHELL) tst-table.sh: SHELL unset (new shell)
                        $(SHELL) tst-table-charmap.sh: fails

 => SHELL being unset in the original env might fail the testsuite.


 So there are two things to fix with SHELL:
 a) fixing upstream's assumptions; this can be done by adding this
    snippet near the top of tst-tables.sh:
    if test "$SHELL" = ""; then SHELL=/bin/sh; fi
    export SHELL

    I confirmed this works for a shebang using bash, zsh, dash, and
    posh; only bash sets SHELL by default, and it sets it to the user
    account db value; so it will work either if called directly or if
    called from make as "$(SHELL) tst-tables.sh".

    Alternatively, you could opt to export SHELL from rules, but in this
    case you need to be very careful with how you set SHELL in your
    rules, or pass it to all upstream command invocations.

    You could either export SHELL from rules, but in that case /bin/sh
    will always be used for the upstream build:
    export SHELL

    Or if you want to check the value of $SHELL in the env in which
    debian/rules was called and export SHELL if it wasn't set, you can
    use:
    ENV_SHELL := $(shell echo $$SHELL)
    ifeq ($(ENV_SHELL),)
    # SHELL is always set to /bin/sh by make
    export SHELL
    endif

    (but I don't recommend this last solution since it will use random
    shells to run the upstream rules)

 b) fixing debian/rules' SHELL, either by dropping it or by setting it
    right.  I think the intent was to use "-e" and the "=" seemed bogus,
    so I propose simply doing:
    SHELL += -e
    NB: I didn't check whether debian/rules actually are set -e safe

    if you care to use the env's SHELL, using -e, and passing that to
    upstream, you could do:
    ENV_SHELL := $(shell echo $$SHELL)
    $(error $(ENV_SHELL))
    ifeq ($(ENV_SHELL),)
    # SHELL is always set to /bin/sh by make
    SHELL += -e
    export SHELL
    else
    # prefer using the env's SHELL, but "set -e"
    SHELL := $(ENV_SHELL) -e
    endif

    (but I don't recommend this last solution since it will use random
    shells to run debian/rules)


 Finally, I'd like to note two things:
 - In glibc 2.1.2-2, Joel Klecker noted:
    Call make with SHELL=/bin/bash, as the test suite seems to rely on
    bash behavior.
   so you might want to force bash
 - In glibc 2.9-2, Aurélien Jarno noted:
    rules.d/build.mk: define SHELL as /bin/bash.
   this doesn't seem to be enough for the testsuite, you might want to
   revert this or fix the testsuite to use that.


 Bye

-- System Information:
Debian Release: 5.0
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)

Kernel: Linux 2.6.28-1-686 (SMP w/2 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

-- 
Loïc Minier



Reply to: