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.
--
Joe
