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

Demi Marie Obenour
2025-09-21T17:13:04Z

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

On 9/21/25 12:15, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
>
>> 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.
>
> In my opinion, adding an ordering requirement when we can do fine
> without increases complexity, and rewriting what we currently have in
> shell doesn't. I can try that myself if you don't want to, and I'm also
> happy to take the following two patches without blocking them on this.

That would be awesome.

> (I would love an implementation of this that didn't require the copies
> at all, of course.)

Me too.
--
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 --]