-
-
Notifications
You must be signed in to change notification settings - Fork 777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: experimental meson buildsystem #1528
Conversation
6ef88ae
to
359eb4f
Compare
d6bf7db
to
2deb9e9
Compare
81acd3a
to
427f27c
Compare
Please rebase this on main when you get a chance and we'll get reviewing it. |
5018bc2
to
607b992
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some initial review notes - we're going to work at reviewing this PR in steps till we've covered all the changes properly.
Very excited to be getting this reviewed and hopefully soon also merged. There's a lot of promise here and improved build system discoverability + ease of use. We're also happy to see fixes for several things that had been niggling us too in the Makefile build system.
f2759c2
to
4a0bae7
Compare
fe11c0c
to
d934b21
Compare
…pecific cross-file
…DEBUG` (BMDA) the ROM table info print would not be included in a debug build under Meson This actually hints at a broader bug in how that subsystem is currently implemented, which needs addressing seperately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi again, hopefully you won't mind a few extra nits. (I'd not noticed the UsingRTT.md file.)
9d9e7d6
to
5ff9e1a
Compare
Co-Authored-By: dragonmux <git@dragonmux.network> Co-Authored-By: amyspark <amy@amyspark.me>
774d477
to
bc77076
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional bits for @dragonmux .
c24d139
to
81bed42
Compare
There, all the review items should now be addressed and the merge conflict resolved. Over to you please Esden! |
81bed42
to
79c20c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
READMEs look good to me 👍 @dragonmux .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two very minor comments. Otherwise it all looks really good!
…produced binaries instead of make Co-Authored-By: dragonmux <git@dragonmux.network>
… to fix a lot of factual errors and issues
79c20c6
to
9d6955d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! This looks great! Thank you everyone for all the hard work making this build system revamp a reality!!! :D
Detailed description
This is a follow up to #1033
After banging my head against the wall that is the build-system once again, I decided to take a new stab at this, the original port was out of date so I ended up redoing it from scratch
This time it is my intent to get this merged
It's an experimental implementation of the build-system for black magic debug firmware in meson build
I made an effort to ensure the original Make build-system continues working with no notable changes, this port is meant to work beside it, as an experimental "feature" developers and experienced users can test and work on until it matures
It's very much experimental, this is a fairly dynamic project and meson does not directly provide convenience functions for some of our use cases like it does for others, this means there are multiple ways we could achieving the desired result (without a "recommended" way), this is my second iteration, the original port remain in the original PR I submitted #1033 if you wish to reference it
The current implementation is fairly functional (I noted experimental above, but that doesn't imply not functional), all probes (with the exception of BMDA 1. and Blackpill-* 2.) are ported and working (compiling, only native was tested), and target support is selectable, most options the Make build system provided were ported over, bootloaders are also available (though some don't compile, but they were not available through make anyway apparently)
Building BMD as a library is not yet implemented, although it is one of the intended use cases for this upgrade
BMDA not supported because BMDA it is to be split into its own project compiled against BMD on v2.0, I chose not to spend the effort porting the target/platform
Blackpill probes are not ported, the way the Blackpill probes support is added does not align well with the other targets, making it harder to port, there is a way to cleanly port them (that's the point of meson, to make this stuff easier), but it would be destructive to the Make build-system, since this is an experiment and I want to keep the current build-system fully functional, I will hold on that port in a separate PR for now
Libopencm3 was added as a sub-project, because like the original BMP buildsystem, its build system is... rudimentary, there's no easy way to integrate into meson directly, and because there is some code generation porting it to meson (the ideal approach) is not trivial, I started on it but opted for a easier hackier approach, if someone is interesting in doing the port they are welcome to do so (and I would be thankful), the approach I opted for was to use the meson external project module, this is meant to allow integrating external build systems with meson, but they need to align with the
configure -> make
workflow and allow out of tree builds, which this does not, hence the hacks included in the PR to make it workI tried to be thorough, with comments, consistent style, and generally a clean implementation, "tried" is the keyword here, I'm very much welcome to feedback
I followed the meson style guide (it's not very comprehensive), with the exception of tabs vs spaces, I opted for tabs to maintain consistency with the rest of the code base, and align with personal preference (and dragonmux's too)
I added copyright headers to all build-system files, it was a last minute call, I don't know if it's appropriate, I will leave that to the official maintainers discretion
One note right now the probe/platform configuration is done through project options, there is an alternative approach I like, using the cross-file to declare everything probe/platform related, this gives more purpose to the cross-file and makes it's use clear (less of just something to remember you have to do, with less experienced users not understanding why they have to do it (we can't make meson fetch the correct cross-file automatically, expressly undesired feature from the devs, understandably so if you understand the build-system, but in some cases it would be reeeally useful)), but this means less integration with the meson configuration and options, which provide some useful functionality, and it would cause some code duplication, I'm torn, but for now I am happy with this
How to test
Because meson overlays some files on libopencm3, it's easiest to deinit and remove the submodule before
git submodule deinit --all
rm -r lib/libopencm3
Create a build directory
mkdir build
Configure the build (cross-file provides the cross compilation toolchain)
meson setup build/ --cross-file crossfile/arm-none-eabi.ini
By default this will configure for the native probe, we can configure other with (note that an invalid probe will print an informative error)
meson setup build/ --cross-file crossfile/arm-none-eabi.ini -Dprobe=stlink
Deploy the ninjas to build (we can also cd to the build dir and setup/build from there ofc)
ninja -C build
To build the bootloader do
ninja -C build bootloader
All binaries can be seen on the top level of the build directory
To flash the connected probe from the build system with the latest build ;) run
ninja -C build flash
Your checklist for this pull request
make PROBE_HOST=native
)make PROBE_HOST=hosted
)Closing issues
replaces #1033