Skip to content

Use collections from the alloc crate in percent_encoding #691

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 4 commits into from
Mar 26, 2021

Conversation

grego
Copy link
Contributor

@grego grego commented Mar 10, 2021

Use collections from alloc in the percent_encoding crate, thus making it fully no_std compatible.

Additionally implements From<PercendEncode>> and From<PercendDecode>> for beef::Cow, using an optional beef feature gate.
Edit: ditched the beef implementation as it didn't pass all of the checks.

@djc
Copy link
Contributor

djc commented Mar 10, 2021

Some downstream users might not have alloc available, right? I think this makes their lives worse?

@grego
Copy link
Contributor Author

grego commented Mar 10, 2021

Fair enough. Added all the usage of alloc types behind an optional alloc feature gate.

Copy link
Contributor

@djc djc 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!

@valenting valenting merged commit d530255 into servo:master Mar 26, 2021
@grego
Copy link
Contributor Author

grego commented Apr 28, 2021

Could you, please, publish a version with this PR? I need it for a project.

@tesaguri
Copy link

tesaguri commented Dec 8, 2021

Technically speaking, this is a breaking change if a downstream user specifies default-features = false, though I don't think it would be a problem in practice since percent-encoding has had no feature flags before. I just thought it worth mentioning.

@djc
Copy link
Contributor

djc commented Dec 8, 2021

I wouldn't be surprised if there are people using default-features = false anyway out of habit.

@tesaguri
Copy link

tesaguri commented Dec 8, 2021

I wouldn't be surprised if there are people using default-features = false anyway out of habit.

I've done a quick search for packages in rust-lang/crates.io-index whose most recently published version has dependency on percent-encoding with default features disabled.

$ curl -fL -o crates.io-index.zip https://github.com/rust-lang/crates.io-index/archive/refs/heads/master.zip
$ stat -f '%Sm' -t '%FT%T%z' crates.io-index.zip
2021-12-08T17:43:38+0900
$ unzip crates.io-index.zip
$ find crates.io-index-master -type f ! -name config.json | xargs -n 253 tail -q -n 1 | jq -c 'select(.deps | any([.name == "percent-encoding", .default_features == false] | all)) | {name, vers}'

Result:

{"name":"instagram-web-api","vers":"0.1.1"}
{"name":"twilight-lavalink","vers":"0.8.0"}
{"name":"twilight-http","vers":"0.8.0"}
{"name":"mendes","vers":"0.0.60"}
{"name":"atlassian-app-auth","vers":"1.0.1"}
{"name":"gcs-rsync","vers":"0.1.10"}
{"name":"cloud-storage-rs","vers":"0.4.3"}
{"name":"cloud-storage","vers":"0.11.0-rc.1"}

So there are indeed downstream crates with default-features = false.

I didn't build them with this change applied so I don't know if this breaks the above crates (I'm too nervous to build random packages in an unsandboxed environment and too lazy to set up a Crater-like sandboxed environment or manually review each package).

tesaguri added a commit to tesaguri/oauth1-request-rs that referenced this pull request Jan 2, 2022
Full `no_std` support is pending the release of
<servo/rust-url#691>.
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

Successfully merging this pull request may close these issues.

4 participants