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

Demi Marie Obenour
2025-09-30T16:01:07Z

[-- 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 --]