Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules

[Josh Poimboeuf]

On Wed, Jun 11, 2025 at 05:44:23PM -0400, Joe Lawrence wrote:
> On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> > +revert_patches() {
> > +	local extra_args=("$@")
> > +	local patches=("${APPLIED_PATCHES[@]}")
> > +
> > +	for (( i=${#patches[@]}-1 ; i>=0 ; i-- )) ; do
> > +		revert_patch "${patches[$i]}" "${extra_args[@]}"
> > +	done
> > +
> > +	APPLIED_PATCHES=()
> > +
> > +	# Make sure git actually sees the patches have been reverted.
> > +	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
> > +}
> 
> < warning: testing entropy field fully engaged :D >
> 
> Minor usability nit: I had run `klp-build --help` while inside a VM that
> didn't seem to have r/w access to .git/.  Since the cleanup code is
> called unconditionally, it gave me a strange error when running this
> `git update-index` when I never supplied any patches to operate on. I
> just wanted to see the usage.
> 
> Could this git update-index be made conditional?
> 
>   if (( ${#APPLIED_PATCHES[@]} > 0 )); then
>       ([[ -d "$SRC/.git" ]] && cd "$SRC" && git update-index -q --refresh)
>       APPLIED_PATCHES=()
>   fi
> 
> Another way to find yourself in this function is to move .git/ out of
> the way.  In that case, since it's the last line of revert_patches(), I
> think the failure of [[ -d "$SRC/.git" ]] causes the script to
> immediately exit:
> 
>   - for foo.patch, at the validate_patches() -> revert_patches() call
>   - for --help, at the cleanup() -> revert_patches() call
> 
> So if you don't like the conditional change above, should
> revert_patches() end with `true` to eat the [[ -d "$SRC/.git" ]] status?
>  Or does that interfere with other calls to that function throughout the
> script?
> 
> FWIW, with either adjustment, the script seems happy to operate on a
> plain ol' kernel source tree without git.

Hm, revert_patch() already has a call to git_refresh(), so there doesn't
appear to be any point to that extra refresh in revert_patches().

Does this fix?

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 7ec07c4079f7..e6dbac89ab03 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -388,9 +388,6 @@ revert_patches() {
 	done
 
 	APPLIED_PATCHES=()
-
-	# Make sure git actually sees the patches have been reverted.
-	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
 }
 
 validate_patches() {