mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4] MAKEALL: sanity check command-line options more
@ 2026-02-23 11:31 Ahmad Fatoum
  2026-02-23 11:31 ` [PATCH 1/4] MAKEALL: early exit if config specified, but no arch Ahmad Fatoum
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:31 UTC (permalink / raw)
  To: barebox

I watched how Claude Code stumbles to use MAKEALL correctly for a while
and indeed MAKEALL could really use some improvement in user experience.

This series adds some sanity checks to make it more difficult to misuse.

Ahmad Fatoum (4):
  MAKEALL: early exit if config specified, but no arch
  MAKEALL: devel mode: abort if no positional config
  MAKEALL: error out on options placed after the defconfig argument
  MAKEALL: flock build directory to prevent concurrent builds

 MAKEALL | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

-- 
2.47.3




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] MAKEALL: early exit if config specified, but no arch
  2026-02-23 11:31 [PATCH 0/4] MAKEALL: sanity check command-line options more Ahmad Fatoum
@ 2026-02-23 11:31 ` Ahmad Fatoum
  2026-02-23 11:31 ` [PATCH 2/4] MAKEALL: devel mode: abort if no positional config Ahmad Fatoum
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:31 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The behavior of MAKEALL to just build all configs and ignore the
specified config is confusing, so add a check guarding against this.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 MAKEALL | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAKEALL b/MAKEALL
index 1e6d0c97b868..001b46a14726 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -461,6 +461,12 @@ else
 				echo "Skipping test due to failed build"
 			fi
 		else
+			if [ ! "${ARCH}" ]; then
+				echo "ARCH must be specified via the -a option or environment"
+				usage
+				exit 1
+			fi
+
 			do_build_defconfig ${ARCH} $config
 		fi
 	done
-- 
2.47.3




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] MAKEALL: devel mode: abort if no positional config
  2026-02-23 11:31 [PATCH 0/4] MAKEALL: sanity check command-line options more Ahmad Fatoum
  2026-02-23 11:31 ` [PATCH 1/4] MAKEALL: early exit if config specified, but no arch Ahmad Fatoum
@ 2026-02-23 11:31 ` Ahmad Fatoum
  2026-02-23 11:31 ` [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument Ahmad Fatoum
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:31 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

devel mode is supposed to make it easy to get started with a particular
config that's built into build/, so let's also expect a single config to
have been specified.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 MAKEALL | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MAKEALL b/MAKEALL
index 001b46a14726..c36aed625820 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -391,6 +391,10 @@ fi
 
 if [ $# -eq 0 ]
 then
+	if [ "$DEVEL" = "1" ]; then
+		echo "DEVEL mode build requires specifying the config as positional argument"
+		exit 1
+	fi
 	if [ ! "${ARCH}" ] || [ ! -d arch/${ARCH} ]
 	then
 		do_build_all
-- 
2.47.3




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument
  2026-02-23 11:31 [PATCH 0/4] MAKEALL: sanity check command-line options more Ahmad Fatoum
  2026-02-23 11:31 ` [PATCH 1/4] MAKEALL: early exit if config specified, but no arch Ahmad Fatoum
  2026-02-23 11:31 ` [PATCH 2/4] MAKEALL: devel mode: abort if no positional config Ahmad Fatoum
@ 2026-02-23 11:31 ` Ahmad Fatoum
  2026-02-23 11:50   ` Sascha Hauer
  2026-02-23 11:31 ` [PATCH 4/4] MAKEALL: flock build directory to prevent concurrent builds Ahmad Fatoum
  2026-02-23 14:55 ` (subset) [PATCH 0/4] MAKEALL: sanity check command-line options more Sascha Hauer
  4 siblings, 1 reply; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:31 UTC (permalink / raw)
  To: barebox; +Cc: Claude Opus 4.6, Ahmad Fatoum

getopts stops parsing at the first non-option argument, so options
like -k after the defconfig name silently end up as positional
arguments. For bare defconfigs these get misinterpreted as pytest
options and are silently ignored, leading to a build without the
intended configuration.

Detect this by checking for trailing arguments when any positional
argument is a bare defconfig (not a labgrid .yaml config, where
extra arguments are intentionally forwarded to pytest).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 MAKEALL | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAKEALL b/MAKEALL
index c36aed625820..dd0b7f8003cc 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -440,6 +440,15 @@ else
 
 		configs+=($i)
 	done
+	if [ ${#pytest_opts[@]} -gt 0 ]; then
+		for i in "${configs[@]}"; do
+			if ! [[ $i =~ .yaml$ ]]; then
+				echo "error: unexpected arguments after defconfig: ${pytest_opts[*]}" >&2
+				echo "hint: place all options before the defconfig argument" >&2
+				exit 1
+			fi
+		done
+	fi
 	for i in "${configs[@]}"; do
 		config=$i
 		if [[ $i =~ ^.*/([^/]+)/([^@]*@|)([^.]+).yaml$ ]]; then
-- 
2.47.3




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] MAKEALL: flock build directory to prevent concurrent builds
  2026-02-23 11:31 [PATCH 0/4] MAKEALL: sanity check command-line options more Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2026-02-23 11:31 ` [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument Ahmad Fatoum
@ 2026-02-23 11:31 ` Ahmad Fatoum
  2026-02-23 14:55 ` (subset) [PATCH 0/4] MAKEALL: sanity check command-line options more Sascha Hauer
  4 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:31 UTC (permalink / raw)
  To: barebox; +Cc: Claude Opus 4.6, Ahmad Fatoum

Two MAKEALL processes using the same build directory concurrently will
corrupt each other's build. Take an exclusive flock on the build
directory at startup so a second MAKEALL targeting the same directory
fails immediately with a clear error message.

To keep the lock valid across non-incremental rebuilds, clear the
directory contents with find -delete instead of rm -rf, preserving
the directory inode the lock is held on.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 MAKEALL | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/MAKEALL b/MAKEALL
index dd0b7f8003cc..7a6fe61f5e84 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -133,7 +133,7 @@ do_build_defconfig() {
 	local log_err="${logdir}/${defconfig}/errors.log"
 	local err=0
 
-	[ "$INCREMENTAL" != "1" ] && rm -rf "${BUILDDIR}"
+	[ "$INCREMENTAL" != "1" ] && find "${BUILDDIR}" -mindepth 1 -delete
 	[ -n "$logdir" ] && mkdir -p "${logdir}/${defconfig}"
 
 	MAKE="make -j${JOBS} ARCH=${arch} O=${BUILDDIR}"
@@ -384,6 +384,15 @@ then
 	mkdir "${logdir}" || exit 1
 fi
 
+# Take an exclusive lock on the build directory to prevent parallel builds
+# from corrupting each other.
+mkdir -p "${BUILDDIR}"
+exec 9<"${BUILDDIR}"
+if ! flock -n 9; then
+	echo "error: another MAKEALL is already using build directory '${BUILDDIR}'" >&2
+	exit 1
+fi
+
 if [ ! "${REGEX}" ]
 then
 	REGEX="*"
-- 
2.47.3




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument
  2026-02-23 11:31 ` [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument Ahmad Fatoum
@ 2026-02-23 11:50   ` Sascha Hauer
  2026-02-23 11:52     ` Ahmad Fatoum
  0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2026-02-23 11:50 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Claude Opus 4.6

On Mon, Feb 23, 2026 at 12:31:34PM +0100, Ahmad Fatoum wrote:
> getopts stops parsing at the first non-option argument, so options
> like -k after the defconfig name silently end up as positional
> arguments. For bare defconfigs these get misinterpreted as pytest
> options and are silently ignored, leading to a build without the
> intended configuration.
> 
> Detect this by checking for trailing arguments when any positional
> argument is a bare defconfig (not a labgrid .yaml config, where
> extra arguments are intentionally forwarded to pytest).

Can't we use getopt for MAKEALL instead of getopts? getopt just does the
right thing.

Sascha

> 
> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
> ---
>  MAKEALL | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAKEALL b/MAKEALL
> index c36aed625820..dd0b7f8003cc 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -440,6 +440,15 @@ else
>  
>  		configs+=($i)
>  	done
> +	if [ ${#pytest_opts[@]} -gt 0 ]; then
> +		for i in "${configs[@]}"; do
> +			if ! [[ $i =~ .yaml$ ]]; then
> +				echo "error: unexpected arguments after defconfig: ${pytest_opts[*]}" >&2
> +				echo "hint: place all options before the defconfig argument" >&2
> +				exit 1
> +			fi
> +		done
> +	fi
>  	for i in "${configs[@]}"; do
>  		config=$i
>  		if [[ $i =~ ^.*/([^/]+)/([^@]*@|)([^.]+).yaml$ ]]; then
> -- 
> 2.47.3
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument
  2026-02-23 11:50   ` Sascha Hauer
@ 2026-02-23 11:52     ` Ahmad Fatoum
  2026-02-23 12:05       ` Sascha Hauer
  0 siblings, 1 reply; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 11:52 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, Claude Opus 4.6

Hi,

On 2/23/26 12:50 PM, Sascha Hauer wrote:
> On Mon, Feb 23, 2026 at 12:31:34PM +0100, Ahmad Fatoum wrote:
>> getopts stops parsing at the first non-option argument, so options
>> like -k after the defconfig name silently end up as positional
>> arguments. For bare defconfigs these get misinterpreted as pytest
>> options and are silently ignored, leading to a build without the
>> intended configuration.
>>
>> Detect this by checking for trailing arguments when any positional
>> argument is a bare defconfig (not a labgrid .yaml config, where
>> extra arguments are intentionally forwarded to pytest).
> 
> Can't we use getopt for MAKEALL instead of getopts? getopt just does the
> right thing.

When using MAKEALL with a labgrid yaml instead of a defconfig, it
derives the defconfig name and interprets the options after the yaml as
pytest options, so use of getopt would break that.

Cheers,
Ahmad

> 
> Sascha
> 
>>
>> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
>> ---
>>  MAKEALL | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/MAKEALL b/MAKEALL
>> index c36aed625820..dd0b7f8003cc 100755
>> --- a/MAKEALL
>> +++ b/MAKEALL
>> @@ -440,6 +440,15 @@ else
>>  
>>  		configs+=($i)
>>  	done
>> +	if [ ${#pytest_opts[@]} -gt 0 ]; then
>> +		for i in "${configs[@]}"; do
>> +			if ! [[ $i =~ .yaml$ ]]; then
>> +				echo "error: unexpected arguments after defconfig: ${pytest_opts[*]}" >&2
>> +				echo "hint: place all options before the defconfig argument" >&2
>> +				exit 1
>> +			fi
>> +		done
>> +	fi
>>  	for i in "${configs[@]}"; do
>>  		config=$i
>>  		if [[ $i =~ ^.*/([^/]+)/([^@]*@|)([^.]+).yaml$ ]]; then
>> -- 
>> 2.47.3
>>
>>
>>
> 




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument
  2026-02-23 11:52     ` Ahmad Fatoum
@ 2026-02-23 12:05       ` Sascha Hauer
  2026-02-23 12:06         ` Ahmad Fatoum
  0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2026-02-23 12:05 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Claude Opus 4.6

On Mon, Feb 23, 2026 at 12:52:52PM +0100, Ahmad Fatoum wrote:
> Hi,
> 
> On 2/23/26 12:50 PM, Sascha Hauer wrote:
> > On Mon, Feb 23, 2026 at 12:31:34PM +0100, Ahmad Fatoum wrote:
> >> getopts stops parsing at the first non-option argument, so options
> >> like -k after the defconfig name silently end up as positional
> >> arguments. For bare defconfigs these get misinterpreted as pytest
> >> options and are silently ignored, leading to a build without the
> >> intended configuration.
> >>
> >> Detect this by checking for trailing arguments when any positional
> >> argument is a bare defconfig (not a labgrid .yaml config, where
> >> extra arguments are intentionally forwarded to pytest).
> > 
> > Can't we use getopt for MAKEALL instead of getopts? getopt just does the
> > right thing.
> 
> When using MAKEALL with a labgrid yaml instead of a defconfig, it
> derives the defconfig name and interprets the options after the yaml as
> pytest options, so use of getopt would break that.

You can use -- as final argument which stops getopt parsing.

Sascha

> 
> Cheers,
> Ahmad
> 
> > 
> > Sascha
> > 
> >>
> >> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
> >> ---
> >>  MAKEALL | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/MAKEALL b/MAKEALL
> >> index c36aed625820..dd0b7f8003cc 100755
> >> --- a/MAKEALL
> >> +++ b/MAKEALL
> >> @@ -440,6 +440,15 @@ else
> >>  
> >>  		configs+=($i)
> >>  	done
> >> +	if [ ${#pytest_opts[@]} -gt 0 ]; then
> >> +		for i in "${configs[@]}"; do
> >> +			if ! [[ $i =~ .yaml$ ]]; then
> >> +				echo "error: unexpected arguments after defconfig: ${pytest_opts[*]}" >&2
> >> +				echo "hint: place all options before the defconfig argument" >&2
> >> +				exit 1
> >> +			fi
> >> +		done
> >> +	fi
> >>  	for i in "${configs[@]}"; do
> >>  		config=$i
> >>  		if [[ $i =~ ^.*/([^/]+)/([^@]*@|)([^.]+).yaml$ ]]; then
> >> -- 
> >> 2.47.3
> >>
> >>
> >>
> > 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument
  2026-02-23 12:05       ` Sascha Hauer
@ 2026-02-23 12:06         ` Ahmad Fatoum
  0 siblings, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2026-02-23 12:06 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, Claude Opus 4.6



On 2/23/26 1:05 PM, Sascha Hauer wrote:
> On Mon, Feb 23, 2026 at 12:52:52PM +0100, Ahmad Fatoum wrote:
>> Hi,
>>
>> On 2/23/26 12:50 PM, Sascha Hauer wrote:
>>> On Mon, Feb 23, 2026 at 12:31:34PM +0100, Ahmad Fatoum wrote:
>>>> getopts stops parsing at the first non-option argument, so options
>>>> like -k after the defconfig name silently end up as positional
>>>> arguments. For bare defconfigs these get misinterpreted as pytest
>>>> options and are silently ignored, leading to a build without the
>>>> intended configuration.
>>>>
>>>> Detect this by checking for trailing arguments when any positional
>>>> argument is a bare defconfig (not a labgrid .yaml config, where
>>>> extra arguments are intentionally forwarded to pytest).
>>>
>>> Can't we use getopt for MAKEALL instead of getopts? getopt just does the
>>> right thing.
>>
>> When using MAKEALL with a labgrid yaml instead of a defconfig, it
>> derives the defconfig name and interprets the options after the yaml as
>> pytest options, so use of getopt would break that.
> 
> You can use -- as final argument which stops getopt parsing.

Fair enough, I think 1,2,4 are good to go though?
I will rework 3/4 later.

Thanks,
Ahmad

> 
> Sascha
> 
>>
>> Cheers,
>> Ahmad
>>
>>>
>>> Sascha
>>>
>>>>
>>>> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
>>>> ---
>>>>  MAKEALL | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/MAKEALL b/MAKEALL
>>>> index c36aed625820..dd0b7f8003cc 100755
>>>> --- a/MAKEALL
>>>> +++ b/MAKEALL
>>>> @@ -440,6 +440,15 @@ else
>>>>  
>>>>  		configs+=($i)
>>>>  	done
>>>> +	if [ ${#pytest_opts[@]} -gt 0 ]; then
>>>> +		for i in "${configs[@]}"; do
>>>> +			if ! [[ $i =~ .yaml$ ]]; then
>>>> +				echo "error: unexpected arguments after defconfig: ${pytest_opts[*]}" >&2
>>>> +				echo "hint: place all options before the defconfig argument" >&2
>>>> +				exit 1
>>>> +			fi
>>>> +		done
>>>> +	fi
>>>>  	for i in "${configs[@]}"; do
>>>>  		config=$i
>>>>  		if [[ $i =~ ^.*/([^/]+)/([^@]*@|)([^.]+).yaml$ ]]; then
>>>> -- 
>>>> 2.47.3
>>>>
>>>>
>>>>
>>>
>>
>>
> 




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: (subset) [PATCH 0/4] MAKEALL: sanity check command-line options more
  2026-02-23 11:31 [PATCH 0/4] MAKEALL: sanity check command-line options more Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2026-02-23 11:31 ` [PATCH 4/4] MAKEALL: flock build directory to prevent concurrent builds Ahmad Fatoum
@ 2026-02-23 14:55 ` Sascha Hauer
  4 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2026-02-23 14:55 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Mon, 23 Feb 2026 12:31:31 +0100, Ahmad Fatoum wrote:
> I watched how Claude Code stumbles to use MAKEALL correctly for a while
> and indeed MAKEALL could really use some improvement in user experience.
> 
> This series adds some sanity checks to make it more difficult to misuse.
> 
> Ahmad Fatoum (4):
>   MAKEALL: early exit if config specified, but no arch
>   MAKEALL: devel mode: abort if no positional config
>   MAKEALL: error out on options placed after the defconfig argument
>   MAKEALL: flock build directory to prevent concurrent builds
> 
> [...]

Applied, thanks!

[1/4] MAKEALL: early exit if config specified, but no arch
      https://git.pengutronix.de/cgit/barebox/commit/?id=ec05602df38e (link may not be stable)
[2/4] MAKEALL: devel mode: abort if no positional config
      https://git.pengutronix.de/cgit/barebox/commit/?id=b1cef49ae15a (link may not be stable)
[4/4] MAKEALL: flock build directory to prevent concurrent builds
      https://git.pengutronix.de/cgit/barebox/commit/?id=9fa9842b35fb (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-02-23 14:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-23 11:31 [PATCH 0/4] MAKEALL: sanity check command-line options more Ahmad Fatoum
2026-02-23 11:31 ` [PATCH 1/4] MAKEALL: early exit if config specified, but no arch Ahmad Fatoum
2026-02-23 11:31 ` [PATCH 2/4] MAKEALL: devel mode: abort if no positional config Ahmad Fatoum
2026-02-23 11:31 ` [PATCH 3/4] MAKEALL: error out on options placed after the defconfig argument Ahmad Fatoum
2026-02-23 11:50   ` Sascha Hauer
2026-02-23 11:52     ` Ahmad Fatoum
2026-02-23 12:05       ` Sascha Hauer
2026-02-23 12:06         ` Ahmad Fatoum
2026-02-23 11:31 ` [PATCH 4/4] MAKEALL: flock build directory to prevent concurrent builds Ahmad Fatoum
2026-02-23 14:55 ` (subset) [PATCH 0/4] MAKEALL: sanity check command-line options more Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox