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

no_std support for the url crate #831

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
35653b0
Make form_urlencoded no_std compatible
madsmtm Feb 17, 2022
194122c
Make data-url no_std compatible
madsmtm Aug 12, 2021
422a260
Make idna no_std compatible
madsmtm Feb 17, 2022
81ee074
Test no_std support
madsmtm Oct 5, 2021
67d2a4c
Initial work at making url no_std compatible
madsmtm Aug 12, 2021
db92dd2
Merge branch 'master' into no_std
domenukk Apr 7, 2023
bb7955a
Fixed testcases for no_std
domenukk Apr 8, 2023
a98aef8
fix no_std for nightly
domenukk Apr 8, 2023
0d14d5a
Add no_std to CI
domenukk Apr 8, 2023
315eb43
Fix no_std for deps
domenukk Apr 10, 2023
6d622ab
Moved to no-std-net
domenukk Apr 10, 2023
3efe254
Added unstable, no_std_net options
domenukk Apr 16, 2023
36bc5b3
Make CI happy by removing no-std-net/serde (not supported in Cargo.to…
domenukk Apr 27, 2023
29bfcf7
Merge branch 'master' into no_std
domenukk Apr 27, 2023
e16eb2b
Only test aarch64 on ubuntu
domenukk May 9, 2023
3dd87d8
Fix CI some more
domenukk May 11, 2023
af1d5d5
Windows fix
domenukk Jun 6, 2023
b908f7f
Merge branch 'master' into no_std
domenukk Jun 6, 2023
fd98554
Merge branch 'master' into no_std
domenukk Jul 12, 2023
b0bb942
Merge branch 'master' into no_std
domenukk Jul 12, 2023
047d334
Merge branch 'master' into no_std
lucacasonato Jul 14, 2023
028424a
Merge branch 'master' into no_std
domenukk Jul 16, 2023
7073957
Only checking crate for no_std
domenukk Jul 16, 2023
be123af
only build no_std aarch64 none on nightly
domenukk Jul 19, 2023
c145584
Only build for aarch64 on nightly
domenukk Jul 19, 2023
be73851
Merge branch 'master' into no_std
domenukk Jul 19, 2023
96e2a04
Merge branch 'master' into no_std
domenukk Nov 23, 2023
268c17e
Fix test
domenukk Nov 23, 2023
0c02cd9
fmt
domenukk Nov 23, 2023
316c868
Merge branch 'master' into no_std
domenukk Nov 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ jobs:
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}
# Add toolchain for no_std tests
- run: rustup toolchain install nightly
- name: Build `aarch64-unknown-none` with `no_std`
if: |
matrix.os == 'ubuntu-latest' &&
matrix.rust == 'nightly'
run: rustup target add aarch64-unknown-none && rustup component add rust-src --toolchain nightly-x86_64-unknown-linux-gnu && cargo +nightly build -Zbuild-std=core,alloc --target aarch64-unknown-none -v --release --no-default-features --features=alloc,unstable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucacasonato you could take over this check to the idna create

- run: cargo build --all-targets
# Run tests
- name: Run tests
Expand All @@ -48,7 +55,7 @@ jobs:
matrix.rust == 'nightly'
run: cargo test --test debugger_visualizer --features "url/serde,url/debugger_visualizer" -- --test-threads=1
- name: Test `no_std` support
run: cargo test --no-default-features --features=alloc
run: cargo test --no-default-features --features=alloc,no_std_net

WASM:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion data-url/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ alloc = []

[dev-dependencies]
tester = "0.9"
serde = {version = "1.0", features = ["derive"]}
serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] }
serde_json = "1.0"

[lib]
Expand Down
4 changes: 3 additions & 1 deletion idna/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ doctest = false
default = ["std"]
std = ["alloc", "unicode-bidi/std", "unicode-normalization/std"]
alloc = []
# Enable nightly features
unstable = []

[[test]]
name = "tests"
Expand All @@ -29,7 +31,7 @@ name = "unit"
assert_matches = "1.3"
bencher = "0.1"
tester = "0.9"
serde_json = "1.0"
serde_json = { version = "1.0" }

[dependencies]
unicode-bidi = { version = "0.3.10", default-features = false, features = ["hardcoded-data"] }
Expand Down
2 changes: 1 addition & 1 deletion idna/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
//! > that minimizes the impact of this transition for client software,
//! > allowing client software to access domains that are valid under either system.
#![no_std]

#[cfg_attr(feature = "unstable", feature("error_in_core"))]
// For forwards compatibility
#[cfg(feature = "std")]
extern crate std;
Expand Down
5 changes: 4 additions & 1 deletion idna/src/uts46.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ impl Idna {
return Errors::default();
}
let mut errors = processing(domain, self.config, &mut self.normalized, out);
self.output = std::mem::replace(out, String::with_capacity(out.len()));
self.output = core::mem::replace(out, String::with_capacity(out.len()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this fixes a bug in the current idna no_std support. Why is it not caught in CI, and should I open an extra PR for it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently dabbled with no_std stuff as well and I think the sure-fire way to test no_std compatibility is to:

  • target a platform that does not have std (certain ARM platforms) OR
  • have a really simple no_std binary crate which uses the libraries

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I'm trying it with cargo +nightly build -Zbuild-std=core,alloc --target aarch64-unknown-none -v --release --no-default-features

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you split this off into a separate PR?

let mut first = true;
for label in self.output.split('.') {
if !first {
Expand Down Expand Up @@ -713,6 +713,9 @@ impl From<Errors> for Result<(), Errors> {
#[cfg(feature = "std")]
impl std::error::Error for Errors {}

#[cfg(feature = "nightly")]
impl core::error::Error for Errors {}

impl fmt::Display for Errors {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(self, f)
Expand Down
3 changes: 1 addition & 2 deletions idna/tests/punycode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@

use crate::test::TestFn;
use idna::punycode::{decode, encode_str};
use serde_json::map::Map;
use serde_json::Value;
use serde_json::{map::Map, Value};
use std::str::FromStr;

fn one_test(decoded: &str, encoded: &str) {
Expand Down
15 changes: 11 additions & 4 deletions url/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,25 @@ debugger_test = "0.1"
debugger_test_parser = "0.1"

[dependencies]
form_urlencoded = { version = "1.2.0", path = "../form_urlencoded" }
idna = { version = "0.4.0", path = "../idna" }
percent-encoding = { version = "2.3.0", path = "../percent_encoding" }
form_urlencoded = { version = "1.2.0", path = "../form_urlencoded", default-features = false, features = ["alloc"] }
idna = { version = "0.4.0", path = "../idna", default-features = false, features = ["alloc"] }
percent-encoding = { version = "2.3.0", path = "../percent_encoding", default-features = false, features = ["alloc"] }
serde = {version = "1.0", optional = true, features = ["derive"]}
no-std-net = { version = "0.6.0", default-features = false, optional = true }

[features]
default = []
default = ["std"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fear this might be a fairly big breaking change?

When I added no_std support for the other libraries in this crate, it was more trivial, since there was only three crates that disabled default features - but that is not the case for url; there are many crates specifying default-features = false, so even though there was no default features previously, this change will affect all of those crates.

Python script for finding crates using `default-features = false`.
import json
import requests
import sys

API_URL = "https://crates.io/api/v1/crates"

crate_name = sys.argv[1]

page = 1
while True:
    # print(f"page {page}")
    data = json.loads(requests.get(f"{API_URL}/{crate_name}/reverse_dependencies?per_page=100&page={page}").content)
    page += 1

    if len(data['versions']) == 0:
        break

    versions = {x['id']: x for x in data['versions']}

    for dep in data['dependencies']:
        if not dep['default_features']:
            crate = versions[dep['version_id']]
            print(f"{crate['crate']} had `default-features = false`")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any other way?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... I see two:

  • Bundling this in with other breaking changes, and releasing url v0.3
  • Calling the feature unstable-no-std instead, to signify that it may break other crates in the dependency chain, and that it is a bad way of doing it which will be rectified once v0.3 is released.

std = ["idna/std", "percent-encoding/std", "form_urlencoded/std", "alloc"]
alloc = []
# UNSTABLE FEATURES (requires Rust nightly)
# Enable to use the #[debugger_visualizer] attribute.
debugger_visualizer = []
# Expose internal offsets of the URL.
expose_internals = []
# For no_std: Allow the use of no_std_net instead of nightly
no_std_net = ["no-std-net"]
# For no_std: Use errors_in_core and net_in_core
unstable = ["idna/unstable"]
domenukk marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/rust#108443 is FCP-ing now, so it'll probably land in a few months - perhaps we should wait with this feature until then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now this PR also uses error_in_core (rust-lang/rust#103765) but that part can be removed if we want to go for stable instead

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value in that feature is that this could become a non-breaking change (other than bumping MSRV, which is probably acceptable).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: core::net has landed in Rust 1.77.0


[[bench]]
name = "parse_url"
Expand Down
13 changes: 10 additions & 3 deletions url/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::cmp;
use std::fmt::{self, Formatter};
use std::net::{Ipv4Addr, Ipv6Addr};
use crate::net::{Ipv4Addr, Ipv6Addr};
use alloc::{
borrow::ToOwned,
string::{String, ToString},
vec::Vec,
};
use core::{
cmp,
fmt::{self, Formatter},
};

use percent_encoding::{percent_decode, utf8_percent_encode, CONTROLS};
#[cfg(feature = "serde")]
Expand Down