Skip to content

Remove unnecessary build.rs #87

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

Merged
merged 1 commit into from
Apr 17, 2020
Merged

Remove unnecessary build.rs #87

merged 1 commit into from
Apr 17, 2020

Conversation

adamgreig
Copy link
Member

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
Copy link

r? @korken89

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

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@therealprof
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 17, 2020

Configuration problem:
bors.toml: not found

@therealprof therealprof merged commit 3571fc9 into master Apr 17, 2020
@therealprof therealprof deleted the rm-build-rs branch April 17, 2020 21:50
@Disasm
Copy link
Member

Disasm commented Apr 18, 2020

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
Copy link
Member Author

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

@jacobrosenthal
Copy link
Contributor

jacobrosenthal commented Apr 20, 2020 via email

@rubberduck203
Copy link
Contributor

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
Copy link
Contributor

jacobrosenthal commented Apr 20, 2020 via email

@korken89
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants