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

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

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