Skip to content
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

Use Cargo's links = "name" feature in singleton crates #467

Open
jonas-schievink opened this issue Jul 3, 2020 · 2 comments
Open

Use Cargo's links = "name" feature in singleton crates #467

jonas-schievink opened this issue Jul 3, 2020 · 2 comments

Comments

@jonas-schievink
Copy link
Contributor

We have several crates that only support linking a single version of them into a crate graph. These crates include:

  • All -rt crates, so cortex-m-rt, riscv-rt, msp430-rt, ...
    • These crates need to ensure this since they define the memory layout of the binary, and doing that multiple times would at best result in very confusing errors, and at worst produce a binary that doesn't work right.
  • All peripheral access crates like cortex-m and all svd2rust-generated PACs
    • These crates need to ensure this, as otherwise you could depend on 2 PACs and .take() their Peripherals twice.

Currently, these crates define a #[no_mangle] symbol to ensure that only a single version of them is linked to by an application, causing a linker error when more than one version is present.

Another way to do this is to use Cargo's links = "<name>" feature, which was designed to support precisely this use case (only for linking to C libraries that have global state). I think we should switch every crate to use the Cargo feature, since it was designed for this, gives a better error message, and cannot fail when no methods from one of the versions are called (a shortcoming of the current approach).

To do this, we need to migrate slowly, since removing the clashing symbol removes the protection with older versions of the library. That means, we start by adding a links key to all affected crates, and only after a couple of releases we remove the clashing symbol.

@adamgreig
Copy link
Member

In general 👍 on this. For the PACs, we don't host them in the wg, and svd2rust doesn't generate Cargo.toml, so I think all we can do is open issues on PACs to encourage them to use links and perhaps update the svd2rust docs.

What value do you think PACs should use for the links key? I wonder if just pac (or peripheral-access-crate) could work, to prevent ever linking two PACs, even alternative PACs such as stm32ral? If you used the crate name, someone might link e.g. stm32f0 from that crate, and another svd2rust-generated crate using stm32f0x1, and even stm32ral all at once.

@jonas-schievink
Copy link
Contributor Author

Yeah, makes sense to use a shared key. We should probably document this in a central place (maybe this repo?).

And, I suppose, I'd like svd2rust to generate Cargo.toml then.

bors bot added a commit to rust-embedded/cortex-m-rt that referenced this issue Jul 9, 2020
276: Replace __ONCE__ with Cargo links key r=jonas-schievink a=adamgreig

This would fix #275. We use the links key in cortex-m already [here](https://github.com/rust-embedded/cortex-m/blob/master/Cargo.toml#L16). See also rust-embedded/wg#467.

Co-authored-by: Adam Greig <adam@adamgreig.com>
adamgreig added a commit to rust-embedded/cortex-m that referenced this issue Jan 12, 2022
276: Replace __ONCE__ with Cargo links key r=jonas-schievink a=adamgreig

This would fix #275. We use the links key in cortex-m already [here](https://github.com/rust-embedded/cortex-m/blob/master/Cargo.toml#L16). See also rust-embedded/wg#467.

Co-authored-by: Adam Greig <adam@adamgreig.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants