[PATCH] menu: Re-enable the ANSI codes

Sun Jun 25 17:26:34 CEST 2023

On Saturday 24 June 2023 13:05:11 Tom Rini wrote:
> 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

So just say it loudly and do not hide your real reasons.

"We have there a bug in code and CI can show it. So we have to drop
tests from CI, to make sure that CI always pass and do not report this
bugs".