no_std support for form_urlencoded, data-url and idna

[madsmtm]

Split out from #717.

no_std support for url is going to take more effort, but we can start with this.

The builds (if they were run) are failing because of servo/unicode-bidi#63 Fixed.

[madsmtm]

The linked unicode-bidi issue has been merged and a new version released, so this PR should be ready

[bors-servo]

☔ The latest upstream changes (presumably #755) made this pull request unmergeable. Please resolve the merge conflicts.

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -0.03 ⚠️

Comparison is base (edeaea7) 82.74% compared to head (4afc982) 82.71%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #722      +/-   ##
==========================================
- Coverage   82.74%   82.71%   -0.03%     
==========================================
  Files          20       20              
  Lines        3303     3339      +36     
==========================================
+ Hits         2733     2762      +29     
- Misses        570      577       +7     

Impacted Files	Coverage Δ
data-url/src/forgiving_base64.rs	87.23% <ø> (ø)
data-url/src/lib.rs	83.47% <ø> (ø)
data-url/src/mime.rs	94.87% <ø> (ø)
form_urlencoded/src/lib.rs	68.65% <ø> (ø)
idna/src/lib.rs	66.66% <ø> (ø)
idna/src/punycode.rs	85.00% <ø> (ø)
idna/src/uts46.rs	94.17% <ø> (ø)
percent_encoding/src/lib.rs	59.83% <ø> (ø)

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[MathiasKoch]

Small ping on this

[bors-servo]

☔ The latest upstream changes (presumably #783) made this pull request unmergeable. Please resolve the merge conflicts.

[madsmtm]

Note that I've added required features to the default list, which is not outlined in cargo's SemVer Compatibility document, but would probably constitute as a "possibly-breaking change".

Specifically, if a user specified default-features = false, their crate would now fail to build until they also specified features = ["alloc"]. Note that they would not have a reason to disable default features, since there were no default features; but they might still have done it, hence why it could be considered a breaking change.
So I wrote a script to check this for all crates on crates.io, and the crates that would be affected are the following (all relying on form_urlencoded):

• did_url: Don't disable default features of form_urlencoded l1h3r/did_url#2
• identity_did + identity_document: Don't disable default features of form_urlencoded iotaledger/identity.rs#1135 / Pin form_urlencoded to 1.1.0 iotaledger/identity.rs#1136
• facebook-fb-login-deauth-callback: Don't disable default features of form_urlencoded bk-rs/facebook-rs#1

If we want to avoid breaking those, I could either open PRs for those projects to fix it, or I could change form_urlencoded such that specifying default-features = false actually means specifying features = ["alloc"] (though that would prevent us from making it no_alloc in the future) EDIT: I have opened PRs for all three projects, see above.

Though I would like some confirmation from the maintainers (@valenting?) that this is a desired change and approach before working further on this.

[madsmtm]

To expand on this: We include it to force no_std users to disable the std feature (since otherwise their program won't compile). This ensures that if we in the future want to add something that relies on std, we can.

[valenting]

Could you explain how that works? Or point me to a resource that does that?

[madsmtm]

If we didn't have this line, a user could add a dependency on data-url on a platform like the Arduino, and since we've specified #![no_std] in the crate, compilation will succeed, even though the std feature is enabled.

Let's say that in the future, we wanted to add an error type to this crate, and have it implement std::error::Error. To do so, we'd have to introduce a #[cfg(feature = "std")] extern crate std;. But since the hypothetical Arduino user didn't specify default-features = false, their crate would now fail to build.

With this line in place to begin with, we know the user has specified default-features = false to even compile the library.

For reference, the error looks something like this (can be simulated with -Zbuild-std=core,alloc):

error[E0463]: can't find crate for `std`
 --> data-url/src/lib.rs:21:1
  |
  | extern crate std as _;
  | ^^^^^^^^^^^^^^^^^^^^^^ can't find crate
  |

---

Did that clear it up? Or do you want me to try explaining it in a different way? Unfortunately, I haven't found many resources for "building Rust libraries that work well with no_std", so I can't really link to a blog post or something explaining the details, this is mostly figured out by trial and error :/

[valenting]

Sorry for the delayed response.

Thank you for looking into which crates would be broken by this change.
I assume there might also be non-public projects that might break, but there's not much we can do about those.
I think this is the right approach, though I don't have a lot of experience with no_std support.

Please fix the remaining CI issues - I think we can merge after that.

[lucacasonato]

Also don't have much experience with no_std - the code change looks correct though. Let's land once green.

[madsmtm]

I've fixed the CI (I think) and opened PRs for the known affected projects.

_{Tip: If you want to avoid having to click the "approve workflow" button all the time, you can change the default in the repository settings.}

[OverOrion]

Thank you very much for your work @madsmtm!

This PR seems to be almost ready to be merged. I can see that there is one CI check failure, is there anything we could do to help with it? @valenting

[lucacasonato]

One minor nit, but LGTM - thanks for working on this. I hope we'll be able to better support no_std going forward.

[madsmtm]

I can see that there is one CI check failure, is there anything we could do to help with it?

Not sure about that one, it's a coverage check that has regressed very, very slightly, in a file that I haven't modified, so it may just be spurious?

[valenting]

Looks great! Thank you for all the good work.