On Tue, Feb 03, 2026 at 09:51:38PM -0500, Joe Lawrence wrote:
> I think this does simplify things, but:
>
> - introduces a dependency on patchutil's recountdiff
I was wondering if we could instead just update fix-patch-lines to fix
the line numbers/counts, but I get the feeling that would require
rewriting the whole script and may not be worth the complexity. That
script is nice and simple and robust at the moment.
> - requires goofy epoch timestamp filtering as `diff -N` doesn't use
> the `git diff` /dev/null, but a localized beginining of time epoch
> that may be 1969 or 1970 depending on the local timezone
> - can be *really* chatty, for example:
>
> Validating patch(es)
> patching file fs/proc/cmdline.c
> Hunk #1 succeeded at 7 (offset 1 line).
> Fixing patch(es)
> patching file fs/proc/cmdline.c
> Hunk #1 succeeded at 7 (offset 1 line).
> patching file fs/proc/cmdline.c
> patching file fs/proc/cmdline.c
> Building patched kernel
> Copying patched object files
> Diffing objects
> vmlinux.o: changed function: override_release
> vmlinux.o: changed function: cmdline_proc_show
> Building patch module: livepatch-cmdline-string.ko
> SUCCESS
>
> My initial thought was that I'd only be interested in knowing about
> patch offset/fuzz during the validation phase. And in the interest of
> clarifying some of the output messages, it would be nice to know the
> patch it was referring to, so how about a follow up patch
> pretty-formatting with some indentation like:
>
> Validating patch(es)
> cmdline-string.patch
> patching file fs/proc/cmdline.c
> Hunk #1 succeeded at 7 (offset 1 line).
> Fixing patch(es)
> Building patched kernel
> Copying patched object files
> Diffing objects
> vmlinux.o: changed function: override_release
> vmlinux.o: changed function: cmdline_proc_show
> Building patch module: livepatch-cmdline-string.ko
> SUCCESS
>
> That said, Song suggested using --silent across the board, so maybe
> tie that into the existing --verbose option?
Hm. Currently we go to considerable effort to make klp-build's output
as concise as possible, which is good. On the other hand, it might be
important to know the patch has fuzz.
I'm thinking I would agree that maybe it should be verbose when
validating patches and silent elsewhere. And the pretty formatting is a
nice upgrade to that.
We might also consider indenting the outputs of the other steps. For
example:
Building patched kernel
vmlinux.o: some warning
Copying patched object files
Diffing objects
vmlinux.o: changed function: override_release
vmlinux.o: changed function: cmdline_proc_show
Or alternatively, print the step names in ASCII bold or something.
> apply_patch() {
> local patch="$1"
> - shift
> - local extra_args=("$@")
>
> [[ ! -f "$patch" ]] && die "$patch doesn't exist"
>
> (
> cd "$SRC"
> -
> - # The sed strips the version signature from 'git format-patch',
> - # otherwise 'git apply --recount' warns.
> - sed -n '/^-- /q;p' "$patch" |
> - git apply "${extra_args[@]}"
> + # The sed strips the version signature from 'git format-patch'.
> + sed -n '/^-- /q;p' "$patch" | \
> + patch -p1 --no-backup-if-mismatch -r /dev/null
Is this still needed now that we don't use git apply --recount?
> @@ -490,7 +468,7 @@ fix_patches() {
>
> cp -f "$old_patch" "$tmp_patch"
> refresh_patch "$tmp_patch"
> - "$FIX_PATCH_LINES" "$tmp_patch" > "$new_patch"
> + "$FIX_PATCH_LINES" "$tmp_patch" | recountdiff > "$new_patch"
> refresh_patch "$new_patch"
Do we still need to refresh after the recountdiff?
--
Josh
