search

ParkMyCar / compact_str

A memory efficient string type that can store up to 24* bytes on the stack
MIT License
557 stars 41 forks source link

fix serde no-std issue #347

Closed dragazo closed 6 months ago

dragazo commented 6 months ago

Hey, @ParkMyCar. I just noticed you put out a beta release with the no-std updates a while ago and I've been using that in my project (very slight speedup, and a lot of memory savings!). The only issue I had was that the serde dependency was using std by default. I hadn't checked for that before since I was doing plain old --no-default-features, but it's pretty common to use (binary) serialization on embedded by using serde with default features off.

I just updated the serde dependency to

serde = { version = "1", optional = true, default-features = false, features = ["derive", "alloc"] }

which (afaik) should be compatible with normal/std serde as well (I at least don't get any errors when I build and test my project with std turned on).

Also, I noticed the cross tool for cross-compiling embedded works by mounting the crate in a docker container. But the README.md inside the compact_str crate was a symlink to the root directory version, which resulted in a file not found error from include_str! when cross building. To fix that, I just flipped it around so that the readme in compact_str is the real file and the one in the root directory is a symlink to it. Not sure how I missed that before - maybe the errors from castaway were hiding it back then.

dragazo commented 6 months ago

It looks like the MSRV check failed with error:

error: package `regex-syntax v0.8.2` cannot be built because it requires rustc 1.65 or newer, while the currently active rustc version is 1.64.0

Apparently they bumped their MSRV from 1.60 to 1.65 as of 0.8.0, but that's unrelated to this PR.

ParkMyCar commented 6 months ago

Fantastic, thanks for the fix @dragazo!

Yeah the MSRV issue should be fixed with https://github.com/ParkMyCar/compact_str/pull/348

ParkMyCar commented 6 months ago

@dragazo I tried rebasing this branch but I can't seem to update the PR. When you get a chance do you mind rebasing? There was a change to the README that's conflicting. Thanks!

dragazo commented 6 months ago

@ParkMyCar Ah sorry, I had original repo owner edits turned off just cause I'm using my fork as a git dep in my project for now. I just merged in the changes and reapplied the symlink switch.

ParkMyCar commented 6 months ago

No problem, thanks for rebasing @dragazo!