[PATCH] menu: Re-enable the ANSI codes

Sun Jun 25 18:25:28 CEST 2023

Am 25. Juni 2023 17:26:34 MESZ schrieb "Pali Rohár" <pali at kernel.org>:
>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".

The suggestion in the thread was: Whoever is interested in the CI running the N900 emulation should upstream it to mainline QEMU.

Best regards

Heinrich