Reorg stm32f3xx crates and CI: enforce no-warnings on test builds

[ppannuto]

Pull Request Overview

Travis wasn't actually enforcing the no warnings policy for test builds before (the CI=true -> deny warnings is a feature of board Makefiels, we need to -D warnings explicitly for other builds). This should prevent stuff like #1567 in the future.

This skips enum_primitive in the short term.

Testing Strategy

Compiling.

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make formatall.

---

TODO

This surfaces some questions in its current form, for example:

   Compiling stm32f4xx v0.1.0 (/Volumes/code/helena-project/tock/chips/stm32f4xx)
warning: unused import: `generic_isr`
  --> src/lib.rs:27:16
   |
27 | use cortexm4::{generic_isr, hard_fault_handler, svc_handler, systick_handler};
   |                ^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: unused import: `generic_isr`
  --> src/lib.rs:27:16
   |
27 | use cortexm4::{generic_isr, hard_fault_handler, svc_handler, systick_handler};
   |                ^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

    Finished test [unoptimized + debuginfo] target(s) in 8.28s
     Running target/debug/deps/stm32f4xx-cd9d3f9bd364c650

This kind of stuff is a pain to maintain, as we're going to have weird cfg on/off imports for tests?

Also, because of the whole Travis + Mac is slow thing, we've managed to let a breaking for Mac on testing slip in again:

[-bash] Tue 28 Jan 14:10 [[test-all-libs $] ~/code/helena-project/tock/chips/ibex]
$ cargo test
   Compiling ibex v0.1.0 (/Volumes/code/helena-project/tock/chips/ibex)
LLVM ERROR: Global variable '_start_trap_vectored' has an invalid section specifier '.riscv.trap_vectored': mach-o section specifier requires a segment whose length is between 1 and 16 characters.
error: could not compile `ibex`.
warning: build failed, waiting for other jobs to finish...
LLVM ERROR: Global variable '_start_trap_vectored' has an invalid section specifier '.riscv.trap_vectored': mach-o section specifier requires a segment whose length is between 1 and 16 characters.
error: could not compile `ibex`.

To learn more, run the command again with --verbose.

If we keep adding more host-compiled testing, this is only going to get worse.

[bradjc]

We shouldn't add #ifdefs/cfgs to remove warnings from tests on platforms Tock doesn't support.

[gendx]

We shouldn't add #ifdefs/cfgs to remove warnings from tests on platforms Tock doesn't support.

How are defined supported platforms? I see two categories:

• Runtime platforms, i.e. physical boards where Tock runs.
• Development platforms, i.e. where Tock can be compiled.

My opinion is that we shouldn't rule out development platforms as unsupported, because there's no Tock without them. I think it would be important to clarify that in the documentation.

It seems to me that the current status quo is that the only supported development platform is Linux, because every pull-request is tested on Linux with Travis-CI. On the contrary, Mac is not supported: due to it not running on Travis-CI, there can always be Mac-specific bugs slipping through, as noticed by @ppannuto.

I see two ways of adding support for Mac, if it's what we want to do: (1) adding it back to Travis-CI i.e. #1395 or (2) have someone manually test each and every pull-request on their Mac. Despite the latency cost of (1), it would IMO be the most viable option as the latency of (2) is likely an order of magnitude higher at the very least. There is also option (3) the current status quo of "best-effort" support for Mac, with someone occasionally filing an issue/pull-request because we broke Mac.

I'm also not convinced that the latency cost of (1) is prohibitive, given that in practice the average review time of pull-requests is much higher than that. Unless I missed something, in 98% of the cases any issue can be detected with the "fast" Linux Travis-CI, and by the time reviews have happened the "slow" Mac Travis-CI will have completed as well.

Also, looking at the Travis-CI status, in the past month the peak daily backlog for Mac was <= 50 jobs (except one day), whereas the peak active Mac builds was >= 200 jobs at a time (sometimes even up to 300 jobs), which suggest that during rush hour the extra slowness is usually 1/4th of the latency of an average build. I'm not sure how much extra latency that is exactly, but I doubt it's a bottleneck in Tock development.

[bradjc]

I looked up when we removed running tests on OSX: #662. In ~1000 PRs since, I can't remember a case where Tock compiled on a mac didn't work (but the a version compiled on linux did). Even if there are maybe a handful of cases, I don't think it is worth any extra travis time to prevent what hasn't really been a problem.

I still think that if the development environment is not working well to build the OS we want, then we need to fix the development environment, not patch the OS to make the development environment function better.

[gendx]

I can't remember a case where Tock compiled on a mac didn't work (but the a version compiled on linux did).

Although it seems uncommon indeed, there were at least two cases:

1. @ppannuto's comment (Reorg stm32f3xx crates and CI: enforce no-warnings on test builds #1568 (comment)) showed an example where it didn't work.
2. There was another case: Add cargo test for chips and archs on ci-travis. #1393 (comment).

Even if there are maybe a handful of cases, I don't think it is worth any extra travis time to prevent what hasn't really been a problem.

So is it fair to say that MacOS is not officially supported? Or that MacOS is officially supported for releases of Tock, but not necessarily every commit? But that issues and pull-requests are welcome to support MacOS on a best-effort basis?

I think it's important to clarify that in the documentation.

Besides that, I think that extra Travis time isn't really a problem either - at least in the current situation according to the rough cost analysis above. But I think the most important it to make it clear what platforms (whether development or boards) are officially supported or not.

---

I still think that if the development environment is not working well to build the OS we want, then we need to fix the development environment.

In this case, the error message on the MacOS development environment was:

LLVM ERROR: Global variable '_start_trap_vectored' has an invalid section specifier '.riscv.trap_vectored': mach-o section specifier requires a segment whose length is between 1 and 16 characters.

I'm not sure how you're planning to "fix" MacOS to remove the constraint that "mach-o section specifier requires a segment whose length is between 1 and 16 characters".

I don't think it's realistic to fix the development environment for all the problems that we face.

[bradjc]

I can't remember a case where Tock compiled on a mac didn't work (but the a version compiled on linux did).

Although it seems uncommon indeed, there were at least two cases:

1. @ppannuto's comment ([#1568 (comment)](https://github.com/tock/tock/pull/1568#issue-368256116)) showed an example where it didn't work.

2. There was another case: [#1393 (comment)](https://github.com/tock/tock/pull/1393#issuecomment-539723255).

I did word my comment carefully...those broke CI on mac, but not the ability to build a working kernel on mac. I think that is a substantial difference. The tests can run on one platform and suitably catch errors for both platforms.

Even if there are maybe a handful of cases, I don't think it is worth any extra travis time to prevent what hasn't really been a problem.

So is it fair to say that MacOS is not officially supported? Or that MacOS is officially supported for releases of Tock, but not necessarily every commit? But that issues and pull-requests are welcome to support MacOS on a best-effort basis?

I think it's important to clarify that in the documentation.

Besides that, I think that extra Travis time isn't really a problem either - at least in the current situation according to the rough cost analysis above. But I think the most important it to make it clear what platforms (whether development or boards) are officially supported or not.

If a PR were to prevent the kernel from compiling on mac we would not merge that PR. I'm not sure where that falls as far as officially supported or not.

I still think that if the development environment is not working well to build the OS we want, then we need to fix the development environment.

In this case, the error message on the MacOS development environment was:

LLVM ERROR: Global variable '_start_trap_vectored' has an invalid section specifier '.riscv.trap_vectored': mach-o section specifier requires a segment whose length is between 1 and 16 characters.

I'm not sure how you're planning to "fix" MacOS to remove the constraint that "mach-o section specifier requires a segment whose length is between 1 and 16 characters".

I don't think it's realistic to fix the development environment for all the problems that we face.

My fix would be to not compile code written for the RISC-V architecture to an x86 platform.

[ppannuto]

Maybe there's a compromise point here:

Travis builds twice for PRs: One is the build that's just the tip of the branch that was pushed (the "push" build), and one is the build that tries merging the branch to master (the "pr" build).

What if we set up travis to only run OS X on the "pr" build? That gives both the fail-fast behavior that @bradjc is looking for as well as the reliability that @gendx is looking for.

Or, I think we could actually go one step further and only do the OS X build when bors attempts to do the final merge. I think the way we'd implement that is bors always uses a branch it owns called staging to do merge attempts; we can configure the OS X build to only test the staging (and probably master) branch(es).

[gendx]

I agree that having the tests running on the final merge commits / on the master branch would be enough to say that OSX is "fully supported".

[gendx]

FYI, with the recent bump in Rust toolchain, I noticed that there are now warnings against unused #[inline] attributes, due to rust-lang/rust#65294.

[ppannuto]

Waiting on #1625

[hudson-ayers]

I think this was actually unblocked by #1786, though it needs to be rebased to account for the recent Makefile reorganization.

[ppannuto]

I did the rebase, but it looks like there are still lingering issues related to the use of features in the STM chips. My guess is that we could just rip those features out and unconditionally include after your STM re-org, but take a look?

[hudson-ayers]

I only touched the stm32f4xx chip crate in my stm re-org because it was the only crate using features to select between different configurations for two different chips.

Oh, looks like when stm32f303vct6 support was added they copied the stm32f4xx crate which included copying over the use of #[cfg(feature)]'s even though there is only one supported stm32f3xx board.

The easy fix would be to just remove the cfg, as it is currently not needed. But I decided to also rename chips/stm32f3xx --> chips/stmf303xc to more accurately reflect that the code in that folder is unlikely to support all stm32f3xx chips.

cc @alexandruradovici I think you are the only one using the stm32f3 chip, so let me know if you have any problem with this.

[alexandruradovici]

@hudson-ayers looks good to me, no problem.

[ppannuto]

But I would if I could! Looks good to me!

[hudson-ayers]

Given that Pat cannot approve this, I will. I'll still wait a bit before merging in case anyone else has feedback.

[bradjc]

I think this avoids the IRQS issue from f4 version by having init still in lib.rs.

[hudson-ayers]

I think this avoids the IRQS issue from f4 version by having init still in lib.rs.

Agreed, the changes here are saved from it by the fact that there was no need for the extra layer of indirection because we only support one stm32f3xx board anyway.

[bradjc]

bors r+

[bors]

Build succeeded:

• ci-build
• ci-format
• ci-tests
• continuous-integration/travis-ci/push
• emulation-check