Skip to content
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

Merged
merged 67 commits into from
Jan 11, 2024

Conversation

perigoso
Copy link
Contributor

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

  1. 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

  2. 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 work

I 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

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

replaces #1033

@perigoso perigoso force-pushed the feature/meson branch 2 times, most recently from 6ef88ae to 359eb4f Compare June 29, 2023 22:44
@perigoso perigoso mentioned this pull request Jun 29, 2023
@perigoso perigoso force-pushed the feature/meson branch 8 times, most recently from d6bf7db to 2deb9e9 Compare June 30, 2023 00:50
@perigoso perigoso force-pushed the feature/meson branch 3 times, most recently from 81acd3a to 427f27c Compare July 28, 2023 18:11
@dragonmux dragonmux added the Enhancement General project improvement label Jul 28, 2023
@perigoso perigoso mentioned this pull request Aug 10, 2023
43 tasks
@dragonmux dragonmux added this to the v2.0 release milestone Oct 28, 2023
@dragonmux
Copy link
Member

Please rebase this on main when you get a chance and we'll get reviewing it.

@perigoso perigoso force-pushed the feature/meson branch 6 times, most recently from 5018bc2 to 607b992 Compare November 11, 2023 03:42
Copy link
Member

@dragonmux dragonmux left a 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.

src/Makefile Outdated Show resolved Hide resolved
meson_options.txt Outdated Show resolved Hide resolved
.github/workflows/build-pr.yml Outdated Show resolved Hide resolved
src/target/meson.build Outdated Show resolved Hide resolved
src/target/meson.build Show resolved Hide resolved
@dragonmux dragonmux force-pushed the feature/meson branch 3 times, most recently from fe11c0c to d934b21 Compare November 28, 2023 15:51
Copy link
Contributor

@amyspark amyspark left a 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.)

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
UsingRTT.md Outdated Show resolved Hide resolved
UsingRTT.md Outdated Show resolved Hide resolved
Copy link
Contributor

@amyspark amyspark left a 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 .

src/platforms/hosted/README.md Outdated Show resolved Hide resolved
src/platforms/hosted/README.md Outdated Show resolved Hide resolved
src/platforms/hosted/README.md Outdated Show resolved Hide resolved
src/platforms/hosted/README.md Outdated Show resolved Hide resolved
src/platforms/hosted/README.md Outdated Show resolved Hide resolved
src/platforms/hosted/README.md Outdated Show resolved Hide resolved
@dragonmux dragonmux force-pushed the feature/meson branch 2 times, most recently from c24d139 to 81bed42 Compare January 11, 2024 20:50
@dragonmux
Copy link
Member

There, all the review items should now be addressed and the merge conflict resolved. Over to you please Esden!

Copy link
Contributor

@amyspark amyspark left a 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 .

Copy link
Member

@esden esden left a 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!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
perigoso and others added 2 commits January 11, 2024 22:46
…produced binaries instead of make

Co-Authored-By: dragonmux <[email protected]>
@esden esden self-requested a review January 11, 2024 22:58
Copy link
Member

@esden esden left a 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

@esden esden merged commit e68c378 into blackmagic-debug:main Jan 11, 2024
17 checks passed
@perigoso perigoso deleted the feature/meson branch January 12, 2024 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement General project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants