[PATCH next] prepare-kernel.sh: Tame link mode cleanup code

[PATCH next] prepare-kernel.sh: Tame link mode cleanup code

[Richard Weinberger]

Reverse mode introduced an odd regression, in link mode suddenly some
links vanished.
The root cause is that after a further refactoring I have changed
the sequence of patch_link() calls.

A sequence of:
patch_link r n include/rtdm/uapi include/xenomai/rtdm/uapi
patch_link r n include/cobalt/kernel/rtdm include/xenomai/rtdm

caused the second patch_link() call to remove all links in
include/xenomai/rtdm/uapi because of the cleanup code.
The existing cleanup code tries to delete stale links from a previous run
of prepare-kernel.sh.
It removes all links below include/xenomai/rtdm/ which don't exist
in the Xenomai include/cobalt/kernel/rtdm/ directory.
So, patch_link() implicitly assumed ascending order of calls,
the new reverse mode feature explicitly needs descending call order.

Digging deeper unearthed further odds in the cleanup code.
Originally it used to rm -Rf stale folders and files, but after commit
e72d695a7 ("scripts/prepare-kernel: only purge links from target directories")
in 2010 code tested whether the target file is a symlink.
So $dir_opt and rm'ing recursively is not needed anymore.

In order to fix the regression, change the removal test to remove
symlinks only if they are broken.
e.g. when they point to a moved/old Xenomai installation.
Also run the cleanup code only when not in reverse mode.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 scripts/prepare-kernel.sh | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/prepare-kernel.sh b/scripts/prepare-kernel.sh
index 5108737ea..0afb812a4 100755
--- a/scripts/prepare-kernel.sh
+++ b/scripts/prepare-kernel.sh
@@ -57,18 +57,16 @@ patch_link() {
 		fi
 		if test x$recursive = xr; then
 			recursive_opt="-mindepth 1"
-			dir_opt="-type d -o"
 		else
 			recursive_opt="-maxdepth 1"
-			dir_opt=""
 		fi
-		find_clean_opt="$recursive_opt \( $dir_opt $link_makefiles_opt -name Kconfig -o -name '*.[chS]' -o -name '*.sh' \)"
+		find_clean_opt="$recursive_opt \( $link_makefiles_opt -name Kconfig -o -name '*.[chS]' -o -name '*.sh' \)"
 		find_link_opt="$recursive_opt \( $link_makefiles_opt -name Kconfig -o -name '*.[chS]' -o -name '*.sh' \)"
 	fi
 
-	if test "x$output_patch" = "x" -a -e $linux_tree/$link_dir; then
+	if test "x$output_patch" = "x" -a "x$reverse" = "x0" -a -e $linux_tree/$link_dir; then
 		cd $linux_tree/$link_dir && eval find . $find_clean_opt | while read f; do
-			if test -L $f -a ! -e $xenomai_root/$target_dir/$f; then rm -Rf $f; fi
+			if test -L $f -a ! -e $f; then rm -f $f; fi
 		done
 	fi
 
-- 
2.35.3

Re: [PATCH next] prepare-kernel.sh: Tame link mode cleanup code

[Jan Kiszka]

On 25.06.24 22:38, Richard Weinberger wrote:
> Reverse mode introduced an odd regression, in link mode suddenly some
> links vanished.
> The root cause is that after a further refactoring I have changed
> the sequence of patch_link() calls.
> 
> A sequence of:
> patch_link r n include/rtdm/uapi include/xenomai/rtdm/uapi
> patch_link r n include/cobalt/kernel/rtdm include/xenomai/rtdm
> 
> caused the second patch_link() call to remove all links in
> include/xenomai/rtdm/uapi because of the cleanup code.
> The existing cleanup code tries to delete stale links from a previous run
> of prepare-kernel.sh.
> It removes all links below include/xenomai/rtdm/ which don't exist
> in the Xenomai include/cobalt/kernel/rtdm/ directory.
> So, patch_link() implicitly assumed ascending order of calls,
> the new reverse mode feature explicitly needs descending call order.
> 
> Digging deeper unearthed further odds in the cleanup code.
> Originally it used to rm -Rf stale folders and files, but after commit
> e72d695a7 ("scripts/prepare-kernel: only purge links from target directories")
> in 2010 code tested whether the target file is a symlink.
> So $dir_opt and rm'ing recursively is not needed anymore.
> 
> In order to fix the regression, change the removal test to remove
> symlinks only if they are broken.
> e.g. when they point to a moved/old Xenomai installation.
> Also run the cleanup code only when not in reverse mode.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  scripts/prepare-kernel.sh | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/prepare-kernel.sh b/scripts/prepare-kernel.sh
> index 5108737ea..0afb812a4 100755
> --- a/scripts/prepare-kernel.sh
> +++ b/scripts/prepare-kernel.sh
> @@ -57,18 +57,16 @@ patch_link() {
>  		fi
>  		if test x$recursive = xr; then
>  			recursive_opt="-mindepth 1"
> -			dir_opt="-type d -o"
>  		else
>  			recursive_opt="-maxdepth 1"
> -			dir_opt=""
>  		fi
> -		find_clean_opt="$recursive_opt \( $dir_opt $link_makefiles_opt -name Kconfig -o -name '*.[chS]' -o -name '*.sh' \)"
> +		find_clean_opt="$recursive_opt \( $link_makefiles_opt -name Kconfig -o -name '*.[chS]' -o -name '*.sh' \)"
>  		find_link_opt="$recursive_opt \( $link_makefiles_opt -name Kconfig -o -name '*.[chS]' -o -name '*.sh' \)"
>  	fi
>  
> -	if test "x$output_patch" = "x" -a -e $linux_tree/$link_dir; then
> +	if test "x$output_patch" = "x" -a "x$reverse" = "x0" -a -e $linux_tree/$link_dir; then
>  		cd $linux_tree/$link_dir && eval find . $find_clean_opt | while read f; do
> -			if test -L $f -a ! -e $xenomai_root/$target_dir/$f; then rm -Rf $f; fi
> +			if test -L $f -a ! -e $f; then rm -f $f; fi
>  		done
>  	fi
>  

This is still fixing up patch 2 of your series, right? Could you rather
send a new version of that patch so that I can fold that in, avoiding
any bisection issues? Please make sure to pick up the version we have in
'next' right now.

Thanks,
Jan

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH next] prepare-kernel.sh: Tame link mode cleanup code

[Richard Weinberger]

Am Dienstag, 25. Juni 2024, 23:19:24 CEST schrieb 'Jan Kiszka' via upstream:
> This is still fixing up patch 2 of your series, right?

Kind of. It fixes existing code which became a problem after patch 2.
So, this patch can get inserted before patch 2, modulo the $reverse check.

So, I'll send you two patches that can be inserted after patch 1 (prepare-kernel.sh: Convert to tabs)
which will replace patch 2 (prepare-kernel.sh: Add reverse mode).

Thanks,
//richard

-- 
​​​​​sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
UID/VAT Nr: ATU 66964118 | FN: 374287y