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