Re: [PATCH 1/3] scripts/make-erofs.sh: Avoid unneeded calls to awk and chmod

Demi Marie Obenour
2025-09-21T15:56:34Z

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

On 9/21/25 07:31, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
>
>> These calls were made to work around permission problems, but it is much
>> cleaner to solve these problems by making every directory in the new
>> filesystem image writable so that cp can write to it.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>> Hyperfine confirms that this does improve performance, though there are
>> outliers.
>> ---
>> scripts/make-erofs.sh | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
>> index b47048ad747bd7dfcc28e0f1dfd75ec090fa7e09..82d37573ad0454e79becfddd05f93585df7b671c 100755
>> --- a/scripts/make-erofs.sh
>> +++ b/scripts/make-erofs.sh
>> @@ -30,18 +30,18 @@ while read -r arg1; do
>> fi
>> echo
>>
>> - parent="$(dirname "$arg2")"
>> - 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 || :
>> - mkdir -p -- "$root/$parent"
>> + if [ "$arg2" = / ]; then
>> + cp -RT -- "$arg1" "$root"
>> + # Nix store paths are read-only, so fix up permissions
>> + # so that subsequent copies can write to directories
>> + # created by the above copy. This means giving all
>> + # directories 0755 permissions.
>> + find "$root" -type d -exec chmod 0755 -- '{}' +
>> + continue
>> + fi
>
> As I said last time, I don't love that this now only handles the case
> where / is given first, when previously the input didn't have to be in
> any particular order.
>
> Maybe we could get the performance win /and/ preserve the unordered
> property of the input by reimplementing the logic from before in shell
> instead of awk, including testing whether any directories actually
> needed to be modified before invoking chmod using [ (probably a shell
> built-in)?

That's always going to be true right now, though, and I can easily add
a check and bail out if it turns out to be false. I don't want extra
complexity.

In the long term this needs to be rewritten in C or Python anyway.

>>
>> + parent=$(dirname "$arg2")
>> + mkdir -p -- "$root/$parent"
>> cp -RT -- "$arg1" "$root/$arg2"
>> done

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