Re: [PATCH 1/2] scripts/make-erofs.sh: run chmod less

[Demi Marie Obenour <demiobenour@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2550 bytes --]

On 9/30/25 05:19, Alyssa Ross wrote:
> The common case here is that we don't have to change any permissions,
> so rewrite it so that (assuming [ is a shell builtin) we don't need to
> run any additional programs unless we actually need to make a
> permissions change.
> 
> I also find this a bit more readable than the previous awk version,
> and we no longer need to ignore errors from chmod.
> 
> Link: https://spectrum-os.org/lists/archives/spectrum-devel/87plbj4w9q.fsf@alyssa.is
> Signed-off-by: Alyssa Ross <hi@alyssa.is>
> ---
>  scripts/make-erofs.sh | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
> index be65de8..551ae4a 100755
> --- a/scripts/make-erofs.sh
> +++ b/scripts/make-erofs.sh
> @@ -1,6 +1,6 @@
>  #!/bin/sh -eu
>  #
> -# SPDX-FileCopyrightText: 2023-2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2023-2025 Alyssa Ross <hi@alyssa.is>
>  # SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
>  # SPDX-License-Identifier: EUPL-1.2+
>  #
> @@ -39,15 +39,28 @@ while read -r arg1; do
>  	(*) parent=.;;
>  	esac
>  
> -	awk -v parent="$parent" -v root="$root" 'BEGIN {
> -		n = split(parent, components, "/")
> -		for (i = 1; i <= n; i++) {
> -			printf "%s/", root
> -			for (j = 1; j <= i; j++)
> -				printf "%s/", components[j]
> -			print
> -		}
> -	}' | xargs -rd '\n' chmod +w -- 2>/dev/null || :
> +	# Ensure any existing directories we want to write into are writable.
> +	(
> +		set --
> +		while :; do
> +			abs="$root/$parent"
> +			if [ -e "$abs" ] && ! [ -w "$abs" ]; then
> +				set -- "$abs" "$@"
> +			fi
> +
> +			# shellcheck disable=SC2030 # shadowed on purpose
> +			case "$parent" in
> +				*/*) parent="${parent%/*}" ;;
> +				*) break ;;
> +			esac
> +		done
> +
> +		if [ "$#" -ne 0 ]; then
> +			chmod +w -- "$@"
> +		fi
> +	)
> +
> +	# shellcheck disable=SC2031 # shadowed in subshell on purpose
>  	mkdir -p -- "$root/$parent"
>  
>  	cp -RT -- "$arg1" "$root/$arg2"

It's probably possible to do something with IFS=/ to avoid the quadratic
runtime on deeply nested filesystem trees.  It is also possible to use
the exec builtin to save a fork.  That said, the constant factor
is low enough that this should not be an issue, and this is an improvement
over the current situation.

Reviewed-by: Demi Marie Obenour <demiobenour@gmail.com>
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]