Bug#1034071: u-boot-menu: Does not show a menu due to 'prompt 0'

2023-04-18 Thread Vagrant Cascadian
On 2023-04-08, Diederik de Haas wrote:
> On Saturday, 8 April 2023 04:24:10 CEST Vagrant Cascadian wrote:
>> On 2023-04-08, Diederik de Haas wrote:
>> I had intended to test a few different versions to see how they were
>> affected before moving forward with that... but I did not get around to
>> it and we are now very late in the freeze.
>> 
>> If it proves harmless on the version of u-boot in bookworm (2023.01*)
>> and bullseye (2021.01*), and if we are being really careful, buster
>> (2019.01*) ... then I think it would be good to consider for bookworm,
>> or maybe the first bookworm point release?
>
> Anyway, the goal is to make it configurable in u-boot-menu and it would be 
> better to keep the default behavior as it was.
> A new/better default could be considered for Trixie.
>
> I've updated the MR and attached the updated patch which only makes the 
> 'prompt' value a variable (with a default of '0').

I've tested u-boot-menu 4.2.2, which contains your patch applied, and at
least on the pinebook-pro it works correctly with u-boot 2023.04 (with both
U_BOOT_PROMPT=0 and U_BOOT_PROMPT=1) and does not break u-boot 2023.01
(although that version effectively ignores any setting, always behaving
as if U_BOOT_PROMPT=1).

So at least for versions of u-boot shipped in bookworm or newer, it does
not appear to cause problems.

Would be good to try the versions from bullseye and buster at some
point, but my hunch is it will be fine. The only problem I would expect
is with arbitrary vendor forks of u-boot which may or may not have
changed the code.

live well,
  vagrant


signature.asc
Description: PGP signature


Bug#1034071: u-boot-menu: Does not show a menu due to 'prompt 0'

2023-04-08 Thread Diederik de Haas
On Saturday, 8 April 2023 04:24:10 CEST Vagrant Cascadian wrote:
> On 2023-04-08, Diederik de Haas wrote:
> > As the subject says, u-boot-menu doesn't show a (boot) menu as the
> > 'prompt' setting has a hardcoded value of '0'. If you'd change that to
> > say '5' then you do see a menu from which you can choose.
> > But the next time you'd run 'u-boot-update' that value gets overwritten.
> 
> Old versions of u-boot basically ignore this value in my experience, but
> that bug was fixed in upstream u-boot 2023.04:
> https://source.denx.de/u-boot/u-boot/-/commit/739e8361f3fe78038251216df6096
> a32bc2d5839

We do indeed use (an) u-boot version 2023.04-rc1-gbe645fef.

> > It would be great if this could still make it into Bookworm and I do
> > believe this is a targeted fix. I set 'Severity: normal', but a case can
> > be made for a higher severity as 'u-boot-menu' doesn't show a menu.
> > I'd be also fine with a default value of '0', so that the only change
> > would be to make 'prompt' configurable.
> 
> The version of u-boot in bookworm (2023.01) should not be affected, so
> it is somewhat less urgent... or are you experiencing this issue with
> u-boot as shipped in bookworm?

Plebian is (for now at least) pure Debian, but with mainline-ish u-boot 
version which *does* support Pine64 Quartz64 devices.
So we don't use u-boot as shipped in Bookworm, but we do use u-boot-menu as 
shipped in Bookworm, which provides customization options for most items.
 
> Before changing the default behavior, I was thinking of proposing a
> patch for bookworm that kept the current behavior but allowed overriding
> it... that would at least be reasonably cautious.

Yep, I agree. That's what we actually need.

In my MR you made the following comment:
> Last I tried, it should also work with "1" as the default (essentially a
> boolean)... "5" seems a bit arbitrary, even if the u-boot code is
> technically checking for zero or non-zero. Or is there a behavioral
> difference with different values?

And it looks like you are right. The value '5' worked as it's !=0, but it was 
indeed arbitrary.

Based on your comment, I tried a few more values and they all had the exact
same effect: 1, 5 or 10
So it does appear to be a boolean.

It looks like the upstream code still has a bug in it as I (then) expected it 
would wait until U_BOOT_TIMEOUT had passed, but it seems ~3 seconds, 
regardless of the value I set for 'prompt' or U_BOOT_TIMEOUT.
If I press a key during the menu, then it waits ... not till U_BOOT_TIMEOUT 
has passed, but indefinitely?

> I had intended to test a few different versions to see how they were
> affected before moving forward with that... but I did not get around to
> it and we are now very late in the freeze.
> 
> If it proves harmless on the version of u-boot in bookworm (2023.01*)
> and bullseye (2021.01*), and if we are being really careful, buster
> (2019.01*) ... then I think it would be good to consider for bookworm,
> or maybe the first bookworm point release?

Anyway, the goal is to make it configurable in u-boot-menu and it would be 
better to keep the default behavior as it was.
A new/better default could be considered for Trixie.

I've updated the MR and attached the updated patch which only makes the 
'prompt' value a variable (with a default of '0').

Cheers,
  Diederik>From 4f2e1b49562205a01865bd37f3cbd7bd245b08e1 Mon Sep 17 00:00:00 2001
From: Diederik de Haas 
Date: Thu, 6 Apr 2023 18:10:12 +0200
Subject: [PATCH] u-boot-update: Make 'prompt' configurable

The 'prompt' value was hard-coded to '0', which means the menu isn't
actually shown and there was no non-hacky way to change that.
So replace the hard-coded '0' with an `U_BOOT_PROMPT` variable to make
it configurable.

Closes: #1034071
---
 debian/changelog | 6 ++
 default  | 1 +
 u-boot-update| 3 ++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 5192c3a..b2b8fc0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+u-boot-menu (4.2.2) UNRELEASED; urgency=medium
+
+  * u-boot-update: Make 'prompt' configurable (Closes: #1034071)
+
+ -- Diederik de Haas   Thu, 06 Apr 2023 18:08:55 +0200
+
 u-boot-menu (4.2.1) unstable; urgency=medium
 
   * Revert "add kalsrseed support" (Closes: #1025954)
diff --git a/default b/default
index 2e29c83..782453b 100644
--- a/default
+++ b/default
@@ -4,6 +4,7 @@
 
 #U_BOOT_ALTERNATIVES="default recovery"
 #U_BOOT_DEFAULT="l0"
+#U_BOOT_PROMPT="0"
 #U_BOOT_ENTRIES="all"
 #U_BOOT_MENU_LABEL="Debian GNU/Linux"
 #U_BOOT_PARAMETERS="ro quiet"
diff --git a/u-boot-update b/u-boot-update
index 90c4087..f887560 100755
--- a/u-boot-update
+++ b/u-boot-update
@@ -96,6 +96,7 @@ fi
 
 U_BOOT_ALTERNATIVES="${U_BOOT_ALTERNATIVES:-default recovery}"
 U_BOOT_DEFAULT="${U_BOOT_DEFAULT:-l0}"
+U_BOOT_PROMPT="${U_BOOT_PROMPT:-0}"
 U_BOOT_ENTRIES="${U_BOOT_ENTRIES:-all}"
 U_BOOT_TIMEOUT="${U_BOOT_TIMEOUT:-50}"
 

Bug#1034071: u-boot-menu: Does not show a menu due to 'prompt 0'

2023-04-07 Thread Vagrant Cascadian
On 2023-04-08, Diederik de Haas wrote:
> As the subject says, u-boot-menu doesn't show a (boot) menu as the
> 'prompt' setting has a hardcoded value of '0'. If you'd change that to
> say '5' then you do see a menu from which you can choose.
> But the next time you'd run 'u-boot-update' that value gets overwritten.

Old versions of u-boot basically ignore this value in my experience, but
that bug was fixed in upstream u-boot 2023.04:

  
https://source.denx.de/u-boot/u-boot/-/commit/739e8361f3fe78038251216df6096a32bc2d5839


> It would be great if this could still make it into Bookworm and I do
> believe this is a targeted fix. I set 'Severity: normal', but a case can
> be made for a higher severity as 'u-boot-menu' doesn't show a menu.
> I'd be also fine with a default value of '0', so that the only change
> would be to make 'prompt' configurable.

The version of u-boot in bookworm (2023.01) should not be affected, so
it is somewhat less urgent... or are you experiencing this issue with
u-boot as shipped in bookworm?

Before changing the default behavior, I was thinking of proposing a
patch for bookworm that kept the current behavior but allowed overriding
it... that would at least be reasonably cautious.

I had intended to test a few different versions to see how they were
affected before moving forward with that... but I did not get around to
it and we are now very late in the freeze.

If it proves harmless on the version of u-boot in bookworm (2023.01*)
and bullseye (2021.01*), and if we are being really careful, buster
(2019.01*) ... then I think it would be good to consider for bookworm,
or maybe the first bookworm point release?


Thanks for submitting the patch and merge request!

> diff --git a/u-boot-update b/u-boot-update
> index 90c4087..c726c9e 100755
> --- a/u-boot-update
> +++ b/u-boot-update
> @@ -96,6 +96,7 @@ fi
>  
>  U_BOOT_ALTERNATIVES="${U_BOOT_ALTERNATIVES:-default recovery}"
>  U_BOOT_DEFAULT="${U_BOOT_DEFAULT:-l0}"
> +U_BOOT_PROMPT="${U_BOOT_PROMPT:-5}"
>  U_BOOT_ENTRIES="${U_BOOT_ENTRIES:-all}"
>  U_BOOT_TIMEOUT="${U_BOOT_TIMEOUT:-50}"
>  U_BOOT_MENU_LABEL="${U_BOOT_MENU_LABEL:-${PRETTY_NAME:-Debian GNU/Linux 
> kernel}}"

My only question here is why "5" vs. say, "1"? I *think* it is
effectively a zero/non-zero boolean.


live well,
  vagrant


signature.asc
Description: PGP signature


Bug#1034071: u-boot-menu: Does not show a menu due to 'prompt 0'

2023-04-07 Thread Diederik de Haas
Package: u-boot-menu
Version: 4.2.1
Severity: normal
Tags: patch

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

As the subject says, u-boot-menu doesn't show a (boot) menu as the
'prompt' setting has a hardcoded value of '0'. If you'd change that to
say '5' then you do see a menu from which you can choose.
But the next time you'd run 'u-boot-update' that value gets overwritten.

https://salsa.debian.org/debian/u-boot-menu/-/merge_requests/13 is my MR
to make the 'prompt' value configurable with a default value of '5'.

It would be great if this could still make it into Bookworm and I do
believe this is a targeted fix. I set 'Severity: normal', but a case can
be made for a higher severity as 'u-boot-menu' doesn't show a menu.
I'd be also fine with a default value of '0', so that the only change
would be to make 'prompt' configurable.

I'll also attach the patch of MR 13 as a patch file.

- -- System Information:
Debian Release: 12.0
  APT prefers testing-security
  APT policy: (990, 'testing-security'), (990, 'testing'), (500, 'unstable')
Architecture: arm64 (aarch64)

Kernel: Linux 6.1.0-7-arm64 (SMP w/4 CPU threads)
Kernel taint flags: TAINT_CRAP
Locale: LANG=en_US.utf8, LC_CTYPE=en_US.utf8 (charmap=UTF-8), LANGUAGE=en_US:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages u-boot-menu depends on:
ii  linux-base  4.9

u-boot-menu recommends no packages.

Versions of packages u-boot-menu suggests:
pn  flash-kernel  

- -- no debconf information

-BEGIN PGP SIGNATURE-

iHUEARYIAB0WIQT1sUPBYsyGmi4usy/XblvOeH7bbgUCZDCVcQAKCRDXblvOeH7b
buLOAP9yKZNz0+Vfk+Y186OcFVIpetwPWIr+XFEw8IVAFLs17QEA7OHU0vY4KWFs
1Wh7DR5LuMimmcbIQcKjxRz3Fdt/wQc=
=uIwT
-END PGP SIGNATURE-
>From 382c1e20b0809d5dd9dcba757e574b4bfbcca0dc Mon Sep 17 00:00:00 2001
From: Diederik de Haas 
Date: Thu, 6 Apr 2023 18:10:12 +0200
Subject: [PATCH] u-boot-update: Make 'prompt' configurable with default value
 of 5

The 'prompt' value was hard-coded to 0, which means the menu isn't
actually shown, thereby losing its function as a 'menu' list from which
the user can choose from.

So create an `U_BOOT_PROMPT` variable to make it configurable and set
the default value to 5, so the menu is actually shown and the user can
make a choice from the menu.
---
 debian/changelog | 6 ++
 default  | 1 +
 u-boot-update| 3 ++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 5192c3a..cf371ca 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+u-boot-menu (4.2.2) UNRELEASED; urgency=medium
+
+  * u-boot-update: Make 'prompt' configurable with default value of 5
+
+ -- Diederik de Haas   Thu, 06 Apr 2023 18:08:55 +0200
+
 u-boot-menu (4.2.1) unstable; urgency=medium
 
   * Revert "add kalsrseed support" (Closes: #1025954)
diff --git a/default b/default
index 2e29c83..1d17be7 100644
--- a/default
+++ b/default
@@ -4,6 +4,7 @@
 
 #U_BOOT_ALTERNATIVES="default recovery"
 #U_BOOT_DEFAULT="l0"
+#U_BOOT_PROMPT="5"
 #U_BOOT_ENTRIES="all"
 #U_BOOT_MENU_LABEL="Debian GNU/Linux"
 #U_BOOT_PARAMETERS="ro quiet"
diff --git a/u-boot-update b/u-boot-update
index 90c4087..c726c9e 100755
--- a/u-boot-update
+++ b/u-boot-update
@@ -96,6 +96,7 @@ fi
 
 U_BOOT_ALTERNATIVES="${U_BOOT_ALTERNATIVES:-default recovery}"
 U_BOOT_DEFAULT="${U_BOOT_DEFAULT:-l0}"
+U_BOOT_PROMPT="${U_BOOT_PROMPT:-5}"
 U_BOOT_ENTRIES="${U_BOOT_ENTRIES:-all}"
 U_BOOT_TIMEOUT="${U_BOOT_TIMEOUT:-50}"
 U_BOOT_MENU_LABEL="${U_BOOT_MENU_LABEL:-${PRETTY_NAME:-Debian GNU/Linux 
kernel}}"
@@ -162,7 +163,7 @@ _CONFIG="\
 
 default ${U_BOOT_DEFAULT}
 menu title U-Boot menu
-prompt 0
+prompt ${U_BOOT_PROMPT}
 timeout ${U_BOOT_TIMEOUT}
 "
 
-- 
2.40.0