This merge request doesn't seem to have attracted any love for 6+ months, so I'll dare to look at it.
The defmod code changes in themselves are all nice and bland, but I am going to query the rash of YAML (same goes for RiscOS/Sources/Apps/SquashApp!1 and RiscOS/Tools/Sources/Squish!1 so an opportunity for 3 birds with 1 stone here) that the deploy step looks pretty fragile. By which I mean: it's got lots of magic paths baked into it that I could imagine could be subject to the whims of GitLab in major version bumps, various sideband back scratchery, and other incantations.
So as to avoid having to keep reworking numerous components when the winds change direction, could all that stuff be swept into a single place then merely referenced here? I don't know much about YAML but I'd guess it supports multiple include
files so you might end up with
include:
- project: 'Support/CI'
file: '/defmod.yml'
deploy:
file: 'CD/defmod.yml'
And all the scripts then live in Support/CD like the CI scripts live in Support/CI
If multiple include
files aren't supported then next option would be to have the scripts live in Support/CD and have the refresh.awk
notice they exist and append them so you have
include:
- project: 'Support/CI'
file: '/defmod.yml'
The power of the shared makefiles and the autogenerated CI scripts allowing 1 change to be made centrally and all the clients inherit that is the pattern I think would work well here.
Based on !1 in order to avoid introducing merge conflicts when rebasing back and forth from current master branch, so merge that first.
ROOL (8bd0deea) at 03 Sep 10:17
Remove redundant source files
= Hdr:System
The name of the assembler binary used to generate object outputs was defined by the preprocessor macro ASMCMD: for native builds, it was "ObjAsm" and for cross-compilation builds, it was "armasm". Extend this so that the name of the assembler can be overridden at run time by the environment variable also called ASMCMD. This permits the tool to generate ELF format object files for use with GCCSDK.
ROOL (dd4cecde) at 20 Aug 10:04
Permit selectable assembler backend
ROOL (be706d9f) at 20 Aug 09:52
Rewrite makefile to use CApp, and support building for Unix
Deploys both a native binary to RiscOS/Library and a cross-compiling one to the GitLab runner machine.
Fix issues identified by CI:
Requires RiscOS/Sources/Lib/OSLib!2.
The name of the assembler binary used to generate object outputs was defined by the preprocessor macro ASMCMD: for native builds, it was "ObjAsm" and for cross-compilation builds, it was "armasm". Extend this so that the name of the assembler can be overridden at run time by the environment variable also called ASMCMD. This permits the tool to generate ELF format object files for use with GCCSDK.
Based on !1 in order to avoid introducing merge conflicts when rebasing back and forth from current master branch, so merge that first.
Looks good, though it's a shame we've been duplicating effort somewhat - I updated the makefile in a remarkably similar way here. That's been hanging around for over a year, waiting for various MRs for other components to get through the review process. Never mind, it's probably just as well to break my changes into multiple smaller MRs anyway - I'll refactor my remaining changes on top of yours.
Comparing our respective makefile changes, the main thing that stands out to me is that I preserved the distinction from the original makefiles whereby the native makefile defined -DTRACE=0 -DYYDEBUG=0 -DYYMAXDEPTH=0
and the cross-compilation makefile didn't. Looking at those individually:
TRACE
appears only to be used by files derived from the Support library, so has no effect on cross-compilation builds. Your new makefile never defines TRACE
, but that looks OK as I think it probably always gets set to 0 if undefined anyway.YYDEBUG
also defaults to 0 if unset, so -DYYDEBUG=0
effectively has no effect.YYMAXDEPTH
is a bit more problematic, as it defaults to 10000 if unset. Setting it to 0 just appears to make it more likely to report a "memory exhausted" error. Unless anyone has any better ideas, my best guess is that this was a memory use optimisation for when running on RISC OS? That would be irrelevant for OSes that do allocate-on-write, which could explain why it was only set for native builds. Given that RISC OS machines tend to have plenty of memory these days, perhaps it's time to leave YYMAXDEPTH at its default setting unconditionaly now?