rust-embedded / cortex-m-quickstart

Template to develop bare metal applications for Cortex-M microcontrollers
782 stars 164 forks source link

Remove unnecessary build.rs #87

Closed adamgreig closed 4 years ago

adamgreig commented 4 years ago

The build.rs script in quickstart copies memory.x into OUT_DIR. However, the linker already searches the current working directory when resolving the include of memory.x, so will always find the version in the project directory before even looking in OUT_DIR. It's not necessary for user applications to have a build.rs copy memory.x, only for dependencies wishing to inject linker scripts.

From experimenting, it seems Cargo always runs the linker from the directory Cargo.toml is in, even if building in a totally different directory or workspace and using --manifest-path and --target-dir. In other words, the linker always finds memory.x that's adjacent to Cargo.toml.

rust-highfive commented 4 years ago

r? @korken89

(rust_highfive has picked a reviewer for you, use r? to override)

therealprof commented 4 years ago

bors r+

bors[bot] commented 4 years ago

Configuration problem: bors.toml: not found

Disasm commented 4 years ago

If I recall correctly, this doesn't work if cortex-m-quickstart is used from within a workspace. In this case memory.x should be placed near the workspace Cargo.toml (opposed to project Cargo.toml) and build.rs helped to overcome this.

adamgreig commented 4 years ago

Ugh, workspaces are annoying as usual. A few things are affected:

I think the build script does resolve the memory.x issue but I'd really rather not have every embedded project copy a build.rs from quickstart needlessly. Is there a way we could document that you need it if you're using a workspace? You already need to take some special measures just to select a target and add the linker script argument...

jacobrosenthal commented 4 years ago

How about another quickstart that is a workspace. Possibly with some of the host side testing explained as well

On Mon, Apr 20, 2020, 3:17 PM Adam Greig notifications@github.com wrote:

Ugh, workspaces are annoying as usual. A few things are affected:

  • .cargo/config in a crate inside a workspace is ignored if you build from the top-level directory, which means your RUSTFLAGs adding the linker script is ignored and you get an empty binary
  • If you've put a .cargo/config or override RUSTFLAGs for the build, or you build from inside the actual crate directory instead, then it's unable to find memory.x
  • It seems like Cargo runs the linker with the working directory set to the workspace, which is why this fails
  • However, even if you put memory.x into the workspace directory, it complains about not being able to find device.x, I don't know why

I think the build script does resolve the memory.x issue but I'd really rather not have every embedded project copy a build.rs from quickstart needlessly. Is there a way we could document that you need it if you're using a workspace? You already need to take some special measures just to select a target and add the linker script argument...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rust-embedded/cortex-m-quickstart/pull/87#issuecomment-616841071, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADPI5CAEXEA5XZJ3EJTOSTRNTCZJANCNFSM4MKJN6OQ .

rubberduck203 commented 4 years ago

I like that idea personally. Templates should be plentiful and I’ve seen quite a few people asking how to write unit tests that run on host.

jacobrosenthal commented 4 years ago

Im basically thinking of something korken has posted https://github.com/korken89/rust-embedded-example-project

On Mon, Apr 20, 2020 at 3:38 PM Christopher McClellan < notifications@github.com> wrote:

I like that idea personally. Templates should be plentiful and I’ve seen quite a few people asking how to write unit tests that run on host.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rust-embedded/cortex-m-quickstart/pull/87#issuecomment-616848435, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADPI5GSOD6H6UQOSPYGCGDRNTFH7ANCNFSM4MKJN6OQ .

korken89 commented 4 years ago

I quite like the template I made, I am still using it as-is to this day when starting a new project.