Building from git breaks when /bin/sh isn't bash

  • Open
  • quality assurance status badge
Details
3 participants
  • elaexuotee
  • Maxim Cournoyer
  • pelzflorian (Florian Pelz)
Owner
unassigned
Submitted by
elaexuotee
Severity
normal
E
E
elaexuotee wrote on 15 Apr 2020 11:06
(address . bug-guix@gnu.org)
2O2NKRGHD2OZD.30CKDJBOP4LE3@wilsonb.com
When building from git, ./bootstrap ends up generating (via automake) several
Makefiles that set SHELL = /bin/sh. However, some targets contain rules that
make use of bashisms. This leads to breakage when /bin/sh is something other
than bash.

In particular, I am building from a foreign distro which links /bin/sh to dash.
Currently, this ends up breaking the build, the details of which I reported
to guix-devel in [0].

As a workaround, at the moment we have to force make's SHELL to point to bash.
The cleanest way to do this is probably as follows:

$ make SHELL=$(command -v sh)

since from within guix environment --pure guix, sh ends up pointing to bash.
Just for clarity, here is how this looks for me, currently:

$ git rev-parse HEAD
2708ae3d69b54d8323ca84fd9a7fb108a6ee96ba
$ guix environment --pure guix
$ readlink -f $(command -v sh)
/gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/bash

Attachment: signature.asc
P
P
pelzflorian (Florian Pelz) wrote on 15 Apr 2020 14:21
(address . elaexuotee@wilsonb.com)(address . 40641@debbugs.gnu.org)
20200415122149.j7b6bcgvrp5cpq5l@pelzflorian.localdomain
On Wed, Apr 15, 2020 at 06:06:25PM +0900, elaexuotee--- via Bug reports for GNU Guix wrote:
Toggle quote (9 lines)
> When building from git, ./bootstrap ends up generating (via automake) several
> Makefiles that set SHELL = /bin/sh. However, some targets contain rules that
> make use of bashisms. This leads to breakage when /bin/sh is something other
> than bash.
>
> In particular, I am building from a foreign distro which links /bin/sh to dash.
> Currently, this ends up breaking the build, the details of which I reported
> to guix-devel in [0].

https://bugs.gnu.org/25258 is related. Your workaround may be more welcome.

Regards,
Florian
E
E
elaexuotee wrote on 17 Apr 2020 16:57
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)(address . 40641@debbugs.gnu.org)
38M8QJ1PDKHCI.2ZBQF20W9BE03@wilsonb.com
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:
Toggle quote (16 lines)
> On Wed, Apr 15, 2020 at 06:06:25PM +0900, elaexuotee--- via Bug reports for GNU Guix wrote:
> > When building from git, ./bootstrap ends up generating (via automake) several
> > Makefiles that set SHELL = /bin/sh. However, some targets contain rules that
> > make use of bashisms. This leads to breakage when /bin/sh is something other
> > than bash.
> >
> > In particular, I am building from a foreign distro which links /bin/sh to dash.
> > Currently, this ends up breaking the build, the details of which I reported
> > to guix-devel in [0].
>
> <https://bugs.gnu.org/25258> is related. Your workaround may be more welcome.
>
> Regards,
> Florian


Florian,

Thanks for the pointer. I ended up doing a little bit of sleuthing and think
I figured out a relatively clean fix---a simple one-liner in configure.ac.
Attached is a proof-of-concept patch against master (974bf81776).

Currently, autoconf sets make's shell to whatever it thinks is best. On a
foreign distribution, this often ends up something external to guix profile.
However, when this isn't bash, we run into problems.

The patch's idea is to let make use its hard-coded default shell. A guix-built
make will correctly fallback to whichever sh is in the profile, so for `guix
environment guix' this effectively becomes $GUIX_ENVIRONMENT/bin/sh. For
example,

$ echo '$(info $(SHELL))' | make -f -
/gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/sh
make: *** No targets. Stop.

I belive this should do the Right Thing. However, is there anything I am
missing? Perhaps this change would break build scenaries I am not thinking of?

Cheers,
B. Wilson
From 6a5533fde0580a777a10f1155714f23a003003d9 Mon Sep 17 00:00:00 2001
From: "B. Wilson" <elaexuotee@wilsonb.com>
Date: Thu, 16 Apr 2020 17:02:06 +0900
Subject: [PATCH] build: Let make use its hard-coded default shell
To: guix-patches@gnu.org

* configure.ac: Set AM_SUBST_NOTMAKE([SHELL])
---
configure.ac | 4 ++++
1 file changed, 4 insertions(+)

Toggle diff (17 lines)
diff --git a/configure.ac b/configure.ac
index 6a6a020585..dbb06f2258 100644
--- a/configure.ac
+++ b/configure.ac
@@ -11,6 +11,10 @@ AC_CONFIG_AUX_DIR([build-aux])
AM_INIT_AUTOMAKE([1.14 gnu silent-rules subdir-objects \
color-tests parallel-tests -Woverride -Wno-portability])
+# Use make's hard-coded default shell. The make in a guix profile
+# defaults to the Right Thing, e.g. $GUIX_ENVIRONMENT/bin/sh
+AM_SUBST_NOTMAKE([SHELL])
+
# Enable silent rules by default.
AM_SILENT_RULES([yes])
--
2.26.1
Attachment: signature.asc
M
M
Maxim Cournoyer wrote on 10 Jun 2022 02:34
(address . elaexuotee@wilsonb.com)
87h74tgxui.fsf@gmail.com
Hello,

elaexuotee@wilsonb.com writes:

Toggle quote (39 lines)
> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:
>> On Wed, Apr 15, 2020 at 06:06:25PM +0900, elaexuotee--- via Bug reports for GNU Guix wrote:
>> > When building from git, ./bootstrap ends up generating (via automake) several
>> > Makefiles that set SHELL = /bin/sh. However, some targets contain rules that
>> > make use of bashisms. This leads to breakage when /bin/sh is something other
>> > than bash.
>> >
>> > In particular, I am building from a foreign distro which links /bin/sh to dash.
>> > Currently, this ends up breaking the build, the details of which I reported
>> > to guix-devel in [0].
>>
>> <https://bugs.gnu.org/25258> is related. Your workaround may be more welcome.
>>
>> Regards,
>> Florian
>
>
> Florian,
>
> Thanks for the pointer. I ended up doing a little bit of sleuthing and think
> I figured out a relatively clean fix---a simple one-liner in configure.ac.
> Attached is a proof-of-concept patch against master (974bf81776).
>
> Currently, autoconf sets make's shell to whatever it thinks is best. On a
> foreign distribution, this often ends up something external to guix profile.
> However, when this isn't bash, we run into problems.
>
> The patch's idea is to let make use its hard-coded default shell. A guix-built
> make will correctly fallback to whichever sh is in the profile, so for `guix
> environment guix' this effectively becomes $GUIX_ENVIRONMENT/bin/sh. For
> example,
>
> $ echo '$(info $(SHELL))' | make -f -
> /gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/sh
> make: *** No targets. Stop.
>
> I belive this should do the Right Thing. However, is there anything I am
> missing? Perhaps this change would break build scenaries I am not thinking of?

This seems odd to me. Perhaps it'd be cleaner to detect which shell is
used at configure time to detect when /bin/sh != Bash, and warn that if
there are issues, the user should set the SHELL variable to Bash.

Or if the Bashisms are scarce enough, perhaps we can rewrite the
routines in POSIXly correct shell, although this being a GNU project I
don't really see the merit of forcing lesser shells (and less readable
code) on ourselves.

Could you provide a list of the problematic targets? Or if my
suggestion sounds good, give it a shot?

Thanks :-)

Maxim
P
P
pelzflorian (Florian Pelz) wrote on 13 Jun 2022 16:40
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20220613144005.gv2j5igdmisnaiqv@pelzflorian.localdomain
Only the tests are affected, as far as I can tell. make runs fine.
The issue with “gnu/local.mk” from [1] got fixed in 2019 via
92d00ca4661e186022732a47956a2bc0ef16be96.

But Makefile.am has

SH_LOG_COMPILER = $(top_builddir)/test-env $(SHELL)
AM_SH_LOG_FLAGS = -x -e

Probably autoconf can be made to detect if $(SHELL) is bash or zsh
somehow™. But I suppose it is not important and I won’t fix it.

Regards,
Florian
M
M
Maxim Cournoyer wrote on 14 Jun 2022 18:09
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
874k0nfcq6.fsf@gmail.com
Hello,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:

Toggle quote (12 lines)
> Only the tests are affected, as far as I can tell. make runs fine.
> The issue with “gnu/local.mk” from [1] got fixed in 2019 via
> 92d00ca4661e186022732a47956a2bc0ef16be96.
>
> But Makefile.am has
>
> SH_LOG_COMPILER = $(top_builddir)/test-env $(SHELL)
> AM_SH_LOG_FLAGS = -x -e
>
> Probably autoconf can be made to detect if $(SHELL) is bash or zsh
> somehow™. But I suppose it is not important and I won’t fix it.

Wilson, could you confirm whether there's still something to fix here?
Else, let's close this ticket and move on.

Thanks,

Maxim
E
E
elaexuotee wrote on 21 Jun 2022 02:20
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
<2N3KLAV9USDUO.2FGVQNI398LFY@"@WILSONB.COM>
Toggle quote (3 lines)
> Wilson, could you confirm whether there's still something to fix here?
> Else, let's close this ticket and move on.

The original issue arose on a Void Linux foreign distro installation. I have
long since converted to Guix System, so cannot easily test the original
environment.

However, Void just runs dash as it's default sh, so you could be able to sanity
check with something like

$ guix shell -C dash guix make <etc>
$ ln -s $(command -v dash) /bin/sh
$ ./configure --localstatedir && make
P
P
pelzflorian (Florian Pelz) wrote on 21 Jun 2022 11:02
(address . elaexuotee@wilsonb.com)
20220621090254.kyskzwkliku53mob@pelzflorian.localdomain
Thank you for getting back to the bug. I am in the same situation in
that I use Guix System now. :D

On Tue, Jun 21, 2022 at 09:20:28AM +0900, elaexuotee@wilsonb.com wrote:
Toggle quote (7 lines)
> so you could be able to sanity
> check with something like
>
> $ guix shell -C dash guix make <etc>
> $ ln -s $(command -v dash) /bin/sh
> $ ./configure --localstatedir && make

I had done exactly this.

guix shell --container --network dash git pkg-config gnutls guile guile-avahi guile-gcrypt guile-json guile-lib guile-sqlite3 guile-zlib guile-lzlib guile-zstd guile-ssh guile-git autoconf automake gettext texinfo graphviz help2man po4a findutils sed coreutils tar xz m4 diffutils grep gcc-toolchain sqlite libgcrypt gawk make glibc-locales -- dash

Many tests fail because of the container though, so I’m not sure how
big the effect is. At least tests/guix-package.sh still use type -P
which is not POSIX, but I don’t think it should be changed nor should
there be a check if $SHELL can do what we need, because we don’t know
which bash features we need.

Regards,
Florian
E
E
elaexuotee wrote on 4 Jul 2022 13:22
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
<3PFHFCEJGQHEZ.2UZN2W0ONMXEV@"@WILSONB.COM>
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:
Toggle quote (21 lines)
> Thank you for getting back to the bug. I am in the same situation in
> that I use Guix System now. :D
>
> On Tue, Jun 21, 2022 at 09:20:28AM +0900, elaexuotee@wilsonb.com wrote:
> > so you could be able to sanity
> > check with something like
> >
> > $ guix shell -C dash guix make <etc>
> > $ ln -s $(command -v dash) /bin/sh
> > $ ./configure --localstatedir && make
>
> I had done exactly this.
>
> guix shell --container --network dash git pkg-config gnutls guile guile-avahi guile-gcrypt guile-json guile-lib guile-sqlite3 guile-zlib guile-lzlib guile-zstd guile-ssh guile-git autoconf automake gettext texinfo graphviz help2man po4a findutils sed coreutils tar xz m4 diffutils grep gcc-toolchain sqlite libgcrypt gawk make glibc-locales -- dash
>
> Many tests fail because of the container though, so I’m not sure how
> big the effect is. At least tests/guix-package.sh still use type -P
> which is not POSIX, but I don’t think it should be changed nor should
> there be a check if $SHELL can do what we need, because we don’t know
> which bash features we need.

Excellent. I agree it's probably not worth POSIXifying the scripts. Forcing
make to default to guix's bash seems like the right approach IMHO, so +1 for
that fix.

FWIW, I ended up working around the original issue by explicitly telling make
to use guix's bash, anyway:

$ guix environment guix bash
$ CONFIG_SHELL=$(command -v bash) ./configure --localstatedir=/var
M
M
Maxim Cournoyer wrote on 7 Jul 2022 23:52
(address . elaexuotee@wilsonb.com)
87edywvbdz.fsf@gmail.com
Hello,

elaexuotee@wilsonb.com writes:

Toggle quote (37 lines)
> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:
>> Thank you for getting back to the bug. I am in the same situation in
>> that I use Guix System now. :D
>>
>> On Tue, Jun 21, 2022 at 09:20:28AM +0900, elaexuotee@wilsonb.com wrote:
>> > so you could be able to sanity
>> > check with something like
>> >
>> > $ guix shell -C dash guix make <etc>
>> > $ ln -s $(command -v dash) /bin/sh
>> > $ ./configure --localstatedir && make
>>
>> I had done exactly this.
>>
>> guix shell --container --network dash git pkg-config gnutls guile
>> guile-avahi guile-gcrypt guile-json guile-lib guile-sqlite3
>> guile-zlib guile-lzlib guile-zstd guile-ssh guile-git autoconf
>> automake gettext texinfo graphviz help2man po4a findutils sed
>> coreutils tar xz m4 diffutils grep gcc-toolchain sqlite libgcrypt
>> gawk make glibc-locales -- dash
>>
>> Many tests fail because of the container though, so I’m not sure how
>> big the effect is. At least tests/guix-package.sh still use type -P
>> which is not POSIX, but I don’t think it should be changed nor should
>> there be a check if $SHELL can do what we need, because we don’t know
>> which bash features we need.
>
> Excellent. I agree it's probably not worth POSIXifying the scripts. Forcing
> make to default to guix's bash seems like the right approach IMHO, so +1 for
> that fix.
>
> FWIW, I ended up working around the original issue by explicitly telling make
> to use guix's bash, anyway:
>
> $ guix environment guix bash
> $ CONFIG_SHELL=$(command -v bash) ./configure --localstatedir=/var

OK. Good to know, glad it can be easily worked around.

Closing.

Maxim
Closed
P
P
pelzflorian (Florian Pelz) wrote on 8 Jul 2022 10:53
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
878rp4c7em.fsf@pelzflorian.de
Wait wait Maxim, the discussion was that
"B. Wilson" <elaexuotee@wilsonb.com> proposed
Toggle quote (1 lines)
> [PATCH] build: Let make use its hard-coded default shell
> To: guix-patches@gnu.org
Toggle quote (1 lines)
>
> * configure.ac: Set AM_SUBST_NOTMAKE([SHELL])
> +# Use make's hard-coded default shell. The make in a guix profile
> +# defaults to the Right Thing, e.g. $GUIX_ENVIRONMENT/bin/sh
Toggle quote (1 lines)
> +AM_SUBST_NOTMAKE([SHELL])
Then Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
Toggle quote (1 lines)
> This seems odd to me. Perhaps it'd be cleaner to detect which shell is
> used at configure time to detect when /bin/sh != Bash, and warn that if
> there are issues, the user should set the SHELL variable to Bash.

elaexuotee@wilsonb.com writes:
Toggle quote (4 lines)
> Excellent. I agree it's probably not worth POSIXifying the scripts. Forcing
> make to default to guix's bash seems like the right approach IMHO, so +1 for
> that fix.

I think we’re not on the same page. Is AM_SUBST_NOTMAKE([SHELL]) really
problematic? Is seems like there is a legitimate use-case that foreign
distro users with /bin/sh = dash would want “guix shell -D guix -- make”
to just work without workaround? We could use elaexuotee’s
AM_SUBST_NOTMAKE([SHELL]) patch, could we not?

Regards,
Florian
P
P
pelzflorian (Florian Pelz) wrote on 8 Jul 2022 10:58
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
874jzsc76v.fsf@pelzflorian.de
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
Toggle quote (3 lines)
> “guix shell -D guix -- make”
> to just work without workaround?

Argh, I meant to write “guix shell -D guix -- make check” …

Regards,
Florian
E
E
elaexuotee wrote on 8 Jul 2022 11:51
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
<3VE840YMQJPO2.2G4HBP5YDDIV4@"@WILSONB.COM>
Toggle quote (6 lines)
> I think we’re not on the same page. Is AM_SUBST_NOTMAKE([SHELL]) really
> problematic? Is seems like there is a legitimate use-case that foreign
> distro users with /bin/sh = dash would want “guix shell -D guix -- make”
> to just work without workaround? We could use elaexuotee’s
> AM_SUBST_NOTMAKE([SHELL]) patch, could we not?

Just to be clear, that CONFIG_SHELL workaround was a giant pain to figure out.
The errors produced when /bin/sh = dash are not at all obvious as to the root
cause, and finding the existence of CONFIG_SHELL takes a certain familiarity
with autoconf, which I didn't have until significant manual reading.

Seems pretty draconian to leave that for foreign distro devs if we do indeed
have a good fix.
M
M
Maxim Cournoyer wrote on 10 Jul 2022 06:56
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
87mtdhftw7.fsf@gmail.com
Hello,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:

Toggle quote (26 lines)
> Wait wait Maxim, the discussion was that
> "B. Wilson" <elaexuotee@wilsonb.com> proposed
>> [PATCH] build: Let make use its hard-coded default shell
> > To: guix-patches@gnu.org
>>
> > * configure.ac: Set AM_SUBST_NOTMAKE([SHELL])
> > +# Use make's hard-coded default shell. The make in a guix profile
> > +# defaults to the Right Thing, e.g. $GUIX_ENVIRONMENT/bin/sh
>> +AM_SUBST_NOTMAKE([SHELL])
>
> Then Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>> This seems odd to me. Perhaps it'd be cleaner to detect which shell is
> > used at configure time to detect when /bin/sh != Bash, and warn that if
> > there are issues, the user should set the SHELL variable to Bash.
>
> elaexuotee@wilsonb.com writes:
>> Excellent. I agree it's probably not worth POSIXifying the scripts. Forcing
>> make to default to guix's bash seems like the right approach IMHO, so +1 for
>> that fix.
>
> I think we’re not on the same page. Is AM_SUBST_NOTMAKE([SHELL]) really
> problematic? Is seems like there is a legitimate use-case that foreign
> distro users with /bin/sh = dash would want “guix shell -D guix -- make”
> to just work without workaround? We could use elaexuotee’s
> AM_SUBST_NOTMAKE([SHELL]) patch, could we not?

Indeed, I had misunderstood, apologies. I've read the Autoconf/Automake
Info manuals about AM_SUBST and AM_SUBST_NOTMAKE, but I don't have a
clear understanding of the mechanisms involved.

from info '(autoconf) config.status Invocation':

-- Variable: CONFIG_SHELL
The shell with which to run ‘configure’. It must be
Bourne-compatible, and the absolute name of the shell should be
passed. The default is a shell that supports ‘LINENO’ if
available, and ‘/bin/sh’ otherwise.

So it appears to me that by default, it'd look for a shell that supports
LINENO if available, such as /bin/bash or something else? E.g., not use
the user's SHELL environment variable directly, but that can be
overridden with CONFIG_SHELL.

Using AC_SUBST_NOTMAKE([SHELL]) would cause SHELL to be substituted in
the build system files but prevent setting Make variables such as SHELL
= '/bin/...' in generated Makefiles... how do that end up causing the
Guix-provided Make to use its own "known" shell?

It seems to me that a potential pitfall would be that by adding
AC_SUBST_NOTMAKE([SHELL]), we'd change the default behavior of Autoconf,
which is to honor CONFIG_SHELL and set the SHELL Make variable based on
that; it seems it could be simpler to document that users on systems
using a Bourne incompatible shell should set CONFIG_SHELL to a Bourne
compatible one to build Guix from sources.

Is someone able to explain how the suggested fix work in more details?

Thanks,

Maxim
E
E
elaexuotee wrote on 10 Jul 2022 12:13
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
<33RZXP6OBFFCS.22N5QDGK5AQQA@"@WILSONB.COM>
CONFIG_SHELL simply acts as a user override; it's not part of autoconf's core
logic. That role belongs to the SHELL macro, who's picks the first available of
the following:

- CONFIG_SHELL environment variable,
- SHELL environment variable, or
- /bin/sh

See autoconf's m4sugar/m4sh.m4 for the gory details. Arguably, this should
also be updated to point to a fixed /bin/sh output fallback.

Anyway, AM_SUBST_NOTMAKE([SHELL]), cf. '(automake) Optional', simply tells
automake to not define SHELL inside the generated Makefile. This means that
make will instead use it's default, which in our case is hard-coded to the
/bin/sh in its implicit bash-minimal dependency. For detailed info about this
behaviour of make, see '(make) Choosing the Shell'. Note, however, you will
have to do a mental sed-replace of "/bin/sh" with "<bash-minimal>/bin/sh" when
reading that page.


Cheers,
B.
M
M
Maxim Cournoyer wrote on 10 Jul 2022 21:55
(address . elaexuotee@wilsonb.com)
87wnckeo8j.fsf@gmail.com
Hi,

elaexuotee@wilsonb.com writes:

Toggle quote (19 lines)
> CONFIG_SHELL simply acts as a user override; it's not part of autoconf's core
> logic. That role belongs to the SHELL macro, who's picks the first available of
> the following:
>
> - CONFIG_SHELL environment variable,
> - SHELL environment variable, or
> - /bin/sh
>
> See autoconf's m4sugar/m4sh.m4 for the gory details. Arguably, this should
> also be updated to point to a fixed /bin/sh output fallback.
>
> Anyway, AM_SUBST_NOTMAKE([SHELL]), cf. '(automake) Optional', simply tells
> automake to not define SHELL inside the generated Makefile. This means that
> make will instead use it's default, which in our case is hard-coded to the
> /bin/sh in its implicit bash-minimal dependency. For detailed info about this
> behaviour of make, see '(make) Choosing the Shell'. Note, however, you will
> have to do a mental sed-replace of "/bin/sh" with "<bash-minimal>/bin/sh" when
> reading that page.

Thanks for the extra details. So if I understand correctly, and
re-reading your original message, the issue is that some tests shell
scripts contain Bashisms that fail to run on POSIX shells such as Dash?
Couldn't we just identify these tests with the proper shebang?
e.g. '#!/usr/bin/env bash' where it is required?

I've just 'ln -sf $(guix build dash)/bin/dash /bin/sh && export
SHELL=/bin/sh' on my Guix System, and could rebuild Guix master from
scratch successfully:

make[1]: Leaving directory '/home/maxim/src/guix-master'
$ echo $?
0
$ ./pre-inst-env guix describe
Git checkout:
repository: /home/maxim/src/guix/.git/worktrees
branch: test-dash-as-bin-sh
commit: bf0a646a5bcde489b602c58fbb63a93acb9d08f6
$ echo $SHELL
/bin/sh
$ ls -al /bin/sh
lrwxrwxrwx 1 root root 66 Jul 10 15:11 /bin/sh -> /gnu/store/nm0hccsphymxi8c24xmg6ixm9vcf25xb-dash-0.5.11.5/bin/dash
$ grep SHELL Makefile
[...]
SHELL = /bin/sh

I'll now try the tests.

Thanks,

Maxim
M
M
Maxim Cournoyer wrote on 11 Jul 2022 15:48
(address . elaexuotee@wilsonb.com)
87h73nep5x.fsf@gmail.com
Hello,

[...]

Toggle quote (22 lines)
> I've just 'ln -sf $(guix build dash)/bin/dash /bin/sh && export
> SHELL=/bin/sh' on my Guix System, and could rebuild Guix master from
> scratch successfully:
>
> make[1]: Leaving directory '/home/maxim/src/guix-master'
> $ echo $?
> 0
> $ ./pre-inst-env guix describe
> Git checkout:
> repository: /home/maxim/src/guix/.git/worktrees
> branch: test-dash-as-bin-sh
> commit: bf0a646a5bcde489b602c58fbb63a93acb9d08f6
> $ echo $SHELL
> /bin/sh
> $ ls -al /bin/sh
> lrwxrwxrwx 1 root root 66 Jul 10 15:11 /bin/sh -> /gnu/store/nm0hccsphymxi8c24xmg6ixm9vcf25xb-dash-0.5.11.5/bin/dash
> $ grep SHELL Makefile
> [...]
> SHELL = /bin/sh
>
> I'll now try the tests.

I've now done so, and there are only 3 tests that fail due to /bin/sh ->
dash:

Toggle snippet (349 lines)
FAIL: tests/guix-package.sh
FAIL: tests/guix-home.sh
FAIL: tests/guix-repl.sh

FAIL: tests/guix-package
========================

+ guix package --version
guix package (GNU Guix) 1.3.0.22041-bf0a6
Copyright (C) 2022 the Guix authors
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
+ module_dir=t-guix-package-16322
+ profile=t-profile-16322
+ tmpfile=t-guix-package-file-16322
+ rm -f t-profile-16322 t-guix-package-file-16322
+ trap rm -f "$profile" "$profile.lock" "$profile-"[0-9]* "$tmpfile"; rm -rf "$module_dir" t-home-16322 EXIT
+ guix package --bootstrap -e +
guix package: error: expression "+" does not evaluate to a package
+ guix build guile-bootstrap
accepted connection from pid 16340, user maxim
+ guix package --bootstrap -p t-profile-16322 -i /home/maxim/src/guix-master/test-tmp/store/ff4yyg2g39ri2zpm0lbmvc2s2f5addv3-guile-bootstrap-2.0
accepted connection from pid 16347, user maxim
The following package will be installed:
guile-bootstrap 2.0

The following derivation will be built:
/home/maxim/src/guix-master/test-tmp/store/4m9bi66d6b4lvj33n792flj071cxip1k-profile.drv

building profile with 1 package...
hint: Consider setting the necessary environment variables by running:

GUIX_PROFILE="/home/maxim/src/guix-master/t-profile-16322"
. "$GUIX_PROFILE/etc/profile"

Alternately, see `guix package --search-paths -p "/home/maxim/src/guix-master/t-profile-16322"'.

+ guix package -A guile-bootstrap
+ cut -f 1-2
+ guix package -p t-profile-16322 -I
+ cut -f 1-2
+ test guile-bootstrap 2.0 = guile-bootstrap 2.0
+ guix package -p t-profile-16322 -I
+ cut -f 3
+ test out = out
+ rm t-profile-16322
+ guix package --bootstrap -p t-profile-16322 -i guile-bootstrap
accepted connection from pid 16381, user maxim
The following package will be installed:
guile-bootstrap 2.0

hint: Consider setting the necessary environment variables by running:

GUIX_PROFILE="/home/maxim/src/guix-master/t-profile-16322"
. "$GUIX_PROFILE/etc/profile"

Alternately, see `guix package --search-paths -p "/home/maxim/src/guix-master/t-profile-16322"'.

+ test -L t-profile-16322
+ test -L t-profile-16322-1-link
+ test -f t-profile-16322/bin/guile
+ guix gc --list-live
+ readlink t-profile-16322-1-link
+ grep /home/maxim/src/guix-master/test-tmp/store/bbxfpsy329libdc30s62az73w8x0b7cv-profile
accepted connection from pid 16388, user maxim
finding garbage collector roots...
accepted connection from pid 16397, user maxim
determining live/dead paths...
/home/maxim/src/guix-master/test-tmp/store/bbxfpsy329libdc30s62az73w8x0b7cv-profile
+ guix package --bootstrap -p t-profile-16322 -i guile-bootstrap
accepted connection from pid 16404, user maxim
The following package will be upgraded:
guile-bootstrap (dependencies or package changed)

nothing to be done
+ test -L t-profile-16322
+ test -L t-profile-16322-1-link
+ test -f t-profile-16322-2-link
+ test -f t-profile-16322/bin/guile
+ guix package -e (begin (use-modules (guix) (gnu packages base)) (package (inherit sed) (supported-systems (list)))) -n
accepted connection from pid 16411, user maxim
The following package would be installed:
sed 4.8

guix package: error: package sed@4.8 does not support x86_64-linux
+ uname -m
+ guix package -i novena-eeprom -n
accepted connection from pid 16419, user maxim
The following package would be installed:
novena-eeprom 2.3

guix package: error: package novena-eeprom@2.3 does not support x86_64-linux
+ break
+ guix package --bootstrap -n -p t-profile-16322 -i g-wrap guile@2.0
accepted connection from pid 16427, user maxim
The following packages would be installed:
g-wrap 1.9.15
guile 2.0.14

guix package: error: profile contains conflicting entries for guile
guix package: error: first entry: guile@2.0.14 /home/maxim/src/guix-master/test-tmp/store/7f6yrypzqyppdcap71ya9342i4kmb3wd-guile-2.0.14
guix package: error: second entry: guile@2.2.7 /home/maxim/src/guix-master/test-tmp/store/wsnd10ajsz7vaapw2bxp8rw4h4x86406-guile-2.2.7
guix package: error: ... propagated from g-wrap@1.9.15
hint: Backtrace:
In ice-9/eval.scm:
619:8 19 (_ #(#(#<directory (guile-user) 7f4e08128c80>)))
In guix/ui.scm:
2238:7 18 (run-guix . _)
2201:10 17 (run-guix-command _ . _)
In ice-9/boot-9.scm:
1752:10 16 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
In guix/status.scm:
835:3 15 (_)
815:4 14 (call-with-status-report _ _)
In guix/store.scm:
1298:8 13 (call-with-build-handler #<procedure 7f4e06dfb2d0 at guix/ui.scm:1170:2 (continue store …> …)
In guix/build/syscalls.scm:
1425:3 12 (_)
1392:4 11 (call-with-file-lock/no-wait _ _ _)
In guix/scripts/package.scm:
151:19 10 (build-and-use-profile #<store-connection 256.99 7f4e081390a0> "t-profile-16322" #<<mani…> …)
In guix/store.scm:
2168:25 9 (run-with-store #<store-connection 256.99 7f4e081390a0> _ #:guile-for-build _ #:system _ # …)
In guix/profiles.scm:
1935:2 8 (_ _)
358:4 7 (_ _)
In guix/store.scm:
1883:0 6 (loop _ _)
In ice-9/boot-9.scm:
1685:16 5 (raise-exception _ #:continuable? _)
In guix/ui.scm:
757:16 4 (_ _)
310:42 3 (display-hint "Try upgrading both @code{guile} and @code{g-wrap},\nor remove one of them…" …)
In ice-9/boot-9.scm:
1747:15 2 (with-exception-handler #<procedure 7f4df271fdb0 at ice-9/boot-9.scm:1831:7 (exn)> _ # _ # …)
In guix/build/syscalls.scm:
2284:35 1 (_)
2273:8 0 (terminal-window-size _)

guix/build/syscalls.scm:2273:8: In procedure terminal-window-size:
In procedure terminal-window-size: Inappropriate ioctl for device
+ guix package --bootstrap -n -p t-profile-16322 -i g-wrap guile@2.0 --allow-collisions
accepted connection from pid 16434, user maxim
The following packages would be installed:
g-wrap 1.9.15
guile 2.0.14

+ guix package -p t-profile-16322 --search-paths
export PATH="t-profile-16322/bin"
+ guix package -p t-profile-16322 --search-paths
+ grep ^export PATH=
export PATH="t-profile-16322/bin"
+ guix package -p t-profile-16322 --search-paths
+ wc -l
+ test 1 = 1
+ set -e
+ set -x
+ guix package --search-paths=prefix -p /home/maxim/src/guix-master/t-profile-16322
+ eval export PATH="/home/maxim/src/guix-master/t-profile-16322/bin${PATH:+:}$PATH"
+ export PATH=/home/maxim/src/guix-master/t-profile-16322/bin:/home/maxim/src/guix-master/scripts:/home/maxim/src/guix-master:/gnu/store/18zvp6m7c63r66849g0pj20bxi3mc501-profile/bin:/gnu/store/18zvp6m7c63r66849g0pj20bxi3mc501-profile/sbin:/home/maxim/.nix-profile/bin:/home/maxim/.local/bin/:/run/setuid-programs:/home/maxim/.config/guix/current/bin:/home/maxim/.guix-profile/bin:/home/maxim/.guix-profile/sbin:/run/current-system/profile/bin:/run/current-system/profile/sbin
+ type -P guile
+ test -P: not found
guile is /home/maxim/src/guix-master/t-profile-16322/bin/guile = /home/maxim/src/guix-master/t-profile-16322/bin/guile
+ rm -f t-profile-16322 t-profile-16322.lock t-profile-16322-1-link t-guix-package-file-16322
+ rm -rf t-guix-package-16322 t-home-16322
FAIL tests/guix-package.sh (exit status: 1)

cause: test -P


FAIL: tests/guix-home
=====================

+ set -e
+ guix home --version
guix show (GNU Guix) 1.3.0.22041-bf0a6
Copyright (C) 2022 the Guix authors
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
+ guile -c (use-modules (guix config))(display %storedir)
+ NIX_STORE_DIR=/gnu/store
+ guile -c (use-modules (guix config))(display %localstatedir)
+ localstatedir=/var
+ GUIX_DAEMON_SOCKET=/var/guix/daemon-socket/socket
+ export NIX_STORE_DIR GUIX_DAEMON_SOCKET
+ guile -c (use-modules (guix)) (exit (false-if-exception (open-connection)))
+ dirname /gnu/store
+ STORE_PARENT=/gnu
+ export STORE_PARENT
+ test /gnu = /
+ mktemp -d
+ test_directory=/tmp/tmp.OAmVzSob58
+ trap chmod -Rf +w "$test_directory"; rm -rf "$test_directory" EXIT
+ cd /tmp/tmp.OAmVzSob58
+ cat
+ echo -n # dot-bashrc test file for guix home
+ guix home extension-graph home.scm
+ grep label = "home-activation"
"140636280345728" [label = "home-activation", shape = box, fontname = sans];
+ guix home extension-graph home.scm
+ grep label = "home-symlink-manager"
"140025808877824" [label = "home-symlink-manager", shape = box, fontname = sans];
+ guix home extension-graph home.scm
+ grep label = "home"
"139908936525792" [label = "home", shape = box, fontname = sans];
+ guix home shepherd-graph home.scm
guix home: error: service of type 'home-shepherd' not found
+ container_supported
+ guile -c ((@ (guix scripts environment) assert-container-features))
+ return 0
+ guix home container home.scm -- true
[...]
The following derivations will be built:
/gnu/store/2xy8zsfkw9v91ryin87ppqdj88ig4p84-home.drv
/gnu/store/0s1m03p4vdcsxcy4iy0p7yn0i4bjbh3c-on-first-login.drv
/gnu/store/c2py3dx3izjfslsdn6g7m1y0pvaz52ya-profile.drv
/gnu/store/fwlni5mm6k9sv78xhrzl8gs6ypsqcwsw-activate.drv
/gnu/store/4badi8l0vfddhg2hkn1rlbpk4f3s17r5-update-symlinks.drv

43.0 MB will be downloaded
listing Emacs sub-directories...
building profile with 2 packages...
...........................................
building /gnu/store/0s1m03p4vdcsxcy4iy0p7yn0i4bjbh3c-on-first-login.drv...
building /gnu/store/4badi8l0vfddhg2hkn1rlbpk4f3s17r5-update-symlinks.drv...
The following build is still in progress:
/gnu/store/4badi8l0vfddhg2hkn1rlbpk4f3s17r5-update-symlinks.drv

building /gnu/store/fwlni5mm6k9sv78xhrzl8gs6ypsqcwsw-activate.drv...
building /gnu/store/2xy8zsfkw9v91ryin87ppqdj88ig4p84-home.drv...
substitute: .substitute: .[Kupdating substitutes from 'http://127.0.0.1:8181'... 0.0%.substitute: .[Kupdating substitutes from 'http://127.0.0.1:8181'... 100.0%
substitute: .substitute: .[Kupdating substitutes from 'https://ci.guix.gnu.org'... 0.0%.substitute: .[Kupdating substitutes from 'https://ci.guix.gnu.org'... 100.0%
substitute: .substitute: .[Kupdating substitutes from 'https://bordeaux.guix.gnu.org'... 0.0%.substitute: .[Kupdating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0%
The following derivations will be built:
/gnu/store/j60d0x8dfg97l4rgrxhra2v66zcyv4g3-module-import.drv
/gnu/store/a707jiqfsd287442lhx5k6l8rr68a4lp-module-import-compiled.drv
/gnu/store/s4578ab9is6z1akkhg76dk9a03w78bj4-home-system-profile.drv

building /gnu/store/j60d0x8dfg97l4rgrxhra2v66zcyv4g3-module-import.drv...
listing Emacs sub-directories...
The following build is still in progress:
/gnu/store/j60d0x8dfg97l4rgrxhra2v66zcyv4g3-module-import.drv

building profile with 1 package...
The following build is still in progress:
/gnu/store/j60d0x8dfg97l4rgrxhra2v66zcyv4g3-module-import.drv

building /gnu/store/a707jiqfsd287442lhx5k6l8rr68a4lp-module-import-compiled.drv...
[...]
+ guix home container home.scm -- echo $HOME
+ test /home/maxim = /home/maxim
+ + guix home container home.scmgrep -- the content of cat
~/.config/test.conf
the content of ~/.config/test.conf
+ guix home container home.scm -- test -h ~/.bashrc
+ guix home container home.scm -- id -u
+ test 1000 = 1000
+ guix home container home.scm -- test -f $HOME/sample/home.scm
+ guix home container home.scm --expose=/tmp/tmp.OAmVzSob58=/home/maxim/sample -- test -f $HOME/sample/home.scm
+ guix home container home.scm --expose=/tmp/tmp.OAmVzSob58=/home/maxim/sample -- rm -v $HOME/sample/home.scm
rm: cannot remove '/home/maxim/sample/home.scm': Read-only file system
+ HOME=/tmp/tmp.OAmVzSob58
+ export HOME
+ echo # This file will be overridden and backed up.
+ mkdir /tmp/tmp.OAmVzSob58/.config
+ echo This file will be overridden too.
+ echo This file will stay around.
+ guix home reconfigure /tmp/tmp.OAmVzSob58/home.scm
guix home: warning: cannot determine provenance for current system
Symlinking /tmp/tmp.OAmVzSob58/.profile -> /gnu/store/dann7r1095xll0kji5yl0ql07096rc8j-shell-profile... done
Symlinking /tmp/tmp.OAmVzSob58/.bash_profile -> /gnu/store/flqaxzvgfv2g3415mhmq6c0zbzdzv2k4-bash_profile... done
Backing up /tmp/tmp.OAmVzSob58/.bashrc... done
Symlinking /tmp/tmp.OAmVzSob58/.bashrc -> /gnu/store/npd40qqa8hsvm7p5aqc3sj79hxjsrdin-bashrc... done
Backing up /tmp/tmp.OAmVzSob58/.config/test.conf... done
Symlinking /tmp/tmp.OAmVzSob58/.config/test.conf -> /gnu/store/bdixb09v30bvhpgi2f6ndiq25wzb9l74-tmp-file.txt... done
Symlinking /tmp/tmp.OAmVzSob58/.config/fontconfig/fonts.conf -> /gnu/store/4261pxafny0g2myhh9yj1771ry7k05lc-fonts.conf... done
done
Finished updating symlinks.

Comparing /gnu/store/non-existing-generation/profile/share/fonts and
/gnu/store/18h8fhfkpfyl8mvkfn9za8sj7dyxzzd4-home/profile/share/fonts... done (same)
Evaluating on-change gexps.

On-change gexps evaluation finished.

+ test -d /tmp/tmp.OAmVzSob58/.guix-home
+ test -h /tmp/tmp.OAmVzSob58/.bash_profile
+ test -h /tmp/tmp.OAmVzSob58/.bashrc
+ tail -n 2 /tmp/tmp.OAmVzSob58/.bashrc
+ test # dot-bashrc test file for guix home
# the content of bashrc-test-config.sh == # dot-bashrc test file for guix home
# the content of bashrc-test-config.sh
./tests/guix-home.sh: 137: test: # dot-bashrc test file for guix home
# the content of bashrc-test-config.sh: unexpected operator
+ chmod -Rf +w /tmp/tmp.OAmVzSob58
+ rm -rf /tmp/tmp.OAmVzSob58
FAIL tests/guix-home.sh (exit status: 2)

cause: bashism in bashrc-test-config.sh


FAIL: tests/guix-repl
=====================

+ guix repl --version
guix repl (GNU Guix) 1.3.0.22041-bf0a6
Copyright (C) 2022 the Guix authors
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
+ mktemp -d
+ test_directory=/tmp/tmp.PREwMwz2Ln
+ export test_directory
+ trap chmod -Rf +w "$test_directory"; rm -rf "$test_directory" EXIT
+ tmpfile=/tmp/tmp.PREwMwz2Ln/foo.scm
+ rm -f /tmp/tmp.PREwMwz2Ln/foo.scm
+ trap rm -f "$tmpfile" EXIT
+ module_dir=t-guix-repl-19912
+ mkdir t-guix-repl-19912
+ trap rm -rf "$module_dir" EXIT
+ cat
+ guix repl /tmp/tmp.PREwMwz2Ln/foo.scm
+ test coreutils = coreutils
+ dirname /tmp/tmp.PREwMwz2Ln/foo.scm
+ cd /tmp/tmp.PREwMwz2Ln
+ basename /tmp/tmp.PREwMwz2Ln/foo.scm
+ guix repl foo.scm
+ test coreutils = coreutils
+ cat
+ cat
+ guix repl /tmp/tmp.PREwMwz2Ln/foo.scm -L t-guix-repl-19912
+ test 42 = 42
+ cat
+ guix repl -- /tmp/tmp.PREwMwz2Ln/foo.scm -a b --input=foo.txt
+ test (-a b --input=foo.txt) = (-a b --input=foo.txt)
+ type -P env
+ cat
+ chmod 755 /tmp/tmp.PREwMwz2Ln/foo.scm
+ /tmp/tmp.PREwMwz2Ln/foo.scm -a b --input=foo.txt
./tests/guix-repl.sh: 1: /tmp/tmp.PREwMwz2Ln/foo.scm: not found
+ test = (-a b --input=foo.txt)
+ rm -rf t-guix-repl-19912
FAIL tests/guix-repl.sh (exit status: 1)

cause: type -P

I'll see if these Bashisms can be easily switched to POSIX variants,
else I'll experiment with setting the shebang of the test scripts to
bash.

Thanks,

Maxim
E
E
elaexuotee wrote on 19 Jul 2022 06:14
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
3NB75IRB8776E.2JKL61N4Y2UUG@wilsonb.com
Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
Toggle quote (4 lines)
> I'll see if these Bashisms can be easily switched to POSIX variants,
> else I'll experiment with setting the shebang of the test scripts to
> bash.

Is there any particular reason to go this route?

Writing portable scripts is full of all sorts of pitfalls and ill-meaning
dragons:

- What if SHELL=tcsh?
- What about incompatible behaviour between bash versions?
- Do we want to write tests to future-proof fixes for the above?

In this case, since the build is running inside a guix shell, I don't really
see any reason to *not* effectively pin the scripts to use the bash available
in that environment.

Am I missing something?

Cheers,
B
M
M
Maxim Cournoyer wrote on 21 Jul 2022 17:29
(address . elaexuotee@wilsonb.com)
8735euqyba.fsf@gmail.com
Hi,

elaexuotee@wilsonb.com writes:

Toggle quote (7 lines)
> Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>> I'll see if these Bashisms can be easily switched to POSIX variants,
>> else I'll experiment with setting the shebang of the test scripts to
>> bash.
>
> Is there any particular reason to go this route?

I can think of at least two:

1. GNU Autotools is designed to work across as many systems as possible,
and produces POSIX-compliant shell scripts such as configure; thus
keeping things in our build system (including the tests authored in
shell scripts) POSIX allows to build and test Guix from source in many
environments, not only in 'guix shell -D guix'.

2. Messing with Autotools-managed variables such as 'SHELL' may end up
causing confusions for those relying on Autotools documented behavior.

Toggle quote (7 lines)
> Writing portable scripts is full of all sorts of pitfalls and ill-meaning
> dragons:
>
> - What if SHELL=tcsh?
> - What about incompatible behaviour between bash versions?
> - Do we want to write tests to future-proof fixes for the above?

You actually don't need to care about Bash incompatible behavior when
targetting POSIX shell script. If the user goes out of their way to use
a non-POSIX shell... well they are on their own.

Toggle quote (4 lines)
> In this case, since the build is running inside a guix shell, I don't really
> see any reason to *not* effectively pin the scripts to use the bash available
> in that environment.

That's true for the specific use case where someone tries to build Guix
inside a 'guix shell -D guix' environment, but there are also people
attempting to build Guix using they system provided dependencies and
shell, where the fix wouldn't help.

My previous experiment showed that the Bashisms used seem limited to
perhaps 2 tests; it doesn't seem too difficult to fix them using the
'shellcheck' tool to spot what syntax used is problematic and needs to
be adjusted.

Thanks,

Maxim
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 40641@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 40641
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch