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...
[ message continues ]

if that's easyly possible it's OK.
But if it becomes possible I'd strongly favour simply changing the 
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
-

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
Why are the added spaces needed?
Did you look at
--

~Randy

-

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"
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
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
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).
No. I should have started by looking at the opengroup.org site when
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

-

As I indicated, I don't object to this part of the patch or to
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

-

Hi,
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#...

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, SUBLEV...
[ message continues ]

On Thu, 1 Nov 2007 23:16:06 +0100 Andreas Mohr wrote:
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
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?
Thanks,

---

~Randy

-

This is POSIX-compliant and has worked with dash from the very
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

-

Hi,
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
"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
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

-

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

-

Hi,
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
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

-

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 [ $stopve...
[ message continues ]

Hi,
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" ]

                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

-

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

-

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 [...
[ message continues ]

The use of $(($x)) should not be needed with latest dash....
---

~Randy

desserts:  http://www.xenotime.net/linux/recipes/

--

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

-