On Thu, 29 Jun 2017 21:40:32 +0200, Simon Marchi wrote:
> On 2017-06-19 22:55, Jan Kratochvil wrote:
> > +if [ "$want_index" = true ]; then
> > +  if [ -z "$GDB_ADD_INDEX" ]
> > +  then
> 
> Nit: use the same style ("then" on same line or next line) for all ifs.

Done.


> > +      if [ -f ./contrib/gdb-add-index.sh ]
> > +      then
> > +	  GDB_ADD_INDEX="./contrib/gdb-add-index.sh"
> > +      elif [ -f ../contrib/gdb-add-index.sh ]
> > +      then
> > +	  GDB_ADD_INDEX="../contrib/gdb-add-index.sh"
> > +      elif [ -f ../../contrib/gdb-add-index.sh ]
> > +      then
> > +	  GDB_ADD_INDEX="../../contrib/gdb-add-index.sh"
> > +      else
> > +	  echo "$myname: unable to find usable contrib/gdb-add-index.sh" >&2
> > +	  exit 1
> > +      fi
> > +  fi
> > +fi
> 
> This strategy doesn't work for out of tree builds.

I use it always with explicit $GDB_ADD_INDEX so this auto-detection does not
really matter to me:
	https://git.jankratochvil.net/?p=nethome.git;a=commitdiff;h=44099fb634eca7837d30ab9b6afde0ec2838f705


> Perhaps cooking up
> something based on $0 would be better?  I see that the GDB variable is
> auto-detected the same way, but the gdb binary is found in the build
> directory, so it works for the tests, whereas gdb-add-index.sh is in the
> source directory.
> 
> I think it would still be nice to improve how GDB is auto-detected (in
> another patch),

OK, I believe that is unrelated to this patchset.  I am fine to always specify
all the tools explicitly.


> Is PIPESTATUS bash specific?  If so, we should probably change the
> interpreter to
> 
> #!/usr/bin/env bash
> 
> The CC/CXX_FOR_TARGET variables in cc-with-tweaks.exp also use /bin/sh
> explicitly, I think it those can just be removed.

Done.


Thanks,
Jan