Re: [PATCH 0/3] gnustep-make: stage 1 of cross-compile awareness

[Ladislav Michl]

On Sun, Jan 12, 2020 at 11:37:59PM +0100, Wolfgang Lux wrote:
> > Am 12.01.2020 um 23:23 schrieb Fred Kiefer <address@hidden>:
> > 
> > Thank you for your patches. The first two are clear improvements. The third 
> > one also looks good to me, but I must admit it is really hard to read 
> > through in a patch mail (and I only read the configure.ac part. After the 
> > merge I would regenerate configure locally). As far as I can tell you 
> > mostly moved things around and unified the compiler checks to always use  
> > ${CLANG_CC}.
> > I am not sure whether we will need a copyright assignment for these 
> > patches. The changes themselves are not that big but it might be better if 
> > you sign one.
> 
> I'm sorry, but with regard to the second patch (Fix GNU Make detection) I 
> feel I must disagree that it's an improvement. From a cursory look it, it 
> seems that this change falls short of the common mistake to assume that 
> /bin/sh is /bin/bash, which simply is not true (not even for Linux, look at 
> Debian). The parameter substitutions
>  MAJOR_MAKE_VERSION="${MAKE_VERSION%%.*}"
>  MINOR_MAKE_VERSION="${MAKE_VERSION#*.}"
>  MINOR_MAKE_VERSION="${MINOR_MAKE_VERSION%%.*}"
> are prone to fail on most shells other than bash.

Care to read section '2.6.2 Parameter Expansion' of POSIX.1-2017 specs?
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

I do agree I didn't check since when it is part of POSIX. Also it would
help if you could point at any document listing supported systems.

Speaking about shell compatibility, configure works as expected with:
- GNU bash, version 5.0.11(1)-release
- dash-0.5.2
- zsh 5.7.1
- ksh (AT&T Research) 2020.0.0

while:
$ csh configure
DUALCASE=1: Command not found.
export: Command not found.
Missing }.

$ elvish configure
Parse error: unexpected rune '&'
configure, line 17: if test -n "${ZSH_VERSION+set}" && (emulate sh) >/dev/null 
2>&1; then :

So there's clearly room for improvements ;-)

Btw, I started this change as current configure claims my make does not
support $(info ...). I'll look into it more closely to find less intrusive
change.

Best regards,
        ladis