[PATCH 0/3] Optimize scripts/make-erofs.sh

[PATCH 0/3] Optimize scripts/make-erofs.sh

[Demi Marie Obenour]

In the future this will hopefully be replaced by mkfs.erofs improvements
or a C program that calls the erofs library.  However, optimizing the
script is much simpler for now.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
Demi Marie Obenour (3):
      scripts/make-erofs.sh: Avoid unneeded calls to awk and chmod
      scripts/make-erofs.sh: Avoid calls to dirname
      scripts/make-erofs.sh: Avoid unneeded calls to mkdir

 scripts/make-erofs.sh | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)
---
base-commit: 15ca6c4684313fcc9fcde3bda97d64698bb267ea
change-id: 20250919-less-dirname-2218643e73a6
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

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

[Demi Marie Obenour]

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
 
+	parent=$(dirname "$arg2")
+	mkdir -p -- "$root/$parent"
 	cp -RT -- "$arg1" "$root/$arg2"
 done
 

-- 
2.51.0

[PATCH 2/3] scripts/make-erofs.sh: Avoid calls to dirname

[Demi Marie Obenour]

Use builtin string manipulation instead.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 scripts/make-erofs.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
index 82d37573ad0454e79becfddd05f93585df7b671c..ac62a65f53e0b6417b164f475a537960fc5203bc 100755
--- a/scripts/make-erofs.sh
+++ b/scripts/make-erofs.sh
@@ -40,7 +40,13 @@ while read -r arg1; do
 		continue
 	fi
 
-	parent=$(dirname "$arg2")
+	# The below simple version of dirname(1) can only handle
+	# a subset of all paths, but this subset includes all of
+	# the ones passed in practice other than /.
+	case $arg2 in
+	(*/*) parent=${arg2%/*};;
+	(*) parent=.;;
+	esac
 	mkdir -p -- "$root/$parent"
 	cp -RT -- "$arg1" "$root/$arg2"
 done

-- 
2.51.0

[PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Demi Marie Obenour]

Don't call it if the target directory already exists.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
 scripts/make-erofs.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
--- a/scripts/make-erofs.sh
+++ b/scripts/make-erofs.sh
@@ -44,10 +44,15 @@ while read -r arg1; do
 	# a subset of all paths, but this subset includes all of
 	# the ones passed in practice other than /.
 	case $arg2 in
-	(*/*) parent=${arg2%/*};;
-	(*) parent=.;;
+	(*/*)
+		# Make the parent directory if needed
+		parent=$root/${arg2%/*}
+		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
+		;;
+	(*)
+		# Parent $root which definitely exists
+		;;
 	esac
-	mkdir -p -- "$root/$parent"
 	cp -RT -- "$arg1" "$root/$arg2"
 done
 

-- 
2.51.0

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

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

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)?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 2/3] scripts/make-erofs.sh: Avoid calls to dirname

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> Use builtin string manipulation instead.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>

With a copyright header added:

Reviewed-by: Alyssa Ross <hi@alyssa.is>

> ---
>  scripts/make-erofs.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
> index 82d37573ad0454e79becfddd05f93585df7b671c..ac62a65f53e0b6417b164f475a537960fc5203bc 100755
> --- a/scripts/make-erofs.sh
> +++ b/scripts/make-erofs.sh
> @@ -40,7 +40,13 @@ while read -r arg1; do
>  		continue
>  	fi
>  
> -	parent=$(dirname "$arg2")
> +	# The below simple version of dirname(1) can only handle
> +	# a subset of all paths, but this subset includes all of
> +	# the ones passed in practice other than /.
> +	case $arg2 in
> +	(*/*) parent=${arg2%/*};;
> +	(*) parent=.;;
> +	esac
>  	mkdir -p -- "$root/$parent"
>  	cp -RT -- "$arg1" "$root/$arg2"
>  done
>
> -- 
> 2.51.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

[Demi Marie Obenour]

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

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

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

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 would love an implementation of this that didn't require the copies
at all, of course.)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

[Demi Marie Obenour]

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

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

[Demi Marie Obenour]

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

Re: [PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> Don't call it if the target directory already exists.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  scripts/make-erofs.sh | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
> index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
> --- a/scripts/make-erofs.sh
> +++ b/scripts/make-erofs.sh
> @@ -44,10 +44,15 @@ while read -r arg1; do
>  	# a subset of all paths, but this subset includes all of
>  	# the ones passed in practice other than /.
>  	case $arg2 in
> -	(*/*) parent=${arg2%/*};;
> -	(*) parent=.;;
> +	(*/*)
> +		# Make the parent directory if needed
> +		parent=$root/${arg2%/*}
> +		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
> +		;;
> +	(*)
> +		# Parent $root which definitely exists
> +		;;
>  	esac
> -	mkdir -p -- "$root/$parent"
>  	cp -RT -- "$arg1" "$root/$arg2"
>  done

I saw a statistically significant speedup in hyperfine. :)

Reviewed-by: Alyssa Ross <hi@alyssa.is>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Demi Marie Obenour]

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

On 9/21/25 14:36, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> Don't call it if the target directory already exists.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>>  scripts/make-erofs.sh | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
>> index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
>> --- a/scripts/make-erofs.sh
>> +++ b/scripts/make-erofs.sh
>> @@ -44,10 +44,15 @@ while read -r arg1; do
>>  	# a subset of all paths, but this subset includes all of
>>  	# the ones passed in practice other than /.
>>  	case $arg2 in
>> -	(*/*) parent=${arg2%/*};;
>> -	(*) parent=.;;
>> +	(*/*)
>> +		# Make the parent directory if needed
>> +		parent=$root/${arg2%/*}
>> +		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
>> +		;;
>> +	(*)
>> +		# Parent $root which definitely exists
>> +		;;
>>  	esac
>> -	mkdir -p -- "$root/$parent"
>>  	cp -RT -- "$arg1" "$root/$arg2"
>>  done
> 
> I saw a statistically significant speedup in hyperfine. :)
> 
> Reviewed-by: Alyssa Ross <hi@alyssa.is>

Yay!  This is independent of the rest of the series, so
feel free to commit.
-- 
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 --]

Re: [PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 9/21/25 14:36, Alyssa Ross wrote:
>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>> 
>>> Don't call it if the target directory already exists.
>>>
>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>> ---
>>>  scripts/make-erofs.sh | 11 ++++++++---
>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
>>> index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
>>> --- a/scripts/make-erofs.sh
>>> +++ b/scripts/make-erofs.sh
>>> @@ -44,10 +44,15 @@ while read -r arg1; do
>>>  	# a subset of all paths, but this subset includes all of
>>>  	# the ones passed in practice other than /.
>>>  	case $arg2 in
>>> -	(*/*) parent=${arg2%/*};;
>>> -	(*) parent=.;;
>>> +	(*/*)
>>> +		# Make the parent directory if needed
>>> +		parent=$root/${arg2%/*}
>>> +		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
>>> +		;;
>>> +	(*)
>>> +		# Parent $root which definitely exists
>>> +		;;
>>>  	esac
>>> -	mkdir -p -- "$root/$parent"
>>>  	cp -RT -- "$arg1" "$root/$arg2"
>>>  done
>> 
>> I saw a statistically significant speedup in hyperfine. :)
>> 
>> Reviewed-by: Alyssa Ross <hi@alyssa.is>
>
> Yay!  This is independent of the rest of the series, so
> feel free to commit.

It does still need the copyright header I mentioned for patch 2.  I can
apply both with your normal one if that's okay with you.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Demi Marie Obenour]

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

On 9/23/25 10:35, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> On 9/21/25 14:36, Alyssa Ross wrote:
>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>
>>>> Don't call it if the target directory already exists.
>>>>
>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>>> ---
>>>>  scripts/make-erofs.sh | 11 ++++++++---
>>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
>>>> index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
>>>> --- a/scripts/make-erofs.sh
>>>> +++ b/scripts/make-erofs.sh
>>>> @@ -44,10 +44,15 @@ while read -r arg1; do
>>>>  	# a subset of all paths, but this subset includes all of
>>>>  	# the ones passed in practice other than /.
>>>>  	case $arg2 in
>>>> -	(*/*) parent=${arg2%/*};;
>>>> -	(*) parent=.;;
>>>> +	(*/*)
>>>> +		# Make the parent directory if needed
>>>> +		parent=$root/${arg2%/*}
>>>> +		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
>>>> +		;;
>>>> +	(*)
>>>> +		# Parent $root which definitely exists
>>>> +		;;
>>>>  	esac
>>>> -	mkdir -p -- "$root/$parent"
>>>>  	cp -RT -- "$arg1" "$root/$arg2"
>>>>  done
>>>
>>> I saw a statistically significant speedup in hyperfine. :)
>>>
>>> Reviewed-by: Alyssa Ross <hi@alyssa.is>
>>
>> Yay!  This is independent of the rest of the series, so
>> feel free to commit.
> 
> It does still need the copyright header I mentioned for patch 2.  I can
> apply both with your normal one if that's okay with you.

It is.  Thanks!
-- 
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 --]

Re: [PATCH 2/3] scripts/make-erofs.sh: Avoid calls to dirname

[Alyssa Ross]

This patch has been committed as 0fcf508e884944a2875fb52dbf58a977aa5df6e8,
which can be viewed online at
https://spectrum-os.org/git/spectrum/commit/?id=0fcf508e884944a2875fb52dbf58a977aa5df6e8.

This is an automated message.  Send comments/questions/requests to:
Alyssa Ross <hi@alyssa.is>

Re: [PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 9/23/25 10:35, Alyssa Ross wrote:
>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>> 
>>> On 9/21/25 14:36, Alyssa Ross wrote:
>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>
>>>>> Don't call it if the target directory already exists.
>>>>>
>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>>>> ---
>>>>>  scripts/make-erofs.sh | 11 ++++++++---
>>>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
>>>>> index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
>>>>> --- a/scripts/make-erofs.sh
>>>>> +++ b/scripts/make-erofs.sh
>>>>> @@ -44,10 +44,15 @@ while read -r arg1; do
>>>>>  	# a subset of all paths, but this subset includes all of
>>>>>  	# the ones passed in practice other than /.
>>>>>  	case $arg2 in
>>>>> -	(*/*) parent=${arg2%/*};;
>>>>> -	(*) parent=.;;
>>>>> +	(*/*)
>>>>> +		# Make the parent directory if needed
>>>>> +		parent=$root/${arg2%/*}
>>>>> +		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
>>>>> +		;;
>>>>> +	(*)
>>>>> +		# Parent $root which definitely exists
>>>>> +		;;
>>>>>  	esac
>>>>> -	mkdir -p -- "$root/$parent"
>>>>>  	cp -RT -- "$arg1" "$root/$arg2"
>>>>>  done
>>>>
>>>> I saw a statistically significant speedup in hyperfine. :)
>>>>
>>>> Reviewed-by: Alyssa Ross <hi@alyssa.is>
>>>
>>> Yay!  This is independent of the rest of the series, so
>>> feel free to commit.

It actually doesn't seem to be.  I get a "Permission denied" build
failure when I apply it on top of 0fcf508 ("scripts/make-erofs.sh: Avoid
calls to dirname"), but I think I didn't see that failure when I applied
patch 1 as well.  We can either just keep this in the queue until we've
figured out what to do about patch 1, or you can investigate and
resubmit a standalone version of this patch.  Up to you.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 3/3] scripts/make-erofs.sh: Avoid unneeded calls to mkdir

[Demi Marie Obenour]

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

On 9/23/25 14:56, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> On 9/23/25 10:35, Alyssa Ross wrote:
>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>
>>>> On 9/21/25 14:36, Alyssa Ross wrote:
>>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>>
>>>>>> Don't call it if the target directory already exists.
>>>>>>
>>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>>>>> ---
>>>>>>  scripts/make-erofs.sh | 11 ++++++++---
>>>>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh
>>>>>> index ac62a65f53e0b6417b164f475a537960fc5203bc..30b83b0b41cbe7bc4fd7786cfcdddcf10c78cc5a 100755
>>>>>> --- a/scripts/make-erofs.sh
>>>>>> +++ b/scripts/make-erofs.sh
>>>>>> @@ -44,10 +44,15 @@ while read -r arg1; do
>>>>>>  	# a subset of all paths, but this subset includes all of
>>>>>>  	# the ones passed in practice other than /.
>>>>>>  	case $arg2 in
>>>>>> -	(*/*) parent=${arg2%/*};;
>>>>>> -	(*) parent=.;;
>>>>>> +	(*/*)
>>>>>> +		# Make the parent directory if needed
>>>>>> +		parent=$root/${arg2%/*}
>>>>>> +		if [ ! -d "$parent" ]; then mkdir -p -- "$parent"; fi
>>>>>> +		;;
>>>>>> +	(*)
>>>>>> +		# Parent $root which definitely exists
>>>>>> +		;;
>>>>>>  	esac
>>>>>> -	mkdir -p -- "$root/$parent"
>>>>>>  	cp -RT -- "$arg1" "$root/$arg2"
>>>>>>  done
>>>>>
>>>>> I saw a statistically significant speedup in hyperfine. :)
>>>>>
>>>>> Reviewed-by: Alyssa Ross <hi@alyssa.is>
>>>>
>>>> Yay!  This is independent of the rest of the series, so
>>>> feel free to commit.
> 
> It actually doesn't seem to be.  I get a "Permission denied" build
> failure when I apply it on top of 0fcf508 ("scripts/make-erofs.sh: Avoid
> calls to dirname"), but I think I didn't see that failure when I applied
> patch 1 as well.  We can either just keep this in the queue until we've
> figured out what to do about patch 1, or you can investigate and
> resubmit a standalone version of this patch.  Up to you.

I think it is best to include patch 1.  It speeds things up, and
I consider the complexity to not be a serious problem.  The code
that generates the file lists already guarantees this.
-- 
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 --]