[PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Hello,

I was non-mildly horrified to find that the rather widely used patch-kernel
script seems to rely on bash despite specifying the interpreter as #!/bin/sh,
since my dash-using Debian install choked on it.

Thus I'm delivering a first, preliminary, non-reviewed change to make
patch-kernel (a little bit more?) POSIX-compatible. It now survives both
a dash and a bash run.

If this mail goes through relatively unscathed, then it might be a good
idea to plant it into -mm via a follow-up mail.

Comments?

- replaced "==" by "="
- the "source" statement most likely needs the ./ prepended, as can be
gathered from e.g. http://osdir.com/ml/colinux.devel/2005-12/msg00036.html
- the newly replaced sed expression below: is it ok? correct? strict enough?

Thanks,

Andreas Mohr

(who's strongly hoping that submitting a patch for this thingy doesn't
automatically equal becoming "maintainer for life" for it ;)

--- linux-2.6.23/scripts/patch-kernel	2007-10-31 21:55:26.000000000 +0100
+++ linux-2.6.23/scripts/patch-kernel.dash	2007-10-31 21:58:37.000000000 +0100
@@ -65,7 +65,7 @@
 patchdir=${2-.}
 stopvers=${3-default}
 
-if [ "$1" == -h -o "$1" == --help -o ! -r "$sourcedir/Makefile" ]; then
+if [ "$1" = "-h" -o "$1" = "--help" -o ! -r "$sourcedir/Makefile" ]; then
 cat << USAGE
 usage: $PNAME [-h] [ sourcedir [ patchdir [ stopversion ] [ -acxx ] ] ]
   source directory defaults to /usr/src/linux,
@@ -185,7 +185,7 @@
 TMPFILE=`mktemp .tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
 grep -E "^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)" $sourcedir/Makefile > $TMPFILE
 tr -d [:blank:] < $TMPFILE > $TMPFILE.1
-source $TMPFILE.1
+. ./$TMPFILE.1
 rm -f $TMPFILE*
 if [ -z "$VERSION" -o -z "$PATCHLEVEL" -o -z "$SUBLEVEL" ]
 then
@@ -202,13 +202,7 @@
 EXTRAVER=
 if [ x$EXTRAVERSION != "x" ]
 then
-	if [ ${EXTRAVERSION:0:1} == "." ]; then
-		EXTRAVER=${EXTRAVERSION:1}
-	else
-		EXTRAVER=$EXTRAVERSION
-	fi
-	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
-	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
+	EXTRAVER=`echo $EXTRAVERSION|sed -s 's/^[\.]\?\([^[:punct:]]*\).*/\1/'`
 fi
 
 #echo "stopvers=$stopvers"
@@ -251,16 +245,16 @@
 do
     CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     EXTRAVER=
-    if [ $stopvers == $CURRENTFULLVERSION ]; then
+    if [ $stopvers = $CURRENTFULLVERSION ]; then
         echo "Stopping at $CURRENTFULLVERSION base as requested."
         break
     fi
 
-    SUBLEVEL=$((SUBLEVEL + 1))
+    SUBLEVEL=$(( $SUBLEVEL + 1 ))
     FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     #echo "#___ trying $FULLVERSION ___"
 
-    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
+    if [ $SUBLEVEL -gt $STOPSUBLEVEL ]; then
 	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
 	exit 1
     fi
@@ -297,7 +291,7 @@
 if [ x$gotac != x ]; then
   # Out great user wants the -ac patches
 	# They could have done -ac (get latest) or -acxx where xx=version they want
-	if [ $gotac == "-ac" ]; then
+	if [ $gotac = "-ac" ]; then
 	  # They want the latest version
 		HIGHESTPATCH=0
 		for PATCHNAMES in $patchdir/patch-${CURRENTFULLVERSION}-ac*\.*

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

Andreas Mohr wrote:
> Hello,
> 
> I was non-mildly horrified to find that the rather widely used patch-kernel
> script seems to rely on bash despite specifying the interpreter as #!/bin/sh,
> since my dash-using Debian install choked on it.
> 
> Thus I'm delivering a first, preliminary, non-reviewed change to make
> patch-kernel (a little bit more?) POSIX-compatible. It now survives both
> a dash and a bash run.
> 
> If this mail goes through relatively unscathed, then it might be a good
> idea to plant it into -mm via a follow-up mail.
> 
> Comments?
> 
> - replaced "==" by "="

Those are OK.

> - the "source" statement most likely needs the ./ prepended, as can be
> gathered from e.g. http://osdir.com/ml/colinux.devel/2005-12/msg00036.html

That email isn't very convincing to me.

http://www.opengroup.org/onlinepubs/009695399/utilities/dot.html just says that
if <filename> does not contain a slash, the search PATH shall be used (searched).
If you want to prevent that, it's OK to use ./, but at least say that in the
patch description, please.


> - the newly replaced sed expression below: is it ok? correct? strict enough?
> 
> Thanks,
> 
> Andreas Mohr
> 
> (who's strongly hoping that submitting a patch for this thingy doesn't
> automatically equal becoming "maintainer for life" for it ;)

Be careful.

> --- linux-2.6.23/scripts/patch-kernel	2007-10-31 21:55:26.000000000 +0100
> +++ linux-2.6.23/scripts/patch-kernel.dash	2007-10-31 21:58:37.000000000 +0100


> @@ -202,13 +202,7 @@
>  EXTRAVER=
>  if [ x$EXTRAVERSION != "x" ]
>  then
> -	if [ ${EXTRAVERSION:0:1} == "." ]; then
> -		EXTRAVER=${EXTRAVERSION:1}
> -	else
> -		EXTRAVER=$EXTRAVERSION
> -	fi
> -	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
> -	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"

What's the problem above?

> +	EXTRAVER=`echo $EXTRAVERSION|sed -s 's/^[\.]\?\([^[:punct:]]*\).*/\1/'`
>  fi
>  
>  #echo "stopvers=$stopvers"
> @@ -251,16 +245,16 @@
>  do
>      CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      EXTRAVER=

>  
> -    SUBLEVEL=$((SUBLEVEL + 1))
> +    SUBLEVEL=$(( $SUBLEVEL + 1 ))

Why are the added spaces needed?

Did you look at
http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html ?

>      FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      #echo "#___ trying $FULLVERSION ___"
>  
> -    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
> +    if [ $SUBLEVEL -gt $STOPSUBLEVEL ]; then

What's the problem here?

>  	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
>  	exit 1
>      fi


-- 
~Randy

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

On Wed, Oct 31, 2007 at 03:24:22PM -0700, Randy Dunlap wrote:
> Andreas Mohr wrote:
> >- the "source" statement most likely needs the ./ prepended, as can be
> >gathered from e.g. http://osdir.com/ml/colinux.devel/2005-12/msg00036.html
> 
> That email isn't very convincing to me.

Actually that place simply was where I found how the

.: 188: .tmpver.eC4856.1: not found

dash error that happened when I replaced the bash-specific "source"
statement with its "." counterpart could be ""fixed"".

> http://www.opengroup.org/onlinepubs/009695399/utilities/dot.html just says 
> that
> if <filename> does not contain a slash, the search PATH shall be used 
> (searched).
> If you want to prevent that, it's OK to use ./, but at least say that in the
> patch description, please.

So this part seems to indicate that merely prepending ./ is problematic,
since ./ implies relative path resolution whereas $TMPFILE.1 could
theoretically be an absolute path already.
It would probably be best to add the ./ to mktemp already, to make sure
we source *exactly* the filename expression we originally mktemp'd.

OTOH prepending ./ to mktemp by default would deny mktemp any
"search for a temporary-capable directory and create the file there"
capabilities. Hmm. I should investigate POSIX shell sourcing
mechanisms more.

> >@@ -202,13 +202,7 @@
> > EXTRAVER=
> > if [ x$EXTRAVERSION != "x" ]
> > then
> >-	if [ ${EXTRAVERSION:0:1} == "." ]; then
> >-		EXTRAVER=${EXTRAVERSION:1}
> >-	else
> >-		EXTRAVER=$EXTRAVERSION
> >-	fi
> >-	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
> >-	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
> 
> What's the problem above?

The variable manipulation seems bash-specific, the resulting dash error was:

linux-2.6.23/scripts/patch-kernel: 205: Syntax error: Bad substitution

which pointed at the

	if [ ${EXTRAVERSION:0:1} == "." ]; then

line.

> > 
> >-    SUBLEVEL=$((SUBLEVEL + 1))
> >+    SUBLEVEL=$(( $SUBLEVEL + 1 ))
> 
> Why are the added spaces needed?

They weren't strictly needed, what was needed was to add the missing $ sign
(you could perhaps say that I added the spaces to emphasize the $ sign).
Maybe they're best removed.

> Did you look at
> http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html ?

No. I should have started by looking at the opengroup.org site when
looking for POSIX specs...

> 
> >     FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
> >     #echo "#___ trying $FULLVERSION ___"
> > 
> >-    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
> >+    if [ $SUBLEVEL -gt $STOPSUBLEVEL ]; then
> 
> What's the problem here?

I got 

linux-2.6.23/scripts/patch-kernel: 270: arith: syntax error: "SUBLEVEL"

thus I simply removed braces since that "fixed" the issue.
May be incorrect, though...


I'll think a bit more about these couple changed places (and whether
this still truly works as intended) and mail a patch then.

(and a big NOTE: I'm no POSIX vs. non-POSIX shell guru at all, only a
semi-versed shell script writer, thus these changes should be reviewed
quite thoroughly)

Thanks for your very fast review,

Andreas Mohr

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

On Thu, 1 Nov 2007 13:11:33 +0100 Andreas Mohr wrote:

> On Wed, Oct 31, 2007 at 03:24:22PM -0700, Randy Dunlap wrote:
> > Andreas Mohr wrote:
> > >- the "source" statement most likely needs the ./ prepended, as can be
> > >gathered from e.g. http://osdir.com/ml/colinux.devel/2005-12/msg00036.html
> > 
> > That email isn't very convincing to me.
> 
> Actually that place simply was where I found how the
> 
> .: 188: .tmpver.eC4856.1: not found
> 
> dash error that happened when I replaced the bash-specific "source"
> statement with its "." counterpart could be ""fixed"".
> 
> > http://www.opengroup.org/onlinepubs/009695399/utilities/dot.html just says 
> > that
> > if <filename> does not contain a slash, the search PATH shall be used 
> > (searched).
> > If you want to prevent that, it's OK to use ./, but at least say that in the
> > patch description, please.
> 
> So this part seems to indicate that merely prepending ./ is problematic,
> since ./ implies relative path resolution whereas $TMPFILE.1 could
> theoretically be an absolute path already.
> It would probably be best to add the ./ to mktemp already, to make sure
> we source *exactly* the filename expression we originally mktemp'd.
> 
> OTOH prepending ./ to mktemp by default would deny mktemp any
> "search for a temporary-capable directory and create the file there"
> capabilities. Hmm. I should investigate POSIX shell sourcing
> mechanisms more.

As I indicated, I don't object to this part of the patch or to
the s/==/=/ part either.  The ./ just needs better patch description.


> > >@@ -202,13 +202,7 @@
> > > EXTRAVER=
> > > if [ x$EXTRAVERSION != "x" ]
> > > then
> > >-	if [ ${EXTRAVERSION:0:1} == "." ]; then
> > >-		EXTRAVER=${EXTRAVERSION:1}
> > >-	else
> > >-		EXTRAVER=$EXTRAVERSION
> > >-	fi
> > >-	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
> > >-	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
> > 
> > What's the problem above?
> 
> The variable manipulation seems bash-specific, the resulting dash error was:
> 
> linux-2.6.23/scripts/patch-kernel: 205: Syntax error: Bad substitution
> 
> which pointed at the
> 
> 	if [ ${EXTRAVERSION:0:1} == "." ]; then
> 
> line.
> 
> > > 
> > >-    SUBLEVEL=$((SUBLEVEL + 1))
> > >+    SUBLEVEL=$(( $SUBLEVEL + 1 ))
> > 
> > Why are the added spaces needed?
> 
> They weren't strictly needed, what was needed was to add the missing $ sign
> (you could perhaps say that I added the spaces to emphasize the $ sign).
> Maybe they're best removed.
> 
> > Did you look at
> > http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html ?
> 
> No. I should have started by looking at the opengroup.org site when
> looking for POSIX specs...
> 
> > 
> > >     FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
> > >     #echo "#___ trying $FULLVERSION ___"
> > > 
> > >-    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
> > >+    if [ $SUBLEVEL -gt $STOPSUBLEVEL ]; then
> > 
> > What's the problem here?
> 
> I got 
> 
> linux-2.6.23/scripts/patch-kernel: 270: arith: syntax error: "SUBLEVEL"
> 
> thus I simply removed braces since that "fixed" the issue.
> May be incorrect, though...
> 
> 
> I'll think a bit more about these couple changed places (and whether
> this still truly works as intended) and mail a patch then.
> 
> (and a big NOTE: I'm no POSIX vs. non-POSIX shell guru at all, only a
> semi-versed shell script writer, thus these changes should be reviewed
> quite thoroughly)

Neither am I.  I read those web pages quickly yesterday, so after
you read them, we can discuss more and/or review more patches.

Thanks.

---
~Randy

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Hi,

On Thu, Nov 01, 2007 at 08:24:57AM -0700, Randy Dunlap wrote:
> On Thu, 1 Nov 2007 13:11:33 +0100 Andreas Mohr wrote:
> > I'll think a bit more about these couple changed places (and whether
> > this still truly works as intended) and mail a patch then.
> > 
> > (and a big NOTE: I'm no POSIX vs. non-POSIX shell guru at all, only a
> > semi-versed shell script writer, thus these changes should be reviewed
> > quite thoroughly)
> 
> Neither am I.  I read those web pages quickly yesterday, so after
> you read them, we can discuss more and/or review more patches.

OK, next iteration (v2).


Make the patch-kernel shell script sufficiently compatible with POSIX shells.


Changes since v1:
- don't actually change string quoting
- prepend ./ at mktemp step already
- remove added superfluous spaces in arithmetic expression
- don't remove double braces in STOPSUBLEVEL evaluation,
  since these are integer values to be compared

Full ChangeLog:
- replaced non-standard "==" by standard "="
- replaced non-standard "source" statement by POSIX "dot" statement
- POSIX shell local file lookup needs ./ prepended, thus have mktemp
  use this from the beginning and comment it properly
- replace non-standard bash string parsing by sed expression
  (is the sed syntax ok? correct? strict enough?)
- added missing $ signs to shell variable names

About the missing $ signs:
http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_14
says:
"If the shell variable x contains a value that forms a valid integer
constant, then the arithmetic expansions
 "$((x))" and "$(($x))" shall return the same value."

Hmm, well, seems dash doesn't... (syntax error).
Thus I still needed to add the $ signs despite opengroup.org specifying
it differently.


Updated version verified again to now work with both bash and dash
on Debian stable.

Patch intended for inclusion in -mm, once it has survived some reviews.

Thanks.

Signed-off-by: Andreas Mohr <andi@lisas.de>

--- linux-2.6.23/scripts/patch-kernel.orig	2007-11-01 22:51:34.000000000 +0100
+++ linux-2.6.23/scripts/patch-kernel	2007-11-01 22:10:14.000000000 +0100
@@ -65,7 +65,7 @@
 patchdir=${2-.}
 stopvers=${3-default}
 
-if [ "$1" == -h -o "$1" == --help -o ! -r "$sourcedir/Makefile" ]; then
+if [ "$1" = -h -o "$1" = --help -o ! -r "$sourcedir/Makefile" ]; then
 cat << USAGE
 usage: $PNAME [-h] [ sourcedir [ patchdir [ stopversion ] [ -acxx ] ] ]
   source directory defaults to /usr/src/linux,
@@ -182,10 +182,12 @@
 }
 
 # set current VERSION, PATCHLEVEL, SUBLEVEL, EXTRAVERSION
-TMPFILE=`mktemp .tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
+# sourcing $TMPFILE.1 below needs lookup in local directory in a POSIX shell,
+# thus prepend ./ to mktemp argument
+TMPFILE=`mktemp ./.tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
 grep -E "^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)" $sourcedir/Makefile > $TMPFILE
 tr -d [:blank:] < $TMPFILE > $TMPFILE.1
-source $TMPFILE.1
+. $TMPFILE.1
 rm -f $TMPFILE*
 if [ -z "$VERSION" -o -z "$PATCHLEVEL" -o -z "$SUBLEVEL" ]
 then
@@ -202,13 +204,7 @@
 EXTRAVER=
 if [ x$EXTRAVERSION != "x" ]
 then
-	if [ ${EXTRAVERSION:0:1} == "." ]; then
-		EXTRAVER=${EXTRAVERSION:1}
-	else
-		EXTRAVER=$EXTRAVERSION
-	fi
-	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
-	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
+	EXTRAVER=`echo $EXTRAVERSION|sed -s 's/^[\.]\?\([^[:punct:]]*\).*/\1/'`
 fi
 
 #echo "stopvers=$stopvers"
@@ -251,16 +247,16 @@
 do
     CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     EXTRAVER=
-    if [ $stopvers == $CURRENTFULLVERSION ]; then
+    if [ $stopvers = $CURRENTFULLVERSION ]; then
         echo "Stopping at $CURRENTFULLVERSION base as requested."
         break
     fi
 
-    SUBLEVEL=$((SUBLEVEL + 1))
+    SUBLEVEL=$(($SUBLEVEL + 1))
     FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     #echo "#___ trying $FULLVERSION ___"
 
-    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
+    if [ $(($SUBLEVEL)) -gt $(($STOPSUBLEVEL)) ]; then
 	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
 	exit 1
     fi
@@ -297,7 +293,7 @@
 if [ x$gotac != x ]; then
   # Out great user wants the -ac patches
 	# They could have done -ac (get latest) or -acxx where xx=version they want
-	if [ $gotac == "-ac" ]; then
+	if [ $gotac = "-ac" ]; then
 	  # They want the latest version
 		HIGHESTPATCH=0
 		for PATCHNAMES in $patchdir/patch-${CURRENTFULLVERSION}-ac*\.*

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

On Thu, 1 Nov 2007 23:16:06 +0100 Andreas Mohr wrote:

[add Herbert]

> Hi,
> 
> On Thu, Nov 01, 2007 at 08:24:57AM -0700, Randy Dunlap wrote:
> > On Thu, 1 Nov 2007 13:11:33 +0100 Andreas Mohr wrote:
> > > I'll think a bit more about these couple changed places (and whether
> > > this still truly works as intended) and mail a patch then.
> > > 
> > > (and a big NOTE: I'm no POSIX vs. non-POSIX shell guru at all, only a
> > > semi-versed shell script writer, thus these changes should be reviewed
> > > quite thoroughly)
> > 
> > Neither am I.  I read those web pages quickly yesterday, so after
> > you read them, we can discuss more and/or review more patches.
> 
> OK, next iteration (v2).
> 
> 
> Make the patch-kernel shell script sufficiently compatible with POSIX shells.
> 
> 
> Changes since v1:
> - don't actually change string quoting
> - prepend ./ at mktemp step already
> - remove added superfluous spaces in arithmetic expression
> - don't remove double braces in STOPSUBLEVEL evaluation,
>   since these are integer values to be compared
> 
> Full ChangeLog:
> - replaced non-standard "==" by standard "="
OK

> - replaced non-standard "source" statement by POSIX "dot" statement
OK

> - POSIX shell local file lookup needs ./ prepended, thus have mktemp
>   use this from the beginning and comment it properly

I would like the patch description to say something like:

Use ./ as a prefix on the TEMP filename so that the current search
PATH will not be searched and thus another file with this same
(pseudo random) name won't be used accidentally.

> - replace non-standard bash string parsing by sed expression
>   (is the sed syntax ok? correct? strict enough?)

I think that this is the part that bothers me.  I can't find
anything at
http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html
that says that this:
	EXTRAVER=${EXTRAVER%%[[:punct:]]*}

is invalid or even optional syntax.  OTOH, it does list such syntax,
so why are we working around this syntax?
Please point out to me what I am missing...

> - added missing $ signs to shell variable names
OK


> About the missing $ signs:
> http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_14
> says:
> "If the shell variable x contains a value that forms a valid integer
> constant, then the arithmetic expansions
>  "$((x))" and "$(($x))" shall return the same value."
> 
> Hmm, well, seems dash doesn't... (syntax error).
> Thus I still needed to add the $ signs despite opengroup.org specifying
> it differently.

Herbert?

> Updated version verified again to now work with both bash and dash
> on Debian stable.
> 
> Patch intended for inclusion in -mm, once it has survived some reviews.
> 
> Thanks.
> 
> Signed-off-by: Andreas Mohr <andi@lisas.de>
> 
> --- linux-2.6.23/scripts/patch-kernel.orig	2007-11-01 22:51:34.000000000 +0100
> +++ linux-2.6.23/scripts/patch-kernel	2007-11-01 22:10:14.000000000 +0100
> @@ -65,7 +65,7 @@
>  patchdir=${2-.}
>  stopvers=${3-default}
>  
> -if [ "$1" == -h -o "$1" == --help -o ! -r "$sourcedir/Makefile" ]; then
> +if [ "$1" = -h -o "$1" = --help -o ! -r "$sourcedir/Makefile" ]; then
>  cat << USAGE
>  usage: $PNAME [-h] [ sourcedir [ patchdir [ stopversion ] [ -acxx ] ] ]
>    source directory defaults to /usr/src/linux,
> @@ -182,10 +182,12 @@
>  }
>  
>  # set current VERSION, PATCHLEVEL, SUBLEVEL, EXTRAVERSION
> -TMPFILE=`mktemp .tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
> +# sourcing $TMPFILE.1 below needs lookup in local directory in a POSIX shell,
> +# thus prepend ./ to mktemp argument
> +TMPFILE=`mktemp ./.tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
>  grep -E "^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)" $sourcedir/Makefile > $TMPFILE
>  tr -d [:blank:] < $TMPFILE > $TMPFILE.1
> -source $TMPFILE.1
> +. $TMPFILE.1
>  rm -f $TMPFILE*
>  if [ -z "$VERSION" -o -z "$PATCHLEVEL" -o -z "$SUBLEVEL" ]
>  then
> @@ -202,13 +204,7 @@
>  EXTRAVER=
>  if [ x$EXTRAVERSION != "x" ]
>  then
> -	if [ ${EXTRAVERSION:0:1} == "." ]; then
> -		EXTRAVER=${EXTRAVERSION:1}
> -	else
> -		EXTRAVER=$EXTRAVERSION
> -	fi
> -	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
> -	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
> +	EXTRAVER=`echo $EXTRAVERSION|sed -s 's/^[\.]\?\([^[:punct:]]*\).*/\1/'`
>  fi
>  
>  #echo "stopvers=$stopvers"
> @@ -251,16 +247,16 @@
>  do
>      CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      EXTRAVER=
> -    if [ $stopvers == $CURRENTFULLVERSION ]; then
> +    if [ $stopvers = $CURRENTFULLVERSION ]; then
>          echo "Stopping at $CURRENTFULLVERSION base as requested."
>          break
>      fi
>  
> -    SUBLEVEL=$((SUBLEVEL + 1))
> +    SUBLEVEL=$(($SUBLEVEL + 1))
>      FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      #echo "#___ trying $FULLVERSION ___"
>  
> -    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
> +    if [ $(($SUBLEVEL)) -gt $(($STOPSUBLEVEL)) ]; then
>  	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
>  	exit 1
>      fi
> @@ -297,7 +293,7 @@
>  if [ x$gotac != x ]; then
>    # Out great user wants the -ac patches
>  	# They could have done -ac (get latest) or -acxx where xx=version they want
> -	if [ $gotac == "-ac" ]; then
> +	if [ $gotac = "-ac" ]; then
>  	  # They want the latest version
>  		HIGHESTPATCH=0
>  		for PATCHNAMES in $patchdir/patch-${CURRENTFULLVERSION}-ac*\.*
> -

Thanks,
---
~Randy

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Herbert Xu]

On Thu, Nov 01, 2007 at 04:08:57PM -0700, Randy Dunlap wrote:
>
> > - replace non-standard bash string parsing by sed expression
> >   (is the sed syntax ok? correct? strict enough?)
> 
> I think that this is the part that bothers me.  I can't find
> anything at
> http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html
> that says that this:
> 	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
> 
> is invalid or even optional syntax.  OTOH, it does list such syntax,

This is POSIX-compliant and has worked with dash from the very
start.

> > About the missing $ signs:
> > http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_14
> > says:
> > "If the shell variable x contains a value that forms a valid integer
> > constant, then the arithmetic expansions
> >  "$((x))" and "$(($x))" shall return the same value."
> > 
> > Hmm, well, seems dash doesn't... (syntax error).
> > Thus I still needed to add the $ signs despite opengroup.org specifying
> > it differently.
> 
> Herbert?

Using variables without dollar signs in arithmetic expansion was
only added to dash very recently.  So please please talk to your
distribution maker to update their dash packages and it will work
correctly.

This usage is compliant with the most recent revision of POSIX
while earlier ones did not specifically require this (due to
the fact that assignment support was not required either).

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Hi,

On Fri, Nov 02, 2007 at 10:01:18AM +0800, Herbert Xu wrote:
> On Thu, Nov 01, 2007 at 04:08:57PM -0700, Randy Dunlap wrote:
> > I think that this is the part that bothers me.  I can't find
> > anything at
> > http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html
> > that says that this:
> > 	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
> > 
> > is invalid or even optional syntax.  OTOH, it does list such syntax,
> 
> This is POSIX-compliant and has worked with dash from the very
> start.

True (just verified again). But then I didn't actually say that this line
is problematic. Since I had to replace the leading . replacement part already
I decided to simply fold everything into one sed expression.

> Using variables without dollar signs in arithmetic expansion was
> only added to dash very recently.  So please please talk to your
> distribution maker to update their dash packages and it will work
> correctly.

"Debian stable.", aka "Update not an option.". Right?

Since the specs list both possibilities I'd simply go with the safe one,
the one that works on older dash versions, too.
Unless adding a $ sign happens to break other equally important
shells...

> This usage is compliant with the most recent revision of POSIX
> while earlier ones did not specifically require this (due to
> the fact that assignment support was not required either).

This is where I'd think the problem is. If dash once upon a time
decided to support the $ variant only and the specs didn't list both
at that time, well...

The dash version on my system is 0.5.3-5, BTW. Even Debian testing
has 0.5.3-7 only.
I'm now quite certain on which side to tweak things ;)

Thanks a lot for your review,

Andreas Mohr

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

On Fri, 2 Nov 2007 21:09:35 +0100 Andreas Mohr wrote:

> Hi,

> I'm now quite certain on which side to tweak things ;)

The other parts of your patch should probably be merged IMO.

Would you resend a trimmed-down patch?
or should I do it?

Thanks,
---
~Randy

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Hi,

On Fri, Nov 02, 2007 at 01:17:07PM -0700, Randy Dunlap wrote:
> On Fri, 2 Nov 2007 21:09:35 +0100 Andreas Mohr wrote:
> 
> > Hi,
> 
> > I'm now quite certain on which side to tweak things ;)
> 
> The other parts of your patch should probably be merged IMO.

Hmm, that particular part consists of _two_ transformations:
- removing the leading dot
- keep everything before [:punct:]

and the first transformation doesn't work on certain dash versions
the way it's currently coded.

> Would you resend a trimmed-down patch?
> or should I do it?

Feel free to go ahead, otherwise I'll try another patch sometime soon.
All I care about is that the result works on (at least)
one shell implementation _more_ than the current status ;)

Thanks a lot,

Andreas Mohr

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

On Mon, 5 Nov 2007 20:58:27 +0100 Andreas Mohr wrote:

> > Would you resend a trimmed-down patch?
> > or should I do it?
> 
> Feel free to go ahead, otherwise I'll try another patch sometime soon.
> All I care about is that the result works on (at least)
> one shell implementation _more_ than the current status ;)

Hi Andreas,

Can you comment on (or test) whether this patch is sufficient
for your needs?  And if so, is the Signed-off-by: A.M. OK?

Thanks.

---

Make the patch-kernel shell script sufficiently compatible with POSIX shells,
i.e., remove bashisms from scripts/patch-kernel.

Full changelog:
- replaced non-standard "==" by standard "="
- replaced non-standard "source" statement by POSIX "dot" command
- use leading ./ on mktemp filename to force the tempfile to a local
  directory, so that the search path is not used
- added missing (optional/not required) $ signs to shell variable names

Signed-off-by: Andreas Mohr <andi@lisas.de>
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>

--- linux-2.6.23/scripts/patch-kernel.orig	2007-11-01 22:51:34.000000000 +0100
+++ linux-2.6.23/scripts/patch-kernel	2007-11-01 22:10:14.000000000 +0100
@@ -65,7 +65,7 @@
 patchdir=${2-.}
 stopvers=${3-default}
 
-if [ "$1" == -h -o "$1" == --help -o ! -r "$sourcedir/Makefile" ]; then
+if [ "$1" = -h -o "$1" = --help -o ! -r "$sourcedir/Makefile" ]; then
 cat << USAGE
 usage: $PNAME [-h] [ sourcedir [ patchdir [ stopversion ] [ -acxx ] ] ]
   source directory defaults to /usr/src/linux,
@@ -182,10 +182,12 @@
 }
 
 # set current VERSION, PATCHLEVEL, SUBLEVEL, EXTRAVERSION
-TMPFILE=`mktemp .tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
+# force $TMPFILEs below to be in local directory: a slash character prevents
+# the dot command from using the search path.
+TMPFILE=`mktemp ./.tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
 grep -E "^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)" $sourcedir/Makefile > $TMPFILE
 tr -d [:blank:] < $TMPFILE > $TMPFILE.1
-source $TMPFILE.1
+. $TMPFILE.1
 rm -f $TMPFILE*
 if [ -z "$VERSION" -o -z "$PATCHLEVEL" -o -z "$SUBLEVEL" ]
 then
@@ -251,16 +247,16 @@
 do
     CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     EXTRAVER=
-    if [ $stopvers == $CURRENTFULLVERSION ]; then
+    if [ $stopvers = $CURRENTFULLVERSION ]; then
         echo "Stopping at $CURRENTFULLVERSION base as requested."
         break
     fi
 
-    SUBLEVEL=$((SUBLEVEL + 1))
+    SUBLEVEL=$(($SUBLEVEL + 1))
     FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     #echo "#___ trying $FULLVERSION ___"
 
-    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
+    if [ $(($SUBLEVEL)) -gt $(($STOPSUBLEVEL)) ]; then
 	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
 	exit 1
     fi
@@ -297,7 +293,7 @@
 if [ x$gotac != x ]; then
   # Out great user wants the -ac patches
 	# They could have done -ac (get latest) or -acxx where xx=version they want
-	if [ $gotac == "-ac" ]; then
+	if [ $gotac = "-ac" ]; then
 	  # They want the latest version
 		HIGHESTPATCH=0
 		for PATCHNAMES in $patchdir/patch-${CURRENTFULLVERSION}-ac*\.*
-

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Hi,

On Wed, Nov 14, 2007 at 02:46:27PM -0800, Randy Dunlap wrote:
> On Mon, 5 Nov 2007 20:58:27 +0100 Andreas Mohr wrote:
> > Feel free to go ahead, otherwise I'll try another patch sometime soon.
> > All I care about is that the result works on (at least)
> > one shell implementation _more_ than the current status ;)
> 
> Hi Andreas,
> 
> Can you comment on (or test) whether this patch is sufficient
> for your needs?  And if so, is the Signed-off-by: A.M. OK?

Sorry, no, using dash (0.5.3-5) there's still a remaining

$ linux-2.6.22/scripts/patch-kernel linux-2.6.22 /usr/src/patch-2.6
Current kernel version is 2.6.22 ( Holy Dancing Manatees, Batman!)
linux-2.6.22/scripts/patch-kernel: 207: Syntax error: Bad substitution

error in the

# strip EXTRAVERSION to just a number (drop leading '.' and trailing additions)
EXTRAVER=
if [ x$EXTRAVERSION != "x" ]
then
>>> [l.207]       if [ ${EXTRAVERSION:0:1} == "." ]; then
                EXTRAVER=${EXTRAVERSION:1}
        else
                EXTRAVER=$EXTRAVERSION
        fi
        EXTRAVER=${EXTRAVER%%[[:punct:]]*}
        #echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
fi

part, which the sed expression (moderately successfully) tried to
take care of.

The good part of the story is that the current corrected almost fully
working version still works with bash (3.1dfsg-8), just like the
original patch-kernel version.

Signed-off-by would be fine by me.

Thanks,

Andreas Mohr

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Herbert Xu]

On Sat, Nov 17, 2007 at 05:33:27PM +0100, Andreas Mohr wrote:
>
> # strip EXTRAVERSION to just a number (drop leading '.' and trailing additions)
> EXTRAVER=
> if [ x$EXTRAVERSION != "x" ]
> then
> >>> [l.207]       if [ ${EXTRAVERSION:0:1} == "." ]; then
>                 EXTRAVER=${EXTRAVERSION:1}
>         else
>                 EXTRAVER=$EXTRAVERSION
>         fi
>         EXTRAVER=${EXTRAVER%%[[:punct:]]*}
>         #echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
> fi
> 
> part, which the sed expression (moderately successfully) tried to
> take care of.

You don't need a sed expression for that.  In fact the inner
if clause can be replaced by the following line:

	EXTRAVER=${EXTRAVERSION#.}

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Sam Ravnborg]

On Sat, Nov 17, 2007 at 05:33:27PM +0100, Andreas Mohr wrote:
> Hi,
> 
> On Wed, Nov 14, 2007 at 02:46:27PM -0800, Randy Dunlap wrote:
> > On Mon, 5 Nov 2007 20:58:27 +0100 Andreas Mohr wrote:
> > > Feel free to go ahead, otherwise I'll try another patch sometime soon.
> > > All I care about is that the result works on (at least)
> > > one shell implementation _more_ than the current status ;)
> > 
> > Hi Andreas,
> > 
> > Can you comment on (or test) whether this patch is sufficient
> > for your needs?  And if so, is the Signed-off-by: A.M. OK?
> 
> Sorry, no, using dash (0.5.3-5) there's still a remaining
> 
> $ linux-2.6.22/scripts/patch-kernel linux-2.6.22 /usr/src/patch-2.6
> Current kernel version is 2.6.22 ( Holy Dancing Manatees, Batman!)
> linux-2.6.22/scripts/patch-kernel: 207: Syntax error: Bad substitution
> 
> error in the
> 
> # strip EXTRAVERSION to just a number (drop leading '.' and trailing additions)
> EXTRAVER=
> if [ x$EXTRAVERSION != "x" ]
> then
> >>> [l.207]       if [ ${EXTRAVERSION:0:1} == "." ]; then
>                 EXTRAVER=${EXTRAVERSION:1}
>         else
>                 EXTRAVER=$EXTRAVERSION
>         fi
>         EXTRAVER=${EXTRAVER%%[[:punct:]]*}
>         #echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
> fi
> 
> part, which the sed expression (moderately successfully) tried to
> take care of.
> 
> The good part of the story is that the current corrected almost fully
> working version still works with bash (3.1dfsg-8), just like the
> original patch-kernel version.

Could you (or Randy) fix it up with the comment from Herbert
and submit the final version to me (with proper changelog and s-o-b).

Thanks,
	Sam

[PATCH] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Make the patch-kernel shell script sufficiently compatible with POSIX
shells,
i.e., remove bashisms from scripts/patch-kernel.
This means that it now also works on dash 0.5.3-5
and still works on bash 3.1dfsg-8.

Full changelog:
- replaced non-standard "==" by standard "="
- replaced non-standard "source" statement by POSIX "dot" command
- use leading ./ on mktemp filename to force the tempfile to a local
  directory, so that the search path is not used
- replace bash syntax to remove leading dot by similar POSIX syntax
- added missing (optional/not required) $ signs to shell variable names

Signed-off-by: Andreas Mohr <andi@lisas.de>
---
Thanks for all comments! I might want to make sure to read more specs
next time...
Cowardly didn't dare to pre-add Randy's line, feel free to ack ;)

--- linux-2.6.23/scripts/patch-kernel.orig	2007-11-17 21:26:47.000000000 +0100
+++ linux-2.6.23/scripts/patch-kernel	2007-11-17 21:27:59.000000000 +0100
@@ -65,7 +65,7 @@
 patchdir=${2-.}
 stopvers=${3-default}
 
-if [ "$1" == -h -o "$1" == --help -o ! -r "$sourcedir/Makefile" ]; then
+if [ "$1" = -h -o "$1" = --help -o ! -r "$sourcedir/Makefile" ]; then
 cat << USAGE
 usage: $PNAME [-h] [ sourcedir [ patchdir [ stopversion ] [ -acxx ] ] ]
   source directory defaults to /usr/src/linux,
@@ -182,10 +182,12 @@
 }
 
 # set current VERSION, PATCHLEVEL, SUBLEVEL, EXTRAVERSION
-TMPFILE=`mktemp .tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
+# force $TMPFILEs below to be in local directory: a slash character prevents
+# the dot command from using the search path.
+TMPFILE=`mktemp ./.tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
 grep -E "^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)" $sourcedir/Makefile > $TMPFILE
 tr -d [:blank:] < $TMPFILE > $TMPFILE.1
-source $TMPFILE.1
+. $TMPFILE.1
 rm -f $TMPFILE*
 if [ -z "$VERSION" -o -z "$PATCHLEVEL" -o -z "$SUBLEVEL" ]
 then
@@ -202,11 +204,7 @@
 EXTRAVER=
 if [ x$EXTRAVERSION != "x" ]
 then
-	if [ ${EXTRAVERSION:0:1} == "." ]; then
-		EXTRAVER=${EXTRAVERSION:1}
-	else
-		EXTRAVER=$EXTRAVERSION
-	fi
+	EXTRAVER=${EXTRAVERSION#.}
 	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
 	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
 fi
@@ -251,16 +249,16 @@
 do
     CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     EXTRAVER=
-    if [ $stopvers == $CURRENTFULLVERSION ]; then
+    if [ $stopvers = $CURRENTFULLVERSION ]; then
         echo "Stopping at $CURRENTFULLVERSION base as requested."
         break
     fi
 
-    SUBLEVEL=$((SUBLEVEL + 1))
+    SUBLEVEL=$(($SUBLEVEL + 1))
     FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     #echo "#___ trying $FULLVERSION ___"
 
-    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
+    if [ $(($SUBLEVEL)) -gt $(($STOPSUBLEVEL)) ]; then
 	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
 	exit 1
     fi
@@ -297,7 +295,7 @@
 if [ x$gotac != x ]; then
   # Out great user wants the -ac patches
 	# They could have done -ac (get latest) or -acxx where xx=version they want
-	if [ $gotac == "-ac" ]; then
+	if [ $gotac = "-ac" ]; then
 	  # They want the latest version
 		HIGHESTPATCH=0
 		for PATCHNAMES in $patchdir/patch-${CURRENTFULLVERSION}-ac*\.*

Re: [PATCH] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

On Sat, 17 Nov 2007 21:51:25 +0100 Andreas Mohr wrote:

> Make the patch-kernel shell script sufficiently compatible with POSIX
> shells,
> i.e., remove bashisms from scripts/patch-kernel.
> This means that it now also works on dash 0.5.3-5
> and still works on bash 3.1dfsg-8.
> 
> Full changelog:
> - replaced non-standard "==" by standard "="
> - replaced non-standard "source" statement by POSIX "dot" command
> - use leading ./ on mktemp filename to force the tempfile to a local
>   directory, so that the search path is not used
> - replace bash syntax to remove leading dot by similar POSIX syntax
> - added missing (optional/not required) $ signs to shell variable names

The use of $(($x)) should not be needed with latest dash....
anyway, Acked-by: Randy Dunlap <randy.dunlap@oracle.com>


> Signed-off-by: Andreas Mohr <andi@lisas.de>
> ---
> Thanks for all comments! I might want to make sure to read more specs
> next time...
> Cowardly didn't dare to pre-add Randy's line, feel free to ack ;)
> 
> --- linux-2.6.23/scripts/patch-kernel.orig	2007-11-17 21:26:47.000000000 +0100
> +++ linux-2.6.23/scripts/patch-kernel	2007-11-17 21:27:59.000000000 +0100
> @@ -65,7 +65,7 @@
>  patchdir=${2-.}
>  stopvers=${3-default}
>  
> -if [ "$1" == -h -o "$1" == --help -o ! -r "$sourcedir/Makefile" ]; then
> +if [ "$1" = -h -o "$1" = --help -o ! -r "$sourcedir/Makefile" ]; then
>  cat << USAGE
>  usage: $PNAME [-h] [ sourcedir [ patchdir [ stopversion ] [ -acxx ] ] ]
>    source directory defaults to /usr/src/linux,
> @@ -182,10 +182,12 @@
>  }
>  
>  # set current VERSION, PATCHLEVEL, SUBLEVEL, EXTRAVERSION
> -TMPFILE=`mktemp .tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
> +# force $TMPFILEs below to be in local directory: a slash character prevents
> +# the dot command from using the search path.
> +TMPFILE=`mktemp ./.tmpver.XXXXXX` || { echo "cannot make temp file" ; exit 1; }
>  grep -E "^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)" $sourcedir/Makefile > $TMPFILE
>  tr -d [:blank:] < $TMPFILE > $TMPFILE.1
> -source $TMPFILE.1
> +. $TMPFILE.1
>  rm -f $TMPFILE*
>  if [ -z "$VERSION" -o -z "$PATCHLEVEL" -o -z "$SUBLEVEL" ]
>  then
> @@ -202,11 +204,7 @@
>  EXTRAVER=
>  if [ x$EXTRAVERSION != "x" ]
>  then
> -	if [ ${EXTRAVERSION:0:1} == "." ]; then
> -		EXTRAVER=${EXTRAVERSION:1}
> -	else
> -		EXTRAVER=$EXTRAVERSION
> -	fi
> +	EXTRAVER=${EXTRAVERSION#.}
>  	EXTRAVER=${EXTRAVER%%[[:punct:]]*}
>  	#echo "$PNAME: changing EXTRAVERSION from $EXTRAVERSION to $EXTRAVER"
>  fi
> @@ -251,16 +249,16 @@
>  do
>      CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      EXTRAVER=
> -    if [ $stopvers == $CURRENTFULLVERSION ]; then
> +    if [ $stopvers = $CURRENTFULLVERSION ]; then
>          echo "Stopping at $CURRENTFULLVERSION base as requested."
>          break
>      fi
>  
> -    SUBLEVEL=$((SUBLEVEL + 1))
> +    SUBLEVEL=$(($SUBLEVEL + 1))
>      FULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      #echo "#___ trying $FULLVERSION ___"
>  
> -    if [ $((SUBLEVEL)) -gt $((STOPSUBLEVEL)) ]; then
> +    if [ $(($SUBLEVEL)) -gt $(($STOPSUBLEVEL)) ]; then
>  	echo "Stopping since sublevel ($SUBLEVEL) is beyond stop-sublevel ($STOPSUBLEVEL)"
>  	exit 1
>      fi
> @@ -297,7 +295,7 @@
>  if [ x$gotac != x ]; then
>    # Out great user wants the -ac patches
>  	# They could have done -ac (get latest) or -acxx where xx=version they want
> -	if [ $gotac == "-ac" ]; then
> +	if [ $gotac = "-ac" ]; then
>  	  # They want the latest version
>  		HIGHESTPATCH=0
>  		for PATCHNAMES in $patchdir/patch-${CURRENTFULLVERSION}-ac*\.*
> -

---
~Randy
desserts:  http://www.xenotime.net/linux/recipes/

[PATCH] Re: [PATCH] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Silence a remaining annoying (or worse, irritating - "is my entire patched tree
broken now!?") bashism-related message that occurs when /bin/sh is configured
to instead deploy dash, a POSIX-compliant shell, as is the pretty much
standard case on e.g. Debian.

# linux-2.6.38.patch-kernel_test/scripts/patch-kernel linux-2.6.38.patch-kernel_test patch-2.6
Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)
===>  linux-2.6.38.patch-kernel_test/scripts/patch-kernel: line 253: [: =: unary operator expected  <===
cannot find patch file: patch-2.6.39
---

Debug log of uncorrected script:

# sh -x linux-2.6.38.patch-kernel_test/scripts/patch-kernel linux-2.6.38.patch-kernel_test patch-2.6
+ PNAME=patch-kernel
+ sourcedir=linux-2.6.38.patch-kernel_test
+ patchdir=patch-2.6
+ stopvers=default
+ '[' linux-2.6.38.patch-kernel_test = -h -o linux-2.6.38.patch-kernel_test = --help -o '!' -r linux-2.6.38.patch-kernel_test/Makefile ']'
+ for PARM in '$*'
+ case $PARM in
+ for PARM in '$*'
+ case $PARM in
++ mktemp ./.tmpver.XXXXXX
+ TMPFILE=./.tmpver.2Yv11Y
+ grep -E '^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)' linux-2.6.38.patch-kernel_test/Makefile
+ tr -d '[:blank:]'
+ . ./.tmpver.2Yv11Y.1
++ VERSION=2
++ PATCHLEVEL=6
++ SUBLEVEL=38
++ EXTRAVERSION=
+ rm -f ./.tmpver.2Yv11Y ./.tmpver.2Yv11Y.1
+ '[' -z 2 -o -z 6 -o -z 38 ']'
++ grep '^NAME' linux-2.6.38.patch-kernel_test/Makefile
+ NAME='NAME = Flesh-Eating Bats with Fangs'
+ NAME=' Flesh-Eating Bats with Fangs'
+ echo 'Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)'
Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)
+ EXTRAVER=
+ '[' x '!=' x ']'
+ '[' default '!=' default ']'
+ STOPSUBLEVEL=9999
+ STOPEXTRA=9999
+ '[' 9999 -lt 38 ']'
+ '[' x '!=' x ']'
+ CURRENTFULLVERSION=2.6.38
+ '[' x '!=' x ']'
+ '[' 9999 -gt 38 ']'
+ :
+ CURRENTFULLVERSION=2.6.38
+ EXTRAVER=
+ '[' = 2.6.38 ']'
linux-2.6.38.patch-kernel_test/scripts/patch-kernel: line 253: [: =: unary operator expected
+ SUBLEVEL=39
+ FULLVERSION=2.6.39
+ '[' 39 -gt 9999 ']'
+ patch=patch-2.6.39
+ findFile patch-2.6/patch-2.6.39
+ filebase=patch-2.6/patch-2.6.39
+ '[' -r patch-2.6/patch-2.6.39.gz ']'
+ '[' -r patch-2.6/patch-2.6.39.bz ']'
+ '[' -r patch-2.6/patch-2.6.39.bz2 ']'
+ '[' -r patch-2.6/patch-2.6.39.zip ']'
+ '[' -r patch-2.6/patch-2.6.39.Z ']'
+ '[' -r patch-2.6/patch-2.6.39 ']'
+ return 1
+ noFile patch-2.6.39
+ echo 'cannot find patch file: patch-2.6.39'
cannot find patch file: patch-2.6.39
+ exit 1




Debug log of patched script (condensed to relevant parts):

+ STOPSUBLEVEL=9999
+ STOPEXTRA=9999
+ '[' 9999 -lt 38 ']'
+ '[' x '!=' x ']'
+ CURRENTFULLVERSION=2.6.38
+ '[' x '!=' x ']'
+ '[' 9999 -gt 38 ']'
+ :
+ CURRENTFULLVERSION=2.6.38
+ EXTRAVER=
+ '[' x = x2.6.38 ']'
+ SUBLEVEL=39
+ FULLVERSION=2.6.39


Since my last patch (the parent mail) happened to modify the very same line,
I'm slightly wondering why last time it didn't exhibit this issue
(IIRC this occurred for the first time a couple months later,
perhaps in newer shell versions).

Patch has been created on git 2.6.39-rc1,
run through checkpatch.pl,
tested to work on dash and still work on bash,
and tested to apply cleanly to 2.6.37, too.

IMHO this is a low-priority item,
certainly not requiring service within current -rc handling,
and neither at -stable (I assume that only point versions
could make good use of this patch anyway).
IOW standard patch circulation.
I'm assuming that Sam would be the one to queue it up,
just like last time.

Thanks!

Signed-off-by: Andreas Mohr <andi@lisas.de>


diff --git a/scripts/patch-kernel b/scripts/patch-kernel
index 46a59ca..20fb25c 100755
--- a/scripts/patch-kernel
+++ b/scripts/patch-kernel
@@ -250,7 +250,7 @@ while :				# incrementing SUBLEVEL (s in v.p.s)
 do
     CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
     EXTRAVER=
-    if [ $STOPFULLVERSION = $CURRENTFULLVERSION ]; then
+    if [ x$STOPFULLVERSION = x$CURRENTFULLVERSION ]; then
         echo "Stopping at $CURRENTFULLVERSION base as requested."
         break
     fi

Re: [PATCH] Re: [PATCH] eradicate bashisms in scripts/patch-kernel

[Randy Dunlap]

On Sun, 3 Apr 2011 20:58:28 +0200 Andreas Mohr wrote:

> Silence a remaining annoying (or worse, irritating - "is my entire patched tree
> broken now!?") bashism-related message that occurs when /bin/sh is configured
> to instead deploy dash, a POSIX-compliant shell, as is the pretty much
> standard case on e.g. Debian.
> 
> # linux-2.6.38.patch-kernel_test/scripts/patch-kernel linux-2.6.38.patch-kernel_test patch-2.6
> Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)
> ===>  linux-2.6.38.patch-kernel_test/scripts/patch-kernel: line 253: [: =: unary operator expected  <===
> cannot find patch file: patch-2.6.39

Hi,

Are you trying to update a 2.6.38 kernel directory to 2.6.39?
I'm a little confused since the command line does not say anything about 2.6.39.

> ---
> 
> Debug log of uncorrected script:
> 
> # sh -x linux-2.6.38.patch-kernel_test/scripts/patch-kernel linux-2.6.38.patch-kernel_test patch-2.6
> + PNAME=patch-kernel
> + sourcedir=linux-2.6.38.patch-kernel_test
> + patchdir=patch-2.6
> + stopvers=default
> + '[' linux-2.6.38.patch-kernel_test = -h -o linux-2.6.38.patch-kernel_test = --help -o '!' -r linux-2.6.38.patch-kernel_test/Makefile ']'
> + for PARM in '$*'
> + case $PARM in
> + for PARM in '$*'
> + case $PARM in
> ++ mktemp ./.tmpver.XXXXXX
> + TMPFILE=./.tmpver.2Yv11Y
> + grep -E '^(VERSION|PATCHLEVEL|SUBLEVEL|EXTRAVERSION)' linux-2.6.38.patch-kernel_test/Makefile
> + tr -d '[:blank:]'
> + . ./.tmpver.2Yv11Y.1
> ++ VERSION=2
> ++ PATCHLEVEL=6
> ++ SUBLEVEL=38
> ++ EXTRAVERSION=
> + rm -f ./.tmpver.2Yv11Y ./.tmpver.2Yv11Y.1
> + '[' -z 2 -o -z 6 -o -z 38 ']'
> ++ grep '^NAME' linux-2.6.38.patch-kernel_test/Makefile
> + NAME='NAME = Flesh-Eating Bats with Fangs'
> + NAME=' Flesh-Eating Bats with Fangs'
> + echo 'Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)'
> Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)
> + EXTRAVER=
> + '[' x '!=' x ']'
> + '[' default '!=' default ']'
> + STOPSUBLEVEL=9999
> + STOPEXTRA=9999
> + '[' 9999 -lt 38 ']'
> + '[' x '!=' x ']'
> + CURRENTFULLVERSION=2.6.38
> + '[' x '!=' x ']'
> + '[' 9999 -gt 38 ']'
> + :
> + CURRENTFULLVERSION=2.6.38
> + EXTRAVER=
> + '[' = 2.6.38 ']'
> linux-2.6.38.patch-kernel_test/scripts/patch-kernel: line 253: [: =: unary operator expected
> + SUBLEVEL=39
> + FULLVERSION=2.6.39
> + '[' 39 -gt 9999 ']'
> + patch=patch-2.6.39
> + findFile patch-2.6/patch-2.6.39
> + filebase=patch-2.6/patch-2.6.39
> + '[' -r patch-2.6/patch-2.6.39.gz ']'
> + '[' -r patch-2.6/patch-2.6.39.bz ']'
> + '[' -r patch-2.6/patch-2.6.39.bz2 ']'
> + '[' -r patch-2.6/patch-2.6.39.zip ']'
> + '[' -r patch-2.6/patch-2.6.39.Z ']'
> + '[' -r patch-2.6/patch-2.6.39 ']'
> + return 1
> + noFile patch-2.6.39
> + echo 'cannot find patch file: patch-2.6.39'
> cannot find patch file: patch-2.6.39
> + exit 1
> 
> 
> 
> 
> Debug log of patched script (condensed to relevant parts):
> 
> + STOPSUBLEVEL=9999
> + STOPEXTRA=9999
> + '[' 9999 -lt 38 ']'
> + '[' x '!=' x ']'
> + CURRENTFULLVERSION=2.6.38
> + '[' x '!=' x ']'
> + '[' 9999 -gt 38 ']'
> + :
> + CURRENTFULLVERSION=2.6.38
> + EXTRAVER=
> + '[' x = x2.6.38 ']'
> + SUBLEVEL=39
> + FULLVERSION=2.6.39
> 
> 
> Since my last patch (the parent mail) happened to modify the very same line,

Parent email from 2007???  wow.  :)

Why doesn't the debug/trace info show the value of STOPFULLVERSION?
Is it empty?

> I'm slightly wondering why last time it didn't exhibit this issue
> (IIRC this occurred for the first time a couple months later,
> perhaps in newer shell versions).
> 
> Patch has been created on git 2.6.39-rc1,
> run through checkpatch.pl,
> tested to work on dash and still work on bash,
> and tested to apply cleanly to 2.6.37, too.
> 
> IMHO this is a low-priority item,
> certainly not requiring service within current -rc handling,
> and neither at -stable (I assume that only point versions
> could make good use of this patch anyway).
> IOW standard patch circulation.
> I'm assuming that Sam would be the one to queue it up,
> just like last time.

Michal is the new kbuild maintainer, so he could queue it, or I can.

> Thanks!
> 
> Signed-off-by: Andreas Mohr <andi@lisas.de>
> 
> 
> diff --git a/scripts/patch-kernel b/scripts/patch-kernel
> index 46a59ca..20fb25c 100755
> --- a/scripts/patch-kernel
> +++ b/scripts/patch-kernel
> @@ -250,7 +250,7 @@ while :				# incrementing SUBLEVEL (s in v.p.s)
>  do
>      CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
>      EXTRAVER=
> -    if [ $STOPFULLVERSION = $CURRENTFULLVERSION ]; then
> +    if [ x$STOPFULLVERSION = x$CURRENTFULLVERSION ]; then
>          echo "Stopping at $CURRENTFULLVERSION base as requested."
>          break
>      fi

Patch is OK, but I still wonder about the value of $STOPFULLVERSION.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] Re: [PATCH] eradicate bashisms in scripts/patch-kernel

[Andreas Mohr]

Hi,

sorry for the delay. Didn't have a suitable environment at that time,
then it got buried...

On Sun, Apr 03, 2011 at 12:40:04PM -0700, Randy Dunlap wrote:
> On Sun, 3 Apr 2011 20:58:28 +0200 Andreas Mohr wrote:
> > # linux-2.6.38.patch-kernel_test/scripts/patch-kernel linux-2.6.38.patch-kernel_test patch-2.6
> > Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)
> > ===>  linux-2.6.38.patch-kernel_test/scripts/patch-kernel: line 253: [: =: unary operator expected  <===
> > cannot find patch file: patch-2.6.39
> 
> Hi,
> 
> Are you trying to update a 2.6.38 kernel directory to 2.6.39?
> I'm a little confused since the command line does not say anything about 2.6.39.

Well, that virtual setup was just for the sake of having a problem test case.
I should have described it in more detail.
It simply tries to increment to the next patch version, which in case
of 2.6.38 input would be 2.6.39 (which... obviously isn't available yet).

> > Since my last patch (the parent mail) happened to modify the very same line,
> 
> Parent email from 2007???  wow.  :)

Yup - why not simply reply on the original patch?
For suitable tracking, I'd rather have one reference too many than......

Perhaps I should have replied on some ~1995 mail? ;-P

> Why doesn't the debug/trace info show the value of STOPFULLVERSION?
> Is it empty?

Augmented version:

do
    CURRENTFULLVERSION="$VERSION.$PATCHLEVEL.$SUBLEVEL"
    EXTRAVER=
    echo "STOPFULLVERSION is $STOPFULLVERSION"
    if [ $STOPFULLVERSION = $CURRENTFULLVERSION ]; then
        echo "Stopping at $CURRENTFULLVERSION base as requested."
        break
    fi


# linux-2.6.30/scripts/patch-kernel linux-2.6.30 patch-2.6
Current kernel version is 2.6.30 ( Man-Eating Seals of Antiquity)
STOPFULLVERSION is 
[: 276: =: unexpected operator
Applying patch-2.6.31 (bzip2)... done.
STOPFULLVERSION is 
[: 276: =: unexpected operator
Applying patch-2.6.32 (bzip2)... done.
STOPFULLVERSION is 
[: 276: =: unexpected operator
Applying patch-2.6.33 (bzip2)... done.
STOPFULLVERSION is 
[: 276: =: unexpected operator
Applying patch-2.6.34 (bzip2)... done.
STOPFULLVERSION is 
[: 276: =: unexpected operator
cannot find patch file: patch-2.6.35

--> seems so.

> Michal is the new kbuild maintainer, so he could queue it, or I can.

He's been queueing it immediately, thus everything went fine (--> thanks!).

Thanks,

Andreas Mohr

Re: [PATCH] Re: [PATCH] eradicate bashisms in scripts/patch-kernel

[Michal Marek]

On Sun, Apr 03, 2011 at 08:58:28PM +0200, Andreas Mohr wrote:
> Silence a remaining annoying (or worse, irritating - "is my entire patched tree
> broken now!?") bashism-related message that occurs when /bin/sh is configured
> to instead deploy dash, a POSIX-compliant shell, as is the pretty much
> standard case on e.g. Debian.
> 
> # linux-2.6.38.patch-kernel_test/scripts/patch-kernel linux-2.6.38.patch-kernel_test patch-2.6
> Current kernel version is 2.6.38 ( Flesh-Eating Bats with Fangs)
> ===>  linux-2.6.38.patch-kernel_test/scripts/patch-kernel: line 253: [: =: unary operator expected  <===
> cannot find patch file: patch-2.6.39
> 
> Signed-off-by: Andreas Mohr <andi@lisas.de>

Applied to kbuild-2.6.git#misc, thanks.

Michal

Re: [PATCH/RFC] eradicate bashisms in scripts/patch-kernel

[Adrian Bunk]

On Wed, Oct 31, 2007 at 10:13:21PM +0100, Andreas Mohr wrote:

> Hello,

Hi Andreas,

> I was non-mildly horrified to find that the rather widely used patch-kernel
> script seems to rely on bash despite specifying the interpreter as #!/bin/sh,
> since my dash-using Debian install choked on it.
> 
> Thus I'm delivering a first, preliminary, non-reviewed change to make
> patch-kernel (a little bit more?) POSIX-compatible. It now survives both
> a dash and a bash run.
>...
> Comments?
>...

if that's easyly possible it's OK.

But if it becomes possible I'd strongly favour simply changing the 
interpreter to #!/bin/bash which would also fix this problem.

> Thanks,
> 
> Andreas Mohr
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed