Repository navigation
fix(util-schemas): compile out the file-path hint where url can't build it - #17600
UebelAndre wants to merge 1 commit into
Conversation
rustbot
added
the
A-infrastructure
rustbot
added
the
S-waiting-on-review
|
Thanks for the pull request, and welcome! The Rust Project has assigned @Muscraft (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks. Please see the contribution instructions and our LLM policy for more information. Why was this reviewer chosen?The reviewer was selected based on:
|
|
Note that our contrib guide asks for atomic commits. |
| # `cargo-util-schemas` is consumed by third-party tools, some of which run | ||
| # on targets with no filesystem, environment, clock, or network. | ||
| - run: rustup target add wasm32-unknown-unknown | ||
| - run: cargo check -p cargo-util-schemas --all-features --target wasm32-unknown-unknown --locked |
There was a problem hiding this comment.
This is unrelated to the job it is operating in
We also have a job for enforcing --locked
| // This is purely a diagnostic: if the spec names an existing | ||
| // path, suggest the `file://` URL that would be accepted instead. | ||
| // `Url::from_file_path` only exists on targets with a known path | ||
| // syntax, so the hint is compiled out elsewhere (e.g. | ||
| // `wasm32-unknown-unknown`), keeping this crate buildable there. |
There was a problem hiding this comment.
This feels fairly verbose for what needs to be said
| target_os = "redox", | ||
| target_os = "wasi", | ||
| target_os = "hermit" | ||
| ))] |
There was a problem hiding this comment.
Is there a negative we could do? I wonder if that would be easier to maintain. Granted, we do have end-to-end tests, so for any platform we run the tests, we'd identify that a cfg was missing, so maybe not an issue.
There was a problem hiding this comment.
I can update the pattern but before I do, this matches what rust-url has so would you still prefer the updated pattern or to match the that crate: https://github.com/servo/rust-url/blob/v2.5.8/url/src/lib.rs#L2542-L2551
…uild it `cargo-util-schemas` is consumed by third-party tools, some of which run on targets like `wasm32-unknown-unknown` that have no filesystem. There `Url::from_file_path` doesn't exist, so the crate failed to compile. The hint that suggests a `file://` URL for a path-like spec is purely diagnostic, so gate it on the same `cfg` `url` uses.
7f4e387 to
4b6862c
Compare
UebelAndre
changed the title
PackageIdSpec::parse free of host access
|
Updated the PR title, description, and commit. @rustbot ready |
This branch has not been deployed
4 participants
Part of #17595.
What does this PR try to resolve?
cargo-util-schemasis consumed by third-party tools, some of which run on targets with no filesystem, environment, or network, such aswasm32-unknown-unknown.PackageIdSpec::parseprobed the current directory to suggest afile://URL for specs that look like paths, and that does not compile there:urlonly providesUrl::from_file_pathon unix, windows, redox, wasi, and hermit.The hint is purely diagnostic, so it is now compiled out behind the same
cfgthaturluses. Behavior is unchanged on every target that built before.How to test and review this PR?
A new CI job checks that
cargo-util-schemasbuilds forwasm32-unknown-unknown; locally that iscargo check -p cargo-util-schemas --all-features --target wasm32-unknown-unknown. The existingpkgidtests cover the hint on supported targets.