replace bashisms in m4/sage_spkg_collect.m4, m4/sage_spkg_enable.m4, build/pkgs/*/spkg-configure.m4, src/bin/sage-env, build/make/Makefile.in

[orlitzky]

Autoconf scripts should be POSIX sh rather than bash, but there are a few bashisms in m4/sage_spkg_collect.m4 and m4/sage_spkg_enable.m4:

• The quoted newlines $'\n'
• The use of VAR+=VALUE

Some of these should be straightforward to fix. The format

SAGE_BUILT_PACKAGES+="    $SPKG_NAME \\"$'\n'

can be changed to

SAGE_BUILT_PACKAGES="$SAGE_BUILT_PACKAGES $SPKG_NAME"

since the newline is only used to make the BUILT_PACKAGES rule in the resulting Makefile look nice. Changing them to spaces doesn't change what the rule does. I'm not sure about

SAGE_PACKAGE_VERSIONS+="vers_$SPKG_NAME = $SPKG_VERSION"$'\n

and

SAGE_PACKAGE_DEPENDENCIES+="deps_$SPKG_NAME = $DEPS"$'\n'

though, because those are inserted verbatim into the Makefile as rules, and the newlines probably matter.

Upstream: Fixed upstream, in a later stable release.

CC: @dimpase @mkoeppe @embray @vbraun

Component: build: configure

Author: Michael Orlitzky

Branch: 0e66a0a

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29345

[mkoeppe]

comment:1

I agree in general - do you actually run this with a CONFIG_SHELL other than bash? Is the other configure code clean?

[mkoeppe]

comment:2

This should probably be done on top of #29287, which touches the same file

[orlitzky]

comment:3

Things seem to run OK with dash if I comment out the part of configure.ac that forces bash. Of course I can't be sure that it works until I have a suggestion for how to fix SAGE_PACKAGE_VERSIONS and SAGE_PACKAGE_DEPENDENCIES.

[mkoeppe]

comment:4

One way to address this is to make things a liitle more static (see #29114 - "Only ./bootstrap should glob build/pkgs"), also getting rid of GNU-make-isms and macrology in build/make/Makefile.

bootstrap already emits macro calls for many packages into m4/sage_spkg_configures.m4, of three types.

• m4_sinclude([build/pkgs/arb/spkg-configure.m4])
• SAGE_SPKG_CONFIGURE_ARB
• SAGE_SPKG_ENABLE([ninja_build], [optional])
  Might as well emit another line for every package that creates whatever is needed in the Makefile.

[orlitzky]

Author: Michael Orlitzky

[orlitzky]

Commit: 4a09db5

[orlitzky]

Branch: u/mjo/ticket/29345

[orlitzky]

comment:6

The printf stuff is going to make anyone reading the code think we're stupid, but it works and didn't involve changing much code. I just completed a full build using /bin/dash as my shell.

[sagetrac-git]

Changed commit from 4a09db5 to 489d8cc

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

26bf7af	Trac #29345: replace a few obsolete "-a" tests with "-e".
fea7ad9	Trac #29345: replace a bash array with something portable.
4f0a56a	Trac #29345: replace a few uses of "source" with "."
42a63fd	Trac #29345: fix some bashisms in sage-env's resolvelinks() function.
489d8cc	Trac #29345: don't force SHELL=bash any longer.

[mkoeppe]

comment:8

Nice on first glance. I'll be happy to do a full review after 9.1 is out

[dimpase]

comment:9

it's a bit in vain, as there are bashisms or just "bash required" in several Sage packages.

[dimpase]

comment:10

regarding newline, perhaps https://stackoverflow.com/questions/21880891/newline-character-in-posix-shell says something.

[orlitzky]

comment:11

Replying to @dimpase:

it's a bit in vain, as there are bashisms or just "bash required" in several Sage packages.

Ultimately my goal is to not install any sage packages =)

I'll always need bash installed for other stuff, but ./configure is noticeably faster with dash. It may be a "micro-optimization" but half of what I do lately is re-run ./configure.

[orlitzky]

comment:12

Replying to @dimpase:

regarding newline, perhaps https://stackoverflow.com/questions/21880891/newline-character-in-posix-shell says something.

The suggestion there is to print a newline and a space at the end of each line, and then remove the space afterwards. Since all of the lines in question are indented, what I did instead was switch to printing a newline and some spaces at the beginning of each line -- that way the newline is still followed by spaces (and therefore doesn't get dropped), but the spaces aren't "extra" and don't need to be removed.

[dimpase]

comment:13

needs rebase.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

165d554	Trac #29345: fix most autotools bashisms.
1b5dafb	Trac #29345: fix one more newline-constant bashism in SAGE_SPKG_ENABLE.
c4a74d8	Trac #29345: replace a few obsolete "-a" tests with "-e".
fcd9cef	Trac #29345: replace a bash array with something portable.
839623a	Trac #29345: replace a few uses of "source" with "."
0717ec4	Trac #29345: fix some bashisms in sage-env's resolvelinks() function.
279765f	Trac #29345: don't force SHELL=bash any longer.

[sagetrac-git]

Changed commit from 489d8cc to 279765f

[orlitzky]

comment:15

fixed, thanks

[dimpase]

comment:16

A interesting test (apart from using dash) would be to use ksh (on OpenBSD - trying this now for lolz) or zsh (the latter is the default on recent macOS, I am told).

[orlitzky]

comment:17

./configure fails with /bin/sh -> /bin/zsh at,

Checking whether SageMath should install SPKG mpir...
checking gmp.h usability... yes
checking gmp.h presence... yes
checking for gmp.h... yes
checking gmpxx.h usability... yes
checking gmpxx.h presence... yes
checking for gmpxx.h... yes
checking for library containing __gmpq_cmp_z... -lgmp
./configure:break:10258: not in while, until, select, or repeat loop

Since it was executed via /bin/sh, I think zsh is already in bourne-shell compatibility mode.

[dimpase]

comment:18

ksh also notices a stray break and prints a warning, no error.

[dimpase]

comment:19

with ksh/openbsd, the ugly hack in spkg-install of lrcalc does not work, as copied over config.status gets really confused, and complains about print not found etc.
The following (needs autotools isntalled, obviously) fixes this:

-cp "$SAGE_ROOT"/config/config.* .
+autoreconf -ivf

[orlitzky]

Dependencies: 29098

[sagetrac-git]

Changed commit from 2f5f383 to b3f9f55

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

647e64c	Trac #29345: fix one more newline-constant bashism in SAGE_SPKG_ENABLE.
4e0b8dd	Trac #29345: replace a few obsolete "-a" tests with "-e".
955da4d	Trac #29345: replace a bash array with something portable.
76bdb66	Trac #29345: replace a few uses of "source" with "."
7f46fae	Trac #29345: fix some bashisms in sage-env's resolvelinks() function.
791194a	Trac #29345: don't force SHELL=bash any longer.
c22ef41	Trac #29345: remove "break" statements from AC_SEARCH_LIBS.
20a1cdb	Trac #29345: don't use sage's config.status for the lrcalc build.
7077feb	Trac #29345: replace the function that populates the CVXOPT_* variables.
b3f9f55	Trac #29345: add Dima's SPKG patches for ksh compatibility.

[orlitzky]

comment:42

That last one is just a rebase onto the rebase in #29098.

[dimpase]

Changed dependencies from 29098 to #29098

[dimpase]

comment:44

oops, needs another rebase, over the beta0 that just came out.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

bc3ea8e	Trac #29345: fix one more newline-constant bashism in SAGE_SPKG_ENABLE.
fb475f3	Trac #29345: replace a few obsolete "-a" tests with "-e".
305d8cf	Trac #29345: replace a bash array with something portable.
d0dff56	Trac #29345: replace a few uses of "source" with "."
5ac420b	Trac #29345: fix some bashisms in sage-env's resolvelinks() function.
0a61795	Trac #29345: don't force SHELL=bash any longer.
5db5318	Trac #29345: remove "break" statements from AC_SEARCH_LIBS.
e810ad1	Trac #29345: don't use sage's config.status for the lrcalc build.
93c9921	Trac #29345: replace the function that populates the CVXOPT_* variables.
0e66a0a	Trac #29345: add Dima's SPKG patches for ksh compatibility.

[sagetrac-git]

Changed commit from b3f9f55 to 0e66a0a

[orlitzky]

Changed dependencies from #29098 to none

[orlitzky]

comment:46

Done again.

[dimpase]

comment:47

lgtm

[dimpase]

Reviewer: Dima Pasechnik

[dimpase]

Changed commit from 0e66a0a to 604657a

[dimpase]

comment:49

rebased over 9.2.beta1 - making this critical, as we're approaching a rebase hell state with this ticket not merged for 4 weeks.

---

Last 10 new commits:

bbf5265	Trac #29345: fix one more newline-constant bashism in SAGE_SPKG_ENABLE.
beccf24	Trac #29345: replace a few obsolete "-a" tests with "-e".
ff63210	Trac #29345: replace a bash array with something portable.
bb69c7e	Trac #29345: replace a few uses of "source" with "."
c549738	Trac #29345: fix some bashisms in sage-env's resolvelinks() function.
b445b6e	Trac #29345: don't force SHELL=bash any longer.
18dd124	Trac #29345: remove "break" statements from AC_SEARCH_LIBS.
6a5d73f	Trac #29345: don't use sage's config.status for the lrcalc build.
3d8ba26	Trac #29345: replace the function that populates the CVXOPT_* variables.
604657a	Trac #29345: add Dima's SPKG patches for ksh compatibility.

[dimpase]

Changed branch from u/mjo/ticket/29345 to public/build/29345rebased

[mkoeppe]

comment:50

0e66a0a is already merged on Volker's branch.

[mkoeppe]

Changed commit from 604657a to 0e66a0a

[mkoeppe]

Changed branch from public/build/29345rebased to u/mjo/ticket/29345

[vbraun]

Changed branch from u/mjo/ticket/29345 to 0e66a0a

[dimpase]

comment:53

Replying to @orlitzky:

New commits fix the break statements and probably lrcalc.

no, lrcalc on OpenBSD is still broken due to this print issue. Probably we really need to run a modern autoconf on it and replace the resulting scripts.

[dimpase]

Changed commit from 0e66a0a to none