All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cmd: Remove default prompt from output
@ 2025-08-15 16:06 Daniel Schultz
  2025-08-15 16:12 ` Tom Rini
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Schultz @ 2025-08-15 16:06 UTC (permalink / raw)
  To: trini, holger.brunck, hs, sjg, jh80.chung, u-boot
  Cc: upstream, Daniel Schultz

Some commands include the U-Boot prompt (=>) in their output,
which can interfere with tools like labgrid that rely on prompt
detection to determine when a command has completed. This may cause
such tools to misinterpret partial output.

To avoid this issue, it's better to update the command output itself
rather than modifying the actual U-Boot prompt. Changing the prompt
is not acceptable in many cases, as some boards have used the default
prompt (=>) for years, and altering it - especially just for testing -
could lead to inconsistencies or unintended side effects.
Instead, replace instances of the prompt that appear within command
output (not the real prompt) with an alternative like -> to ensure
correct parsing by tools that rely on prompt recognition.

Signed-off-by: Daniel Schultz <d.schultz@phytec.de>
---
 cmd/i2c.c      |  2 +-
 cmd/mtdparts.c | 12 ++++++------
 common/hash.c  |  6 +++---
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/cmd/i2c.c b/cmd/i2c.c
index e021067e68a..f7695047629 100644
--- a/cmd/i2c.c
+++ b/cmd/i2c.c
@@ -715,7 +715,7 @@ static int do_i2c_crc(struct cmd_tbl *cmdtp, int flag, int argc,
 	 */
 	count = hextoul(argv[3], NULL);
 
-	printf ("CRC32 for %08lx ... %08lx ==> ", addr, addr + count - 1);
+	printf("CRC32 for %08lx ... %08lx --> ", addr, addr + count - 1);
 	/*
 	 * CRC a byte at a time.  This is going to be slooow, but hey, the
 	 * memories are small and slow too so hopefully nobody notices.
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index 571b79f091d..48d06f2e000 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -242,7 +242,7 @@ static void index_partitions(void)
 			if (dev == current_mtd_dev) {
 				mtddevnum += current_mtd_partnum;
 				env_set_ulong("mtddevnum", mtddevnum);
-				debug("=> mtddevnum %d,\n", mtddevnum);
+				debug("-> mtddevnum %d,\n", mtddevnum);
 				break;
 			}
 			mtddevnum += dev->num_parts;
@@ -252,17 +252,17 @@ static void index_partitions(void)
 		if (part) {
 			env_set("mtddevname", part->name);
 
-			debug("=> mtddevname %s\n", part->name);
+			debug("-> mtddevname %s\n", part->name);
 		} else {
 			env_set("mtddevname", NULL);
 
-			debug("=> mtddevname NULL\n");
+			debug("-> mtddevname NULL\n");
 		}
 	} else {
 		env_set("mtddevnum", NULL);
 		env_set("mtddevname", NULL);
 
-		debug("=> mtddevnum NULL\n=> mtddevname NULL\n");
+		debug("-> mtddevnum NULL\n-> mtddevname NULL\n");
 	}
 }
 
@@ -282,12 +282,12 @@ static void current_save(void)
 		env_set("partition", buf);
 		strncpy(last_partition, buf, 16);
 
-		debug("=> partition %s\n", buf);
+		debug("-> partition %s\n", buf);
 	} else {
 		env_set("partition", NULL);
 		last_partition[0] = '\0';
 
-		debug("=> partition NULL\n");
+		debug("-> partition NULL\n");
 	}
 	index_partitions();
 }
diff --git a/common/hash.c b/common/hash.c
index 0c45992d5c7..07a4df963ad 100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -535,7 +535,7 @@ static void hash_show(struct hash_algo *algo, ulong addr, ulong len, uint8_t *ou
 {
 	int i;
 
-	printf("%s for %08lx ... %08lx ==> ", algo->name, addr, addr + len - 1);
+	printf("%s for %08lx ... %08lx --> ", algo->name, addr, addr + len - 1);
 	for (i = 0; i < algo->digest_size; i++)
 		printf("%02x", output[i]);
 }
@@ -621,8 +621,8 @@ int hash_command(const char *algo_name, int flags, struct cmd_tbl *cmdtp,
 
 		crc = crc32_wd(0, (const uchar *)addr, len, CHUNKSZ_CRC32);
 
-		printf("CRC32 for %08lx ... %08lx ==> %08lx\n",
-				addr, addr + len - 1, crc);
+		printf("CRC32 for %08lx ... %08lx --> %08lx\n",
+		       addr, addr + len - 1, crc);
 
 		if (argc >= 3) {
 			ptr = (ulong *)hextoul(argv[0], NULL);
-- 
2.25.1


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

* Re: [PATCH] cmd: Remove default prompt from output
  2025-08-15 16:06 [PATCH] cmd: Remove default prompt from output Daniel Schultz
@ 2025-08-15 16:12 ` Tom Rini
  2025-08-19  9:14   ` Quentin Schulz
  2025-09-01  5:12   ` Heiko Schocher
  0 siblings, 2 replies; 4+ messages in thread
From: Tom Rini @ 2025-08-15 16:12 UTC (permalink / raw)
  To: Daniel Schultz; +Cc: holger.brunck, hs, sjg, jh80.chung, u-boot, upstream

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

On Fri, Aug 15, 2025 at 09:06:35AM -0700, Daniel Schultz wrote:

> Some commands include the U-Boot prompt (=>) in their output,
> which can interfere with tools like labgrid that rely on prompt
> detection to determine when a command has completed. This may cause
> such tools to misinterpret partial output.
> 
> To avoid this issue, it's better to update the command output itself
> rather than modifying the actual U-Boot prompt. Changing the prompt
> is not acceptable in many cases, as some boards have used the default
> prompt (=>) for years, and altering it - especially just for testing -
> could lead to inconsistencies or unintended side effects.
> Instead, replace instances of the prompt that appear within command
> output (not the real prompt) with an alternative like -> to ensure
> correct parsing by tools that rely on prompt recognition.
> 
> Signed-off-by: Daniel Schultz <d.schultz@phytec.de>
> ---
>  cmd/i2c.c      |  2 +-
>  cmd/mtdparts.c | 12 ++++++------
>  common/hash.c  |  6 +++---
>  3 files changed, 10 insertions(+), 10 deletions(-)

This is two cases, and I'm not sure I like the proposal here, sorry. For
cmd/mtdparts.c, it's debug statements. We can change them, but are they
also enabled by default in anything? The other cases, we're changing
output along the lines of:

> -	printf ("CRC32 for %08lx ... %08lx ==> ", addr, addr + count - 1);
> +	printf("CRC32 for %08lx ... %08lx --> ", addr, addr + count - 1);

In each case, and I'm surprised there's not testing that globs on that
today. I see the wget case that uses "==>" and we do have a test that
checks it, today. Is your testing framework not able to handle "=>" in
the middle of a line? Thanks.

-- 
Tom

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

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

* Re: [PATCH] cmd: Remove default prompt from output
  2025-08-15 16:12 ` Tom Rini
@ 2025-08-19  9:14   ` Quentin Schulz
  2025-09-01  5:12   ` Heiko Schocher
  1 sibling, 0 replies; 4+ messages in thread
From: Quentin Schulz @ 2025-08-19  9:14 UTC (permalink / raw)
  To: Tom Rini, Daniel Schultz
  Cc: holger.brunck, hs, sjg, jh80.chung, u-boot, upstream

Hi Daniel, Tom,

On 8/15/25 6:12 PM, Tom Rini wrote:
> On Fri, Aug 15, 2025 at 09:06:35AM -0700, Daniel Schultz wrote:
> 
>> Some commands include the U-Boot prompt (=>) in their output,
>> which can interfere with tools like labgrid that rely on prompt
>> detection to determine when a command has completed. This may cause
>> such tools to misinterpret partial output.
>>
>> To avoid this issue, it's better to update the command output itself
>> rather than modifying the actual U-Boot prompt. Changing the prompt
>> is not acceptable in many cases, as some boards have used the default
>> prompt (=>) for years, and altering it - especially just for testing -
>> could lead to inconsistencies or unintended side effects.
>> Instead, replace instances of the prompt that appear within command
>> output (not the real prompt) with an alternative like -> to ensure
>> correct parsing by tools that rely on prompt recognition.
>>
>> Signed-off-by: Daniel Schultz <d.schultz@phytec.de>
>> ---
>>   cmd/i2c.c      |  2 +-
>>   cmd/mtdparts.c | 12 ++++++------
>>   common/hash.c  |  6 +++---
>>   3 files changed, 10 insertions(+), 10 deletions(-)
> 
> This is two cases, and I'm not sure I like the proposal here, sorry. For
> cmd/mtdparts.c, it's debug statements. We can change them, but are they
> also enabled by default in anything? The other cases, we're changing
> output along the lines of:
> 
>> -	printf ("CRC32 for %08lx ... %08lx ==> ", addr, addr + count - 1);
>> +	printf("CRC32 for %08lx ... %08lx --> ", addr, addr + count - 1);
> 
> In each case, and I'm surprised there's not testing that globs on that
> today. I see the wget case that uses "==>" and we do have a test that
> checks it, today. Is your testing framework not able to handle "=>" in
> the middle of a line? Thanks.
> 

In the case of labgrid, I assume this should be possible.

c.f. https://labgrid.readthedocs.io/en/latest/configuration.html#ubootdriver

Arguments:

         prompt (regex, default=””): U-Boot prompt to match

I assume you just want to be using

UBootDriver:
   prompt: '^=> '

instead of simply using '=> ' ?

While SYS_PROMPT defaults to => for anything but the Zynq architecture, 
it can be set from defconfigs as well and look what we have here:

configs/brcp150_defconfig
63:CONFIG_SYS_PROMPT="-> "

configs/brcp170_defconfig
62:CONFIG_SYS_PROMPT="-> "

configs/brcp1_1r_defconfig
62:CONFIG_SYS_PROMPT="-> "

configs/brcp1_1r_switch_defconfig
62:CONFIG_SYS_PROMPT="-> "

configs/brcp1_2r_defconfig
62:CONFIG_SYS_PROMPT="-> "

configs/brsmarc2_defconfig
62:CONFIG_SYS_PROMPT="-> "

So now we would be breaking those users instead. I'm not sure it's a 
good idea for U-Boot to do this, we'll never find a way to have the 
prompt never repeated in log messages for all supported platforms. You 
can always fix those in your own fork if you want though. But I fear 
this will be the beginning of an endless loop of revert commits because 
it broke one's CI.

Cheers,
Quentin

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

* Re: [PATCH] cmd: Remove default prompt from output
  2025-08-15 16:12 ` Tom Rini
  2025-08-19  9:14   ` Quentin Schulz
@ 2025-09-01  5:12   ` Heiko Schocher
  1 sibling, 0 replies; 4+ messages in thread
From: Heiko Schocher @ 2025-09-01  5:12 UTC (permalink / raw)
  To: Tom Rini, Daniel Schultz; +Cc: holger.brunck, sjg, jh80.chung, u-boot, upstream

Hi Daniel,

On 15.08.25 18:12, Tom Rini wrote:
> On Fri, Aug 15, 2025 at 09:06:35AM -0700, Daniel Schultz wrote:
> 
>> Some commands include the U-Boot prompt (=>) in their output,
>> which can interfere with tools like labgrid that rely on prompt
>> detection to determine when a command has completed. This may cause
>> such tools to misinterpret partial output.
>>
>> To avoid this issue, it's better to update the command output itself
>> rather than modifying the actual U-Boot prompt. Changing the prompt
>> is not acceptable in many cases, as some boards have used the default
>> prompt (=>) for years, and altering it - especially just for testing -
>> could lead to inconsistencies or unintended side effects.
>> Instead, replace instances of the prompt that appear within command
>> output (not the real prompt) with an alternative like -> to ensure
>> correct parsing by tools that rely on prompt recognition.
>>
>> Signed-off-by: Daniel Schultz <d.schultz@phytec.de>
>> ---
>>   cmd/i2c.c      |  2 +-
>>   cmd/mtdparts.c | 12 ++++++------
>>   common/hash.c  |  6 +++---
>>   3 files changed, 10 insertions(+), 10 deletions(-)
> 
> This is two cases, and I'm not sure I like the proposal here, sorry. For
> cmd/mtdparts.c, it's debug statements. We can change them, but are they
> also enabled by default in anything? The other cases, we're changing
> output along the lines of:
> 
>> -	printf ("CRC32 for %08lx ... %08lx ==> ", addr, addr + count - 1);
>> +	printf("CRC32 for %08lx ... %08lx --> ", addr, addr + count - 1);
> 
> In each case, and I'm surprised there's not testing that globs on that
> today. I see the wget case that uses "==>" and we do have a test that
> checks it, today. Is your testing framework not able to handle "=>" in
> the middle of a line? Thanks.

That would be exactly also my question ... fix the tool instead?

Changing this output may breaks automated tests with other tools...

for example tbot handles this case:

https://tbot.tools/modules/machine_channel.html#tbot.machine.channel.Channel.read_until_prompt

bye,
Heiko
-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic

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

end of thread, other threads:[~2025-09-01  5:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15 16:06 [PATCH] cmd: Remove default prompt from output Daniel Schultz
2025-08-15 16:12 ` Tom Rini
2025-08-19  9:14   ` Quentin Schulz
2025-09-01  5:12   ` Heiko Schocher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.