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: 3117 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.
I think that the script should be able to assume that root is the first
passed, just like it assumes that the filenames passed to it are correct.
The Nix code that generates the packages file already ensures that.
-- 
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 --]