fixes maybstd imports for no_std on thumbv6m-none-eabi

[sunny-g]

I'm having a tiny bit of trouble getting my Cargo workspace to completely work with patches, paths, a 0.0.0 version for this library and a few other build dep things, but I think this fixes the inability to use borsh in no_std b/c a failure to use alloc::sync on line 32.

The remainder of the changes are just in keeping with the rest of the codebase, which only depends on rc and sync when cfg(feature = "rc")

[austinabell]

alloc::sync does exist, so not sure what could be the issue there, but I did notice that rc was broken when used without std feature. I fixed that specific problem with #88. Were you using this with std feature not set and rc set? If not, can you produce a repro of this error or more details?

This change probably can't come in by the way because it's a breaking change (removing rc and sync modules without the rc feature set)

[sunny-g]

Hmmm, this could be unique to my env. I'm currently blocked just adding borsh as a dependency to the sample Ledger Nano S rust app.

Cargo.toml (simplified):

[dependencies]
borsh = { version = "0.9", default-features = false }
nanos_sdk = { git = "https://github.com/LedgerHQ/ledger-nanos-sdk" }
nanos_ui = { git = "https://github.com/LedgerHQ/ledger-nanos-ui" }

and this is the error produced:

> cargo build
    Updating crates.io index
   Compiling borsh-derive-internal v0.9.3
   Compiling borsh-schema-derive-internal v0.9.3
   Compiling borsh-derive v0.9.3
   Compiling borsh v0.9.3
error[E0432]: unresolved import `alloc::sync`
  --> /home/sunnyg/.cargo/registry/src/github.com-1ecc6299db9ec823/borsh-0.9.3/src/lib.rs:32:56
   |
32 |     pub use alloc::{borrow, boxed, format, rc, string, sync, vec};
   |                                                        ^^^^ no `sync` in the root

For more information about this error, try `rustc --explain E0432`.
error: could not compile `borsh` due to previous error

[sunny-g]

But to answer your questions, as you can see above I've disabled all features, under the assumption that you couldn't use rc without std. If that's the fix, then my bad!

[sunny-g]

So that's not the fix - I'm getting substantially more errors, including the aforementioned one. The target is thumbv6m-none-eabi with a recent nightly toolchain - could that be the culprit? I'm not too familiar with embedded rust.

[austinabell]

So that's not the fix - I'm getting substantially more errors, including the aforementioned one. The target is thumbv6m-none-eabi with a recent nightly toolchain - could that be the culprit? I'm not too familiar with embedded rust.

Hmm, yeah so this is just a target-specific issue. I think support for this can be added, but I don't personally know the cleanest way to exclude Arc things just for this family of architectures. Also not sure if we are open to very specific configurations in this repo, we can open an issue but you might want to just fork where you remove the incompatible parts as a stopgap solution for now.

[sunny-g]

That's understandable - though not my preference, I can maintain a fork for now.

The last thing I'd like to highlight is that from what I can tell, this only a pseudo-breaking change, b/c of a potential contradiction in the API. You mentioned this earlier:

I did notice that rc was broken when used without std feature. I fixed that specific problem with #88

which is kinda the other half of what I want to highlight:

• where rc and sync are used within borsh, they are behind #[cfg(feature = "rc")] as expected
• where they're exported in maybestd, they aren't behind any flag, which (at least for me) was unexpected. Put another way, feature = "std" seems "broken" if rc's feature-gates are ignored by the std/no_std flag.

So with respect to the users of borsh::maybestd, I do understand this as a breaking change, but with regards to the actual usage within the library, this change doesn't seem to break anything (though correct me if I'm wrong, I might be missing something).

[austinabell]

• where they're exported in maybestd, they aren't behind any flag, which (at least for me) was unexpected. Put another way, feature = "std" seems "broken" if rc's feature-gates are ignored by the std/no_std.

So with respect to the users of borsh::maybestd, I do understand this as a breaking change, but with regards to the actual usage of the library, this change doesn't seem to break anything (though correct me if I'm wrong, I might be missing something).

Ack that this is probably correct, but I'm just being cautious since I'm not sure of all cases the lib is used. cc @matklad @frol any objections to putting rc and sync behind the rc feature flag?

[frol]

any objections to putting rc and sync behind the rc feature flag?

No objections

[austinabell]

If no objections for a breaking change and this doesn't come with a patch release