Daily Pi ROM

The Menu bug was reported over a year ago so it’s frustrating and somewhat concerning we’re still having to work around it. Why wasn’t the underlying change rolled back immediately?

Has the original developer walked away, or is the issue wider than a simple code submission?

Does anyone know the code fix required? Is it a one-liner or a complete rewrite?

Why wasn’t the underlying change rolled back immediately?

I’d imagine you’re not the only one asking that. It would seem logical that if a problem is identified, the offending change gets rolled back until a fix is available.

Maybe it’s harder in Git? Maybe there are side effects? I don’t know…

The Menu bug was reported over a year ago

So you reported a bug over a year ago in some code not written until January 2023? That’s impressive clairvoyance! I’m sitting with my fingers on my temples right now, could you divine to me the winning lottery numbers for 11th May 2024 when you’re ready…

In reality, I suspect you may still be confused about the bug I fixed for you following that report (whose symptoms may have been in the Menu module, but the source code was elsewhere).

OK, so I have not been paying too much attention. Is there a simple re-cap/digest on this?

What are the steps to reproduce the issue? (in, I assume, Menu 0.41)

Here’s the history of Menu:
https://gitlab.riscosopen.org/RiscOS/Sources/Toolbox/Menu/-/commits/master/

This is the change where the version number went from 0.40 to 0.41
https://gitlab.riscosopen.org/RiscOS/Sources/Toolbox/Menu/-/commit/df1861f92839c79bdabcd8cb693f2c8fab06e2fe

However, I suppose the issue could have crept in anywhere in those five commits between 0.40 and 0.41?

Is there a simple re-cap/digest on this?

Search this forum for ‘0.41’.

you may still be confused about the bug I fixed for you following that report

Of course you are correct, but in my defence I did go on to report the bug in Menu 0.41 in that same thread on Jan 23th, so 6 months agoish not 12 – I just got confused about which post in that thread was specific to Menu 0.41 as the whole thread is related to Menu issues.

Is there a simple re-cap/digest on this?

Menu 0.41 has a bug where sub-menus do not appear. My Repro was to use FTPc and trying to open a sub-menu on a file, but I’m guessing you can probably reproduce the issue on any App that uses Toolbox for sub-menus.

OK, so I’ve used PackMan to install SimpleGit.

set UnixEnv$sgit$sfix ""
sgit clone <a href="https://gitlab.riscosopen.org/RiscOS/Sources/Toolbox/Menu.git">https://gitlab.riscosopen.org/RiscOS/Sources/Toolbox/Menu.git</a> /Dev/Menu

Wowsers. I’ve got the source code! I tried running !MkRam but I get ‘File amu_machine not found’.

Is this because I don’t have a full set of RISC OS source code downloaded and ‘prepared’? Is my usual (admittedly a bit old) DDE not sufficient?

Well, Textile has made a total mess of that command, despite me trying code blocks and the pre tag.

Actually, scratch that. I downloaded a TAR of all the sources and ran !Prepare and !Builder. Then when I ran !MkRam again, I no longer get the amu/-machine error. Woo!

However, I get an error from AMU saying ’Don’t know how to make CLIB:o.stubs’. I’ve missed something, haven’t I?

However, I get an error from AMU saying ’Don’t know how to make CLIB:o.stubs’. I’ve missed something, haven’t I?

Builder’s Export phases? Having done those, and with a WimpSlot of 4MB, Menu !MkRam does the build.

Ah, yes. How silly of me. Done that now. Ignored lots of errors (!) and with a bigger wimp slot, I can indeed run MkRam and get Menu 0.41 built. Marvellous.

I have also ran FTPc, connected to a site, and observed that opening the menu works, but it fails when you try to slide-off the first sub-menu (File/Dir). The other slide-off items work OK, like Options, so it’s a bit strange!

OK, so I’ve been playing about with the latest commit, and I can confirm that my fixed version allows the submenu to open in FTPc. Funny, having now read some of the history, I’m pretty much in the same place you were in April, David! ;)

What I don’t understand, probably because I’ve only looked at it for 20 minutes, is the purpose of the change. What extra case(s) is it trying to cover? I wondered if it was about the first and/or last menu items opening submenus?

I used ResTest to create a test case: window with menu and some submenus. They work OK with my fixed version. But they also work in the (broken) ROM version! So there is something more subtle going on here, or ResTest is a massive fraud. There I was thinking that the sensible thing to do would be to make some tests…

EDIT: Ah yes, ResTest is a fraud. I opened the FTPc Res file in there, with the broken Menu module, and the menu works! So I cannot rely on that for testing.

The issue looks to be associated with the dynamic creation of the menu immediately before it is shown. ResEd shows an event is delivered before showing, if that is removed the bug goes away, and so also will any menu changes. The menu creation seems to have gone off on some endless loop. It is only some instances though. That is as far as I got.

We’re similarly stuck, I guess. Never mind.

To complete my learning… I am now struggling with sgit on RISC OS.

If I do:

sgit config --global user.name "Richard Walker"

I get:

libgit error (2): failed to stat '/SDFS::RISCOSpi.$.!BOOT.Choices/.gitconfig': Input/output error

I did try manually creating a suitable file in Choices (called /gitconfig) but that cannot be read when I try to make commits.

Not got anywhere searching the Internet. Looks like a filename conversion issue to me?

libgit error (2): failed to stat ‘/SDFS::RISCOSpi.$.!BOOT.Choices/.gitconfig’: Input/output error

I am suspicious of those slashes in the file name, are they simply reporting artefacts or are they really there. Hmm!!

It’s like some code has evaluated Choices$Write and decided to surround it in slashes. Adding the ‘/gitconfig’ file myself made chuff-all differences, so I don’t think it’s even attempting to read from that name. Is there some sort of UnixLib configuration I can twiddle?

Is anyone using simpleGit on RISC OS?!

OK, so in the interest of moving things on, I’ve caved in and managed to copy the modified file to my MacBook, which meant I could use the GitLab IDE to make the Merge Request. Not the best way, but it’s a way!

I cannot figure out how to assign the MR to ROOL, which I believe I am supposed to do.

Anyway, here it is in all its glory:

https://gitlab.riscosopen.org/RiscOS/Sources/Toolbox/Menu/-/merge_requests/3

I looked at the code for that menu_showing() function.

First, don’t the SWI names need their X form it their return value is tested? Anyway _swix could be used instead.

Line 371-373: the check can be moved above the Wimp_GetMenuState call as there is no need for that call in that case.

Line 368: !menu_intb not needed as already tested in line 355

I also wonder why the test is “while (*(pposition + 1) != -1)” and not “while (*pposition != -1)”. Don’t you miss out the last level in this case? This would also remove the need for the test in line 368.

Edit: if you ignore the test against global_menu.current, those purpose in not known, the changes made to the loop in 0.41 seem to be intended to fix exactly this issue of missing out the last level.

That’s the sort of observation that could really help the bounty developer make progress. I do hope they make progress – and feel able to seek out more help on the forum if they need it!

Andre,

I wondered about the SWI X-ness, but look at the rest of the file. It’s similar.

Surely line 367 has checked the beginning of pposition, so the while loop should indeed start with the +1. It reads to me like the first item wasn’t being considered (in the new code).

It’s like 403 I am not 100% sure on.

I would feel more confident if it was tested. Surely that’s the ultimate proof? Is there a Res file floating about with all the possible cases, and we can stick thet into a simple app and give it a go? If there isn’t one already, then maybe that’s the next step… to make something?

Just to bring this to a close. ROOL have assigned me to the relevant group, and I can now assign the MR to them. So the gitlab journey makes sense to me now.

However, the issue at hand, the Menu bug… I’m told that the MR is unlikely to be accepted, and discussions are (still) ongoing with the bounty developer. See https://www.riscosopen.org/content/documents/bounties and the entries dated August 2023. So I’m not going to bother assigning the MR to ROOL, and for now, I’ll just leave it. I’ll withdraw the MR at some point – perhaps when the bounty author submits the official solution. In the meantime, anyone using a test ROM just has to lump it.

(O.T.-ish. And then I got caught out some more by the hidden !Boot thing, how to install a Pi ROM in (hidden) Loader. The Pi ROM does not have to be in !Boot either. Another Obey file is called for.)

I believe this update to the configure plugin should handle the case of Choices being relocated (or hidden) as !Choices automagically. It just reads Choices$Dir on startup instead of assuming it’s at !Boot.Choices all the time.

I believe this update to the configure plugin should handle the case of Choices being relocated (or hidden) as !Choices automagically. It just reads Choices$Dir on startup instead of assuming it’s at !Boot.Choices all the time.

It does indeed steer Boot merges into the active Choices. Thanks, perfect.

I have not been paying sufficient attention, I had not noticed it had not yet been merged, an HD4Dev build from the sources was done to test this.

More closure! The ‘daily Pi ROM’ (as per subject) now has Menu 0.42, which fixes the bug. So good news all round. Congrats to all involved.

I would advise anyone using a nightly ROM to update.

@Richard Walker

Don’t forget to close your merge request attempting to fix the above bug.