Enable support for other ARM GNU Toolchains, add arm-none-linux-gnueabihf toolchain #30
Conversation
|
Hey, thanks for this contribution, I think it's a good idea to unify some of these toolchains under one roof and submit that to the Bazel registry. Since this repo is getting some traction I was thinking about cleaning it up and uploading. I think your additions are a good thing to get in before submitting to the BCR. Here's some quick answers to your points:
4 and 5, we def agree. I also have a set of RISCV GNU toolchains that we could update and push if you care about embedded dev tools I'll review this PR shortly, but could you take care of fixing the CI issues? |
|
CI issues appear to be resolved now! I also resolved the issues outlined in 4 and 5. Unfortunately rules_cc does not yet define Very good question about the intuitiveness of the APIs! I think that in general they are, at least there isn't any specific way that I would change them, after reviewing Feel free to provide additional comment on my latest changes, but I otherwise feel that this PR is ready to merge! I think what's next for me might be adding support for the |
There was a problem hiding this comment.
Sorry for the delay, this is an excellent PR and it cleans up some of the sharp edges I was worried about.
We just merged a more ergonomic way to handle standard includes by @RyanDraves, if we can rebase on the main branch and include those changes here this should be good to merge (happy to do it given how much work you already added, just let me know if you don't have time)
I think we should add the arm-none-eabi missing versions and then release to Bazel mod before adding support for many other toolchains. I fear scope creep otherwise
| """ | ||
| _arm_gnu_toolchain(name, toolchain = "arm_none_eabi", | ||
| toolchain_prefix = "arm-none-eabi", version = version, | ||
| abi_version = "eabi", copts = ["-nostdinc"] + copts, |
There was a problem hiding this comment.
nit: we just merged a change to optionally disable standard includes (which sometimes are needed for C++ folks), it would be great to use those changes. Happy to patch this otherwise excellent PR if you don't have the time
|
No problem at all! I'll address the C++ standard library includes and the rebase within the next couple of days. Roger on releasing to BCR before adding support for new toolchains--I'm a frequent perpretrator of the "just one more feature" mindset. 😆 |
Previously, there was a list of compatible CPUs that were used to form the target constraints for the toolchain. Now, however, we need to distinguish toolchains on more than just @platforms//cpu, so manage these as a collection of constraint values instead.
This commit renames the module to arm_gnu_toolchain from arm_none_eabi. API-compatibility is _mostly_ retained through the new top-level repository generation mechanism. Using this module to configure an arm-none-eabi toolchain results in the generation of an external repository called @arm_none_eabi. In this way, clients can still rely on tools and targets that were previously defined by the old module. Consumers must now import module extensions and repository rules from the new module, however. See the updated README.md for more information.
Introduce a new custom repository rule to allow us to templatize the
"compiler" BUILD.bazel file (toolchain/compilers{nix,win}.BUILD). This
allows us an easy path to changing out the toolchain prefix when
integrating new toolchain types.
We now encode all toolchain-specific items in the attributes of the generic "cc_arm_gnu_toolchain_config" rule. This allows us to use this rule for other ARM GNU toolchains in the future.
All the interesting stuff now lives in a "private" _arm_gnu_toolchain function that specific toolchains can delegate to. This provide a common toolchain mechanism that can be used for creating and registering arm linux toolchains, for example.
Continuing with the trend of "genericization," this commit abstracts everything specific to the arm-none-eabi platform out of the arm_none_eabi_deps function into a new function that can be leveraged by additional toolchains. The arm_none_eabi_deps function is kept to maintain API stability.
Previously, there was a single tag class for the "arm_gnu" module extension which would be used by consumers to pull in arm vendored toolchains. Now, however, the tags map to the particular toolchain through the toolchain's target name. See the updated README for more information.
This commit introduces the named toolchain, which functions much in the same way as the existing arm-none-eabi toolchains. A couple of minor changes to existing common code are required to get simple applications to compile.
Previously, the BUILD template for the cross-hosted toolchain external repository (host-specific) had a defect that caused the paths of tools to be resolved incorrectly.
Currently, there is an issue where the consuming MODULE.bazel is not capable of overriding the version of the toolchain specified in our MODULE.bazel. Create a FIXME comment in extensions.bzl. Previously, there was also an issue where selecting any toolchain version besides 9.x.x caused a failure when setting up the external repository, because the generated BUILD.bazel wasn't provided the version of the toolchain it was generating.
This commit fixes an issue with the bzlmod extension that prevents consuming MODULE.bazel files from being able to select the version of the toolchain. Now, the bzlmod extension selects the minimum version from the modules that have requested the toolchain and uses that for the system. This allows consumers to upgrade the version of arm_gnu_toolchain without needing to ugprade their compiler version, as well.
This commit combines the two previous BUILD templates for the generated repos, nix.BUILD and win.BUILD, into one templatized compiler.BUILD template, and it also moves the other existing templates into the toolchain/templates folder.
Previously, there was a change that required the version of the toolchain to be specified in the cc_arm_*_config rules. This isn't ideal, because it requires consumers to list the version in two places. So instead, introduce some new filegroup targets in the cross-hosted toolchain repository that allows these rules to infer the toolchain version. Also fix some other existing issues with the examples.
Organize the examples by toolchain so that CI workflows can selectively build examples based on which toolchains are available for the host platform.
Previously, CI builds were failing for Windows because the arm-linux toolchain include paths exceeded the Maximum Path Length. Use --output_user_root in the workflow to avoid this limitation.
This commit gets us almost all the way to using rules_cc instead of bazel_tools for forward compatibility after the starlarkification of the C++ rules is complete. The only import that can't currently be changes is ACTION_NAMES, because ACTION_NAMES.llvm_cov is not defined in rules_cc as of today.
This affects default arguments and the README, but should not have any effect on consumers using older toolchains.
|
Alright, and I believe that's it! I set |
|
Merged, thanks for the amazing work @AmateurECE |
Hey, there!
Thanks for your great work on this hermetic toolchain. I needed the
arm-none-eabitoolchain, as well as thearm-none-linux-gnueabihftoolchain for a project I'm working on, so I decided to extend this repository to make both of them available. This PR also lays the groundwork for easily integrating other ARM GNU toolchains (see 2e0befc for an example of how this might look for additional toolchains).You may not be interested in integrating this capability into your existing repository. That's totally fine! If that's the case, I'd like to seek your permission to submit my fork to the Bazel Central Registry.
If you are interested in integrating this capability, I've listed some notes that I'd like you to be aware of as you're reviewing this pull request, along with some items for your consideration:
arm-none-eabiAPI available to both WORKSPACE and bzlmod consumers. Unfortunately, there were a few areas in which I felt this was not possible--the tag classes in the bzlmod extension, as an example.arm-none-linux-gnueabihfAPI mirrors the existingarm-none-eabiAPI. Is this desired? If not, are there any API changes you might like to see?@arm_gnu_toolchain. I am open to ideas if this is not a desirable name, but I do feel that there should be some change from@arm_none_eabiif we are to add support for other toolchains.@bazel_toolsrepository with the@rules_ccrepository. There is also an issue with the bzlmod extension that does not allow the toolchain version to be overridden in dependent MODULE.bazel files which should be fixed.Thank you very much for your time in reviewing this pull request, and don't hesitate to reach out with any questions you may have!