[PATCH 0/2] scripts/make-erofs.sh: run mkdir/chmod less
This is an alternative to Demi's proposal[1] to speed this up. This implementation has the advantage that it does not impose an ordering requirement on the inputs. Nevertheless, it runs just as fast, if not faster (first commit is main; second is Demi's patch; third is this series): | Command | Mean [s] | Min [s] | Max [s] | Relative | |---------------------------+----------------+---------+---------+-------------| | =make (commit = 2551f9e)= | 13.275 ± 0.853 | 11.881 | 16.074 | 1.18 ± 0.11 | | =make (commit = 28fc640)= | 11.467 ± 0.729 | 10.063 | 14.588 | 1.02 ± 0.09 | | =make (commit = 6c3d020)= | 11.215 ± 0.720 | 9.947 | 15.616 | 1.00 | Link: https://spectrum-os.org/lists/archives/spectrum-devel/20250919-less-dirname-... [1] Alyssa Ross (2): scripts/make-erofs.sh: run chmod less scripts/make-erofs.sh: run mkdir less scripts/make-erofs.sh | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) base-commit: 2551f9eb1a6c9245699ff5cf77f9957d1e2d14be -- 2.51.0
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.... 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" -- 2.51.0
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.... 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)
This patch has been committed as f7542419686ec6d795810ca497fcd36164848b78, which can be viewed online at https://spectrum-os.org/git/spectrum/commit/?id=f7542419686ec6d795810ca497fc.... This is an automated message. Send comments/questions/requests to: Alyssa Ross <hi@alyssa.is>
[ is likely a shell builtin, so by checking for existence before running mkdir, we avoid lots of mkdir spawns. Signed-off-by: Alyssa Ross <hi@alyssa.is> --- scripts/make-erofs.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh index 551ae4a..ad04844 100755 --- a/scripts/make-erofs.sh +++ b/scripts/make-erofs.sh @@ -61,7 +61,9 @@ while read -r arg1; do ) # shellcheck disable=SC2031 # shadowed in subshell on purpose - mkdir -p -- "$root/$parent" + if ! [ -e "$root/$parent" ]; then + mkdir -p -- "$root/$parent" + fi cp -RT -- "$arg1" "$root/$arg2" done -- 2.51.0
On 9/30/25 05:19, Alyssa Ross wrote:
[ is likely a shell builtin, so by checking for existence before running mkdir, we avoid lots of mkdir spawns.
Signed-off-by: Alyssa Ross <hi@alyssa.is> --- scripts/make-erofs.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh index 551ae4a..ad04844 100755 --- a/scripts/make-erofs.sh +++ b/scripts/make-erofs.sh @@ -61,7 +61,9 @@ while read -r arg1; do )
# shellcheck disable=SC2031 # shadowed in subshell on purpose - mkdir -p -- "$root/$parent" + if ! [ -e "$root/$parent" ]; then + mkdir -p -- "$root/$parent" + fi
cp -RT -- "$arg1" "$root/$arg2" done
Reviewed-by: Demi Marie Obenour <demiobenour@gmail.com> -- Sincerely, Demi Marie Obenour (she/her/hers)
This patch has been committed as fecb4c6822a6200d0046155478e0599edf7010f0, which can be viewed online at https://spectrum-os.org/git/spectrum/commit/?id=fecb4c6822a6200d0046155478e0.... This is an automated message. Send comments/questions/requests to: Alyssa Ross <hi@alyssa.is>
participants (3)
-
Alyssa Ross -
Alyssa Ross -
Demi Marie Obenour