A pull request (to be discussed)

Using, learning, programming and modding the Gigatron and anything related.
Forum rules
Be nice. No drama.
lb3361
Posts: 367
Joined: 17 Feb 2021, 23:07

A pull request (to be discussed)

Post by lb3361 »

Because I believe in collaboration: https://github.com/kervinck/gigatron-rom/pull/248

This is a combined pull-request that could be split as needed.

- The first commit upgrades GLCC to the latest version

- The next seven commits (up to 78f3021) are a ROMv6 release. The native code itself is essentially dev rom, that is, an approximation of what ROMv5 could have been (bugs fixed plus good support for expansion cards). The application selection removes pictures and tetronis, adds at67
s invaders (pristine gt1) and hans61's gtmine. New credits application gives credits for all these applications. At this point the dev rom is the same as ROMv6 save for romTypeValue.

- The last tree commits upgrade the dev rom to dev7, without changing the application selection other than a slightly faster Mandelbrot (found another 25%) and having a version of MSCP in Apps (but not included in the ROM)

All three items can be discussed independently to best satisfy the wishes of our community.
I'll update the pull request accordingly.

The pull request does not provide the dev rom binaries but they can be found in my master branch.
at67
Site Admin
Posts: 647
Joined: 14 May 2018, 08:29

Re: A pull request (to be discussed)

Post by at67 »

Pull request won't be accepted as per https://forum.gigatron.io/viewtopic.php?t=19, I don't have the hours required to review the amount of changes you have made to the root/core.

The simple solution is to refactor it all so that it sits in your Contrib area, as everyone else, (including myself has done), see the above forum thread.
lb3361
Posts: 367
Joined: 17 Feb 2021, 23:07

Re: A pull request (to be discussed)

Post by lb3361 »

Hello at67.

Three points and a proposal:
  • GLCC update: I see no problems maintaining a version of glcc inside my Contrib dir. But this means that one should also revert the commit that creates the subdirectory Compilers, which currently contains only glcc (outdated). I will prepare a new pull request with only this change. We can agree that this is non-controversial.
  • ROMv6: This is in fact a small change because the native code is from the current dev rom. The application changes are mostly trivial and can be revisited. The hardest part is janitorial: making sure everything is versioned correctly, that interface.json is updated accordingly, etc. This makes little sense in a Contrib directory.
  • Dev7: This is a substantial change of native code. It is well tested at this point and known to be very backward compatible. This is why I thought it suitable for a dev rom because, well, a dev rom is tentative, not definitive. But I know this is a big one. It would not be hard to fold this into a Contrib directory, partly because this was intended as an experiment rather than a release.
Behind 2 and 3 lies the point that might be more controversial. Is the official ROM supposed to evolve at all or to remain frozen at ROMv5a? And if it evolves, should it be minimal (e.g. ROMv6 which is essentially a fixed ROMv5a), or should it embrace bolder moves (like augmenting vCPU)? If the official ROM is supposed to evolve, then there must be a broad agreement, which has proven to be harder than we once hoped. If it is not supposed to evolve, then it will become obsolete, raising the question of its replacement. Then we get a landscape of competing and incompatible developments where whoever wins presides on a diminished community. This is not good for an already small community.

My gut feeling is that we should try to agree. The purpose of this pull request is precisely to give my code to the community without strings attached.

I propose to reshuffle the commit as follows:
  • Move GLCC under Contrib/lb3361 and update it there.
  • Keep the ROMv6 changes into the main directories, because (a) it is a small change, (b) it is a fair approximation of what we know Marcel had in mind, and (c) because there is value in providing a stable development target to app developers, with properly versionned syscalls and interface.json. This would improve the life expectancy of the official gigatron rom, which I see as a benefit our community
  • Move all Dev7 developments under Contrib/lb3361.
at67
Site Admin
Posts: 647
Joined: 14 May 2018, 08:29

Re: A pull request (to be discussed)

Post by at67 »

lb3361 wrote: 05 Jul 2023, 16:37 [*] GLCC update: I see no problems maintaining a version of glcc inside my Contrib dir. But this means that one should also revert the commit that creates the subdirectory Compilers, which currently contains only glcc (outdated).
The Compilers folder was mutually agreed to be beneficial to multiple users using both compilers and to have minimal impact on software compatibility. There's no need to revert this change.
lb3361 wrote: 05 Jul 2023, 16:37 [*] ROMv6: This is in fact a small change because the native code is from the current dev rom. The application changes are mostly trivial and can be revisited.
This is how all pull requests to core should be, small, thoroughly tested and community approved. But any changes to core *have* to be 100% backward compatible with all previous ROM applications, this was one of the features that Marcel and I discussed in the early days and one of the things he charged me with when he gave me access to the main repo. This means core changes need to be thoroughly tested and reviewed for backwards compatibility and regressions.
lb3361 wrote: 05 Jul 2023, 16:37 [*] Dev7: This is a substantial change of native code. It is well tested at this point and known to be very backward compatible. This is why I thought it suitable for a dev rom because, well, a dev rom is tentative, not definitive.
Contrib devroms are temporary and for testing, they never belong in core, they belong in each user's Contrib folder as you eventually worked out.

Do whatever you like in your own Contrib folder, (as stated originally by Marcel both in the forums and in the documentation).
lb3361
Posts: 367
Joined: 17 Feb 2021, 23:07

Re: A pull request (to be discussed)

Post by lb3361 »

at67 wrote: 05 Jul 2023, 23:19 The Compilers folder was mutually agreed to be beneficial to multiple users using both compilers and to have minimal impact on software compatibility. There's no need to revert this change.
Then it seems logical to update GLCC in place ?
at67 wrote: 05 Jul 2023, 23:19
lb3361 wrote: 05 Jul 2023, 16:37 [*] ROMv6: This is in fact a small change because the native code is from the current dev rom. The application changes are mostly trivial and can be revisited.
This is how all pull requests to core should be, small, thoroughly tested and community approved. But any changes to core *have* to be 100% backward compatible with all previous ROM applications, this was one of the features that Marcel and I discussed in the early days and one of the things he charged me with when he gave me access to the main repo. This means core changes need to be thoroughly tested and reviewed for backwards compatibility and regressions.
I fully agree with this! This especially important when we are talking about a named rom instead of a devrom. In Marcel's guidelines, a named ROM should only contain permanently versioned apps (that will never change) and properly versioned syscall names published in interface.json. It is easy to forget something. Even Marcel forgot a ROM type check in the v5a cardboot. In fact, thoroughly testing this step was the *only* reason for creating a staged ROMv6 version on my GitHub. After a couple months, I believe I have it correct. Yet it is still necessary that other people also believe it to be correct.
at67 wrote: 05 Jul 2023, 23:19
lb3361 wrote: 05 Jul 2023, 16:37 [*] Dev7: This is a substantial change of native code. It is well tested at this point and known to be very backward compatible. This is why I thought it suitable for a dev rom because, well, a dev rom is tentative, not definitive.
Contrib devroms are temporary and for testing, they never belong in core, they belong in each user's Contrib folder as you eventually worked out. Do whatever you like in your own Contrib folder, (as stated originally by Marcel both in the forums and in the documentation).
Well, Contrib devroms belong to the Contrib directories because they are Contrib devroms, no need to work this out.

I often prefer to do such work in my own repositories because things can change quickly during the development process. You do not want ten Contrib pull requests per day! At some point things get less likely to change. Maybe that's a good time for moving into Contrib? I have a lot of contributions like this, ranging from hardware expansion boards to software for reading SD cards on the SPI port. These things are pretty well tested in fact. Even dev7 is pretty well tested by now and highly compatible. The problem with dev7 is not that it is not well tested, it is that it represents a substantial departure that might not be desirable in the context of Marcel's repository.
at67
Site Admin
Posts: 647
Joined: 14 May 2018, 08:29

Re: A pull request (to be discussed)

Post by at67 »

lb3361 wrote: 06 Jul 2023, 01:56 Then it seems logical to update GLCC in place ?
Yes.
qwertyface
Posts: 68
Joined: 16 Jul 2019, 09:19
Location: UK

Re: A pull request (to be discussed)

Post by qwertyface »

I'm keen to see further ROMs in the "mainline" series. Naturally it's important that additions are well tested, documented, and agreed to be useful by the community (although what the process for that looks like, I don't know), and backwards and forwards compatibility is essential.

If it would be useful, I'd like to volunteer to write some automated functional tests for the mainline series ROMs. I can't promise a timescale, but it doesn't seem too hard to check that most of the vCPU instructions and SYS Functions work correctly (including meeting their timing requirements).
lb3361
Posts: 367
Joined: 17 Feb 2021, 23:07

Re: A pull request (to be discussed)

Post by lb3361 »

at67 wrote: 06 Jul 2023, 11:25
lb3361 wrote: 06 Jul 2023, 01:56 Then it seems logical to update GLCC in place ?
Yes.
I have now reduced this pull request to only the GLCC update, a big commit strictly confined to Compilers/glcc.
More to come for ROMv6 later, in small commits.

qwertyface wrote: 06 Jul 2023, 15:16 I'm keen to see further ROMs in the "mainline" series. Naturally it's important that additions are well tested, documented, and agreed to be useful by the community (although what the process for that looks like, I don't know), and backwards and forwards compatibility is essential. If it would be useful, I'd like to volunteer to write some automated functional tests for the mainline series ROMs. I can't promise a timescale, but it doesn't seem too hard to check that most of the vCPU instructions and SYS Functions work correctly (including meeting their timing requirements).
The proposed ROMv6 (not a release, just a proposal) is visible in my dev repo: https://github.com/lb3361/gigatron-rom (master branch). Useful tests include (a) functional tests (how well does the rom work from the menu), (b) compliance tests (whether interface.json is accurate and whether all the programs have version suffixes, and (c) compatibility tests (how well the rom runs past gt1 files). Note that the native code in this rom is 99.99% similar to the current dev rom. The vCPU implementation has not changed since v5a. Most sys calls are the same as v5a. The changes are new sys calls. The only compatibility issue I know is related to vIRQ handling in v6502 mode, something that is only exercised in the Apple-1 emulator: the oldest versions can occasionally misbehave, the latest version contains a patch to prevent this.
lb3361
Posts: 367
Joined: 17 Feb 2021, 23:07

Re: A pull request (to be discussed)

Post by lb3361 »

Here is a tentative pull-request for a ROMv6, to be scrutinized as needed.

Details of each commit discussed in https://github.com/kervinck/gigatron-rom/pull/249

Two commits of this pull request touch code created by at67 such as adding Invader to the app selection. This is a path-breaking program that, in the context of a mainline rom, showcases both the Gigatron capabilities and at67s contributions.
lb3361
Posts: 367
Joined: 17 Feb 2021, 23:07

Re: A pull request (to be discussed)

Post by lb3361 »

At67 does not wish to see Invader or any of his applications in ROMv6 (see his own words in the pull request; let's abstain from discussing his decision in this thread). This leaves about 12KB of space for a different application. Possible options: Please DM me with your preferences. I'll count the answers, contact authors to obtain permission, and go from there. Note that restoring "Pictures" is not a bad option since this program only runs from ROM. Note also that restoring "Tetronis" may not be an option because this is also an at67 application.
Last edited by lb3361 on 12 Jul 2023, 09:58, edited 1 time in total.
Post Reply