crane icon indicating copy to clipboard operation
crane copied to clipboard

mkDummySrc: support freestanding targets

Open rvolosatovs opened this issue 3 years ago • 3 comments

Currently, when building for targets that do not support std, like x86_64-unknown-none, cargo check on the costructed dummy source fails with:

 > error[E0463]: can't find crate for `std`
       >   |
       >   = note: the `x86_64-unknown-none` target may not support the standard library
       >   = note: `std` is required by `sallyport` because it does not declare `#![no_std]`
       >   = help: consider building the standard library from source with `cargo build -Zbuild-std`
       >
       > For more information about this error, try `rustc --explain E0463`.

To prevent this issue from happening, set #![no_std] for freestanding targets.

Signed-off-by: Roman Volosatovs [email protected]

Motivation

Checklist

  • [ ] added tests to verify new behavior
  • [ ] added an example template or updated an existing one
  • [ ] updated docs/API.md with changes
  • [x] updated CHANGELOG.md

rvolosatovs avatar Oct 07 '22 17:10 rvolosatovs

Would you be interested in adding a test case to avoid future regressions?

I'd love to, if fact I looked into doing that while working on this, but was not clear how to proceed, so I did not. I suppose the way to do it would be to add a freestanding target to the CI toolchain to ensure it's picked up with --all-targets?

rvolosatovs avatar Oct 10 '22 11:10 rvolosatovs

I'd love to, if fact I looked into doing that while working on this, but was not clear how to proceed, so I did not. I suppose the way to do it would be to add a freestanding target to the CI toolchain to ensure it's picked up with --all-targets?

I forked your branch, rebased it onto the latest master, and added a commit with a cortex-m #![no_std] example: https://github.com/newAM/crane/tree/fix/no_std

The examples get tested in CI as far as I can tell.

Feel free to take that commit, or remix it however you want :smile:

newAM avatar Oct 10 '22 15:10 newAM

@rvolosatovs

I'd love to, if fact I looked into doing that while working on this, but was not clear how to proceed, so I did not.

The test suite is a bit disorganized atm so I apologize :sweat_smile:

Here's the approach I'd take (in priority order, but happy to help finish up any steps you don't get to):

  1. Define a simple-nostd crate in the checks directory, which should fail to build via buildPackage if your improvements to mkDummySrc are commented out (you can use nix build .#checks.x86_64-linux.NAMEOFTEST to avoid having to rebuild the entire test suite just to check this works)
  • Ideally the crate would just have some small dependency (also #![no_std]) which is enough to check for regressions, but feel free to adapt @newAM 's example above if it helps getting started
  1. The checks/mkDummySrc tests will likely need to be updated with the new expected values. Basically these tests verify what the dummified source looks like so the newly added lines will likely need to be copied in there as well
  2. As a bonus, it might be good to also add a compilesFresh test for the simple-nostd crate as well. That test checks that the artifacts from the buildDepsOnly step are actually usable by cargo (without having to rebuild) which won't hurt to check

Feel free to ask if you have any questions!

ipetkov avatar Oct 10 '22 16:10 ipetkov

@ipetkov apologies for the delay, but I have just updated the PR, PTAL.

Note that the issue is not in #![no_std] crates, it's in compiling for freestanding targets. Freestanding targets lack a standard library and therefore cannot be compiled for: 1.) when no_std is not set 2.) when the bare-minimum required runtime handlers are not defined

rvolosatovs avatar Oct 18 '22 17:10 rvolosatovs

Hmm.. I failed to reproduce the error in the check, let me do another iteration

rvolosatovs avatar Oct 18 '22 18:10 rvolosatovs

Done, https://github.com/ipetkov/crane/pull/126/commits/758ee594f6faaafbfe759459fd7364709dcb1c50 can be used to reproduce the error on latest master (nix build -L .#checks.x86_64-linux.noStdFreestandingTarget). This error is fixed in the following commit.

rvolosatovs avatar Oct 18 '22 19:10 rvolosatovs

This looks great, thanks again @rvolosatovs!

I've pushed some very minor fixes which weren't even worth mentioning here, will merge as soon as CI passes!

ipetkov avatar Oct 19 '22 01:10 ipetkov