Change how top-level phony rules are overridden
A common pattern previously in use was for a shared makefile to define a
top-level phony rule (the sort of thing that srcbuild
or a !Mk
file would
use as the target to pass to amu
) but with a macro expansion within the
rule name. This allowed clients to override the rule by assigning any value
to the macro.
However, this approach makes it relatively difficult for clients to be adapted to support cross-compilation. This is because when cross-compiling, the shared makefiles utilise nested invocations of the make tool in order to facilitate an out-of-tree build directory layout. Therefore, clients need to determine whether they are cross-compiling, and if so, whether they are in the context of the inner or outer make invocation, before they can decide whether or not to redefine the phony target. This can be achieved in as little as 3 extra lines:
CURDIR ?= objs
ifeq (objs,$(notdir ${CURDIR}))
# redefinitions of top-level phony rules placed here
endif
but this wasn't universally popular. Instead, taking inspiration from
INSTTYPE
, we can chain from each ${PHONY}
rule to a ${PHONY}_custom
rule
and have the client makefile define the ${PHONY}_custom
rule instead.
Crucially, in the cross-compiling case, the shared makefile can then take
care of ensuring this only happens on the inner make
invocation.
In summary:
When setting CUSTOMEXP
to custom
, clients should henceforth
- define
export_hdrs_custom
andexport_libs_custom
rather than redefineexport
- define
export_hdrs_custom
rather than redefineexport_hdrs
- define
export_libs_custom
rather than redefineexport_libs
When setting CUSTOMROM
to custom
, clients should henceforth
- define
install_rom_custom
rather than redefineinstall_rom
- define
rom_custom
rather than redefinerom
- define
rom_link_custom
rather than redefinerom_link
When setting CUSTOMSA
to custom
, clients should henceforth
- define
install_custom
rather than redefineinstall
- define
standalone_custom
rather than redefinestandalone
CUSTOMDBG
and CUSTOMGPA
are now removed. These were not being used by
any client makefiles, but it seems probable that the rules they related to
(debug
and gpa_debug
respectively) would have needed overriding in
precisely the same circumstances as install
and standalone
, so in
future CUSTOMSA
should be used for these also.
Merge requests have also been raised for all the affected client makefiles to prepare them for this change. These should be merged first:
- RiscOS/Sources/FileSys/ADFS/ADFSFiler!6 (merged)
- RiscOS/Sources/Networking/AUN/AUNMsgs!2 (merged)
- RiscOS/Sources/Networking/Ethernet/EtherCPSW!4 (merged)
- RiscOS/Sources/Networking/Ethernet/EtherGENET!3 (merged)
- RiscOS/Sources/Networking/Ethernet/EtherK!1 (merged)
- RiscOS/Sources/Networking/Ethernet/EtherUSB!4 (merged)
- RiscOS/Sources/Networking/Ethernet/EtherY!1 (merged)
- RiscOS/Sources/FileSys/FSLock!5 (merged)
- RiscOS/Sources/Networking/Fetchers/Gopher!2 (closed)
- RiscOS/Sources/SystemRes/InetRes!24 (merged)
- RiscOS/Sources/Kernel!69 (merged)
- RiscOS/Sources/Internat/Territory/TerritoryModule!2 (merged)
- RiscOS/Sources/Toolbox/ToolAction!3 (merged)
- RiscOS/Sources/HWSupport/USB/USBDriver!11 (merged)
- RiscOS/Sources/Desktop/Wimp!35 (merged)
Once this MR is merged, amu
will start to emit warnings when building each of them about redefinition of the affected phony targets in those client makefiles, however, those are harmless and can safely be ignored. It is proposed that as and when each of those makefiles is updated to support cross-compilation, the redundant phony target rules be removed.
A ROM build has been completed successfully to ensure the merge requests work in conjunction.
mentioned in commit bavison/ADFSFiler@5086c6a3
mentioned in commit bavison/AUNMsgs@57811bba
mentioned in commit bavison/EtherCPSW@122d7550
mentioned in commit bavison/EtherGENET@d56a3e26
mentioned in commit bavison/EtherK@d4271ffc
mentioned in commit bavison/EtherUSB@9caa4883
mentioned in commit bavison/EtherY@71bb499b
mentioned in commit bavison/FSLock@665714b2
mentioned in commit bavison/Gopher@8a1bd3b1
mentioned in commit bavison/InetRes@cb82fa3c
mentioned in commit bavison/Kernel@8ec374a7
mentioned in commit bavison/TerritoryModule@3645fbec
mentioned in commit bavison/ToolAction@363f8acf
mentioned in commit bavison/USBDriver@398f7311
mentioned in commit bavison/Wimp@54c93e2f
mentioned in commit bavison/Kernel@417a998d
mentioned in commit bavison/Kernel@1be54f21
mentioned in commit bavison/ToolAction@fc5c0275
mentioned in commit bavison/USBDriver@b67c01a2
mentioned in merge request RiscOS/Sources/FileSys/ADFS/ADFSFiler!6 (merged)
mentioned in merge request RiscOS/Sources/Networking/AUN/AUNMsgs!2 (merged)
mentioned in merge request RiscOS/Sources/Networking/Ethernet/EtherCPSW!4 (merged)
mentioned in merge request RiscOS/Sources/Networking/Ethernet/EtherGENET!3 (merged)
mentioned in merge request RiscOS/Sources/Networking/Ethernet/EtherK!1 (merged)
mentioned in merge request RiscOS/Sources/Networking/Ethernet/EtherUSB!4 (merged)
mentioned in merge request RiscOS/Sources/Networking/Ethernet/EtherY!1 (merged)
mentioned in merge request RiscOS/Sources/FileSys/FSLock!5 (merged)
mentioned in merge request RiscOS/Sources/Networking/Fetchers/Gopher!2 (closed)
mentioned in merge request RiscOS/Sources/SystemRes/InetRes!24 (merged)
mentioned in merge request RiscOS/Sources/Kernel!69 (merged)
mentioned in merge request RiscOS/Sources/Internat/Territory/TerritoryModule!2 (merged)
mentioned in merge request RiscOS/Sources/Toolbox/ToolAction!3 (merged)
mentioned in merge request RiscOS/Sources/HWSupport/USB/USBDriver!11 (merged)
mentioned in merge request RiscOS/Sources/Desktop/Wimp!35 (merged)
added 7 commits
-
1ae18252...eef021dc - 6 commits from branch
RiscOS:master
- bfb5a787 - Change how top-level phony rules are overridden
-
1ae18252...eef021dc - 6 commits from branch
added 3 commits
-
bfb5a787...b1dfe962 - 2 commits from branch
RiscOS:master
- fe34c406 - Change how top-level phony rules are overridden
-
bfb5a787...b1dfe962 - 2 commits from branch
added 11 commits
-
fe34c406...4de16c09 - 10 commits from branch
RiscOS:master
- 42d2ac8b - Change how top-level phony rules are overridden
-
fe34c406...4de16c09 - 10 commits from branch
The reason I marked this MR as a draft was to ensure it couldn't be accidentally merged before any of the MRs it depends upon were merged. At the time of writing this comment, none of those have been merged yet (though to be fair, only the ADFSFiler one was assigned to @rool, because I hoped to avoid having any discussion spread across multiple MRs).
This MR changes things so the shared makefile always defines rules such as
install
. It's then the shared makefile's responsibility to decide whether to chain onto aninstall_custom
rule (and similarly for other such rules).There are two aspects to the changes this requires of client makefiles. One is that they must start defining
install_custom
(etc). The other is that they should stop defininginstall
(etc). These two aspects do not necessarily have to happen at the same time; in fact, not doing so grants us a degree of flexibility. Not only do we not then need to merge MRs for all the other 14 components identified in the description above on the same day as this one in order to avoid breaking the overnight builds, but changing both aspects simultaneously would make it significantly harder to perform regression testing on any of the affected components at a later date (i.e. whenever updating any one of them across this API-change boundary, in either direction, you'd need to update all the other 14 as well).It's perhaps worth noting that there is another group of client makefiles that redefine top-level phony rules, but do so before they include a shared makefile, not afterwards. Amu will continue to treat such makefiles the same both before and after this MR is applied, so there is no dependency relationship with those components.
mentioned in commit bavison/FSLock@6720dd19
mentioned in commit bavison/InetRes@c81149f4
mentioned in commit bavison/Kernel@295aee5f
mentioned in commit bavison/USBDriver@6ab4f288
mentioned in commit bavison/Wimp@402a9c1d
At the time of writing this comment, none of those have been merged yet (though to be fair, only the ADFSFiler one was assigned to @rool, because I hoped to avoid having any discussion spread across multiple MRs).
Please try to follow these guidelines for submission rather than trying to be clever: "Make sure you select ROOL as the assignee, or nobody will know to look at your merge request!"
Of the related affected components
- FSLock
- Kernel
- USBDriver
already had a non-backwards compatible change when the HostApp fragment was introduced, so would only gain 8 months of possible regression grace, which is unlikely to be useful, and - ToolAction
already had a non-backwards compatible change when the Toolbox #includes were changed, so would only gain 7 months of possible regression grace, which is unlikely to be useful, and - Ether*
only use the rules for the disc install of AutoSense which a developer is unlikely to exercise manually.