search

rust-embedded / cortex-m-quickstart

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

force rebuild if memory.x changed #91

Closed obraunsdorf closed 4 years ago

rust-highfive commented 4 years ago

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @therealprof (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

adamgreig commented 4 years ago

By default the linker will search the crate's directory without needing to be told, so having memory.x in the same place as Cargo.toml works fine; adding a build.rs which only adds cargo:rerun-if-changed=memory.x is enough to rerun the build when memory.x changes in that case. Whether we want/should (re)include that longer build.rs example in the quickstart, I don't know... the only edge case where it can be required is in workspaces or weird build configurations.

therealprof commented 4 years ago

I'm not sure where the more elaborate build.rs is coming from. Also I don't see too much value in a build.rs in general but if we have one it should probably provide the full benefit and not just half. Are there any downsides to providing the full version?

thalesfragoso commented 4 years ago

I'm not sure where the more elaborate build.rs is coming from. Also I don't see too much value in a build.rs in general but if we have one it should probably provide the full benefit and not just half. Are there any downsides to providing the full version?

I always use the complex one, but some people did some testing and it seems that the cargo directory is already in the linker search path, so it's unnecessary to copy it somewhere else.

therealprof commented 4 years ago

As discussed in the meeting we should add a comment to explain when and why this might be necessary to give users a good base for a decision whether this is necessary.

therealprof commented 4 years ago

Addressed by #92.