[PATCH] menu: Re-enable the ANSI codes

[Tom Rini]

On Sat, Jun 24, 2023 at 11:01:13AM +0200, Pali Rohár wrote:
> On Saturday 17 June 2023 22:28:24 Heinrich Schuchardt wrote:
> > On 6/17/23 12:48, Simon Glass wrote:
> > > Hi Pali,
> > > 
> > > On Thu, 15 Jun 2023 at 17:56, Pali Rohár <pali at kernel.org> wrote:
> > > > 
> > > > On Thursday 15 June 2023 13:34:09 Simon Glass wrote:
> > > > > Hi Pali,
> > > > > 
> > > > > On Mon, 12 Jun 2023 at 22:33, Pali Rohár <pali at kernel.org> wrote:
> > > > > > 
> > > > > > On Monday 12 June 2023 22:17:48 Simon Glass wrote:
> > > > > > > Hi Pali,
> > > > > > > 
> > > > > > > On Mon, 12 Jun 2023 at 21:22, Pali Rohár <pali at kernel.org> wrote:
> > > > > > > > 
> > > > > > > > On Monday 12 June 2023 21:14:32 Simon Glass wrote:
> > > > > > > > > The intent here was to allow ANSI codes to be disabled, since it was
> > > > > > > > > proving impoosible to test operation of the menu code when it kept moving
> > > > > > > > > the cursor. Unfortunately this ended up in the patch.
> > > > > > > > > 
> > > > > > > > > Correct this by enabling ANSI again.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > Reported-by: Pali Rohár <pali at kernel.org>
> > > > > > > > > Reported-by: Mark Kettenis <mark.kettenis at xs4all.nl>
> > > > > > > > > Reported-by: Frank Wunderlich <frank-w at public-files.de>
> > > > > > > > > Fixes: 32bab0eae51b ("menu: Make use of CLI character processing")
> > > > > > > > > ---
> > > > > > > > > 
> > > > > > > > >   common/menu.c | 2 +-
> > > > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/common/menu.c b/common/menu.c
> > > > > > > > > index 94514177e4e9..b55cf7b99967 100644
> > > > > > > > > --- a/common/menu.c
> > > > > > > > > +++ b/common/menu.c
> > > > > > > > > @@ -15,7 +15,7 @@
> > > > > > > > > 
> > > > > > > > >   #include "menu.h"
> > > > > > > > > 
> > > > > > > > > -#define ansi 0
> > > > > > > > > +#define ansi 1
> > > > > > > > > 
> > > > > > > > >   /*
> > > > > > > > >    * Internally, each item in a menu is represented by a struct menu_item.
> > > > > > > > > --
> > > > > > > > > 2.41.0.162.gfafddb0af9-goog
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Hello, I have tested this change but bootmenu still does not work. There
> > > > > > > > is still same issue which I reported month ago. When I press DOWN key
> > > > > > > > then bootmenu immediately quit instead of moving into the next entry.
> > > > > > > 
> > > > > > > Thanks for testing this.
> > > > > > > 
> > > > > > > Is there a way for me to test this with sandbox? Or does your Nokia
> > > > > > > test check it?
> > > > > > 
> > > > > > I guess that bootmenu command could work in sandbox. But I have not
> > > > > > tried it.
> > > > > > 
> > > > > > Nokia CI test does not try any terminal, keyboard or VGA interaction, so
> > > > > > broken rendering or broken keyboard input is not caught by CI.
> > > > > > 
> > > > > > But it is possible to test it manually. See U-Boot documentation how to
> > > > > > run Nokia u-boot image in qemu. Bootmenu is automatically started.
> > > > > > https://u-boot.readthedocs.io/en/latest/board/nokia/rx51.html#run-in-qemu
> > > > > 
> > > > > I tried to follow this but got stuck here:
> > > > > 
> > > > > ./configure --enable-system --target-list=arm-softmmu --disable-werror
> > > > > 
> > > > > ERROR: Cannot use 'python', Python 2.4 or later is required.
> > > > >         Note that Python 3 or later is not yet supported.
> > > > >         Use --python=/path/to/python to specify a supported Python.
> > > > > 
> > > > > Python 2 has been deprecated for years and I think it was removed recently.
> 
> So install all required dependencies? It is really stupid argument to
> say that _I have removed X from my PC_ and then _I cannot compile Y
> because it depends on X_.

We're not talking about adding some random but modern dependency. We're
talking about adding a language that every person responsible for it
says to not use and migrate away from.  This is Heinrich's point.

> > Our Docker image uses Ubuntu 22.04 (Jammy). Ubuntu 22.10 (Kinetic) was
> > the last Ubuntu release providing Python 2.7. So when upgrading our
> > Docker image next year we will have to quit emulating that 14 year old
> > device.
> 
> Why to quit? Why you cannot compile and install required dependency?

We'll have to quit because Python 2.7 won't be available from the
distribution.  And no, we don't want to add building Python 2.7 to the
list of things our container does. I don't even know that Python 2.7
will compile with the gcc and related that will ship in the host. And
that's without repeating what I said above about Python 2.7 being a dead
and do not use language.

> Also was not the whole purpose of using containers to make easier of
> using different languages in different versions and also different
> software?

No, the point of containers here is to give everyone a common and
reproducible base environment. We don't want to have to have a special
container just for this one test case, which is what it's looking like
would end up being a solution.

> Also what is the problem with 14 year old device and software? It is
> _working_. Or are you saying that you do not want to support _working_
> device just because it is _old_? What is the logic here?

It only works so long as everything else is still available that it
requires.  Which is starting to not be the case in the near future. It
would be better for everyone is this platform was part of modern QEMU
instead.

> > > > Ach :-( Is not there some configure option to disable python?
> > > 
> > > I don't know, but the qemu you are using seems to be 10 years old?
> > 
> > The Nokia N900 support never existed in upstream QEMU. Is is only in a
> > Linaro repository that has not been update since 2015. (No clue why our
> > CI uses the 2013 version and not the 2015 one.)
> 
> We are using the latest version which contains working n900 emulator. It
> is written also in the u-boot documentation.

Is it on anyones TODO list to move this emulation to a modern release?

> > I anybody cares about N900 being used in U-Boot's CI he should rework
> > the N900 patch from the Linaro QEMU archive and get it integrated into
> > upstream QEMU.
> 
> Why? Emulator was already written and it is _working_. Why to invest new
> time for writing a new emulator?
> 
> I think you completely missed the point here. U-Boot is being test in
> this emulator to ensure that changes in u-boot does not break something.
> In the past it already caught issues which were not uncovered by any
> people review and neither by any other CI tests. Also for things which
> are not n900-related.

I think you're missing the point which is that it will very soon be much
harder to support this emulator in CI due to it not being actively
maintained.

> > > > In U-Boot CI is qemu compiling without issues (but variant without GUI).
> > > 
> > > You mean that it still uses python 2? But for how much longer?
> > > 
> > > I don't think it is reasonable to base a test on an old version of qemu.
> 
> Why not? U-Boot should work on any compatible hardware, so also on older
> qemu version.
> 
> Released hardware is _not_ being changed, so any software testing should
> use also older emulators, which are bound to the release date of
> hardware to better match hardware testing.

It'll become a problem when we cannot build the emulator anymore.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230624/afd085ea/attachment.sig>