Refactor MCU package build configuration#854
Refactor MCU package build configuration#854hamish-milne wants to merge 12 commits intocnlohr:masterfrom
Conversation
451d645 to
077ac22
Compare
| "sh", | ||
| join("ch32fun", "parse_mcu_package.sh"), | ||
| chip_name.upper() | ||
| ], text=True).strip().splitlines() |
There was a problem hiding this comment.
Invoking sh is not portable for Windows, hency why the Makefile logic was duplicated in Python for PlatformIO here.
There was a problem hiding this comment.
This is a good point that we discussed on Discord - the new script installs windows-build-tools from xpack (which also owns the gcc distribution we use), which includes sh inside busybox alongside make. Since this makes the separate make dependency redundant, the total package count is the same.
There was a problem hiding this comment.
That means I'll have to also pull in sh as a new PlatformIO tool into https://github.com/Community-PIO-CH32V/platform-ch32v, which previously was not a dependency thanks to the code duplication here, adding onto the number of requirements for just the linker script generation-
There was a problem hiding this comment.
In general, I'm a big fan of dropping dependencies instead of introducing new ones. Why can't the new ch32fun/parse_mcu_package.sh file be equally expressed in pure Makefile syntax instead of requiring bash? Is it because a switch-case is more cumbersome to write? We could still have refactored Makefile logic to generate these newly, better named macros, and the new .ld template, without having to introduce bash, no?
There was a problem hiding this comment.
There's two reasons I went with this approach:
- It makes this logic independent of the build system, allowing consumers to use cmake, xmake, meson etc. without much difficulty. Putting everything in the makefile limits this to, by definition, GNU make.
- While it is possible to do, the syntax in make is a fair bit more cumbersome, since it wasn't really designed for this sort of thing.
Ultimately there's a tradeoff between portability and flexibility here. While I'd personally be fine to use Python as the portable language for this, which would fit in better with PlatformIO, doing this would naturally impose Python as a dependency for other users. With all that in mind, the most lightweight option possible for this sort of task is Posix sh, which even on Windows can be satisfied by a ~650kb portable exe.
| shell: pwsh | ||
| run: | | ||
| ./misc/install_xpack_gcc.ps1 -SkipPrompts | ||
| cat ./misc/xpacks.json -Raw | ./misc/xpm_lite.ps1 -SkipPrompts |
There was a problem hiding this comment.
Removal of ./misc/install_xpack_gcc.ps1 will also neccesitate adaption of Wiki doc (https://github.com/cnlohr/ch32fun/wiki/Installation#windows).
There was a problem hiding this comment.
A good point - this is one reason why my proposal is to merge the Wiki pages into the main repository, making it much easier to keep this kind of thing in sync.
|
Are you willing to push this forward? I am really interested in this. Sorry it took a while to get around to it. |
Yes, of course - apologies, I've had quite a busy few weeks myself! I'll rebase and push a new version that includes |
A number of compile flags and other parameters of the build are controlled by the
TARGET_MCUandTARGET_MCU_PACKAGEparameters. At present, these parameters are fed into a parser function built inmake, generating more outputs which are used as inputs for the linker script pre-processor.This change accomplishes a few things:
.envfile which can be loaded into GNU Make, CMake, or XMake with minimal effort. Lua and Python were also considered for this, but I wanted to avoid any additional build dependencies if possible.TARGET_MCUis now derived fromTARGET_MCU_PACKAGE, instead of being a separate, disjoint input. Compatibility with existing makefile projects should be fine unless there's something very weird being done.Incidentally, I also added a third case for
PREFIX_DEFAULTto match the compiler name used by Nix and XPack.