Re: linux/scripts/patch-kernel

Nick Holloway (Nick.Holloway@alfie.demon.co.uk)
9 Mar 1996 19:04:31 -0000


kaos@audio.apana.org.au (Keith Owens) writes:
> scripts/patch-kernel was itself patched at 1.3.69, alas it's wrong. I've
> mailed a patch to the maintainer, in the meantime edit scripts/patch-kernel,
> replace
>
> if (gunzip -dc ...
> if ! (gunzip -dc ...

Although I am the original author to patch-kernel, and my name is on
the top, I have had nothing to do with the changes that have been made
since :-(

The change to look for .*.rej as well as *.rej was a good one (also
*.orig), and one I would have sent to Linus anyway.

Another change was to remove the flag '-f'. I thought '-f' meant
disable assuming '-R', but actually enables fuzzy patching. Mea culpa.
Another good modification. I suggest '-N' be added.

The change that introduced the error above was to check the status of
a pipeline in a subshell, where the pipeline includes a grep to remove
part of the patch! Why? [Aside: there is no need for the subshell,
the status of a pipeline is the status of the last process in a pipeline].

The original rationale for the script is feed the Linus produced (hence
quality) patch into the patch program. Arguments were provided to keep
it on the straight and narrow. The benefit of checking the exit status
of patch is that the fs becoming full is also caught as an error.

There is no need to pipe the output through grep, as there are no
extraneous lines in the patches I've seen (and I read them _all_).

I also consider the fix above to be erroneous, as '!' is not valid in a
shell script, it is known to bash, and the script starts '#!/bin/sh'.
[Aside: I notice that it isn't excutable in the distribution tar].
The portable way to do this is "if ... | patch; then :; else abort; fi".

Actually, I notice Linus has already used this format in 1.3.72.

As a result of writing this, here are some small changes that I think
should be made.

* Remove the subshell.
* Remove the grep, unless somebody can convince me that it is
absolutely necessary (i.e affects the exit status of patch).
* Add the flag to only apply forward patches.
* Some cosmetics on the messages.

--- patch-kernel~ Sat Mar 9 18:49:27 1996
+++ patch-kernel Sat Mar 9 19:01:40 1996
@@ -36,11 +36,11 @@
fi

echo -n "Applying $patch... "
- if (gunzip -dc $patchdir/$patch | grep -v '^\\' | patch -p1 -s -E -d $sourcedir)
+ if gunzip -dc $patchdir/$patch | patch -p1 -s -N -E -d $sourcedir
then
- echo "done"
+ echo "done."
else
- echo "Patch failed. Clean up yourself."
+ echo "failed. Clean up yourself."
break
fi
if [ "`find $sourcedir '(' -name '*.rej' -o -name '.*.rej' ')' -print`" ]

-- 
 `O O'  | Home: Nick.Holloway@alfie.demon.co.uk
// ^ \\ | Work: Nick.Holloway@parallax.co.uk  http://www.parallax.co.uk/~alfie/