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

Regression in beta: failed to parse slash in a custom cfg name #7613

Closed
ordian opened this issue Nov 21, 2019 · 3 comments · Fixed by #7623
Closed

Regression in beta: failed to parse slash in a custom cfg name #7613

ordian opened this issue Nov 21, 2019 · 3 comments · Fixed by #7623
Labels
C-bug Category: bug

Comments

@ordian
Copy link
Contributor

ordian commented Nov 21, 2019

According to https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies, custom target specification is allowed, e.g.

[target."x86_64/windows.json".dependencies]
winhttp = "0.4.0"

cargo +stable metadata works on this toml, while cargo +beta metadata fails

❯ cargo +stable --version      
cargo 1.39.0 (1c6ec66d5 2019-09-30)
❯ cargo +beta --version
cargo 1.40.0-beta (5da4b4d47 2019-10-28)
❯ cat Cargo.toml 
[package]
name = "None"
version = "0.1.0"
[target."x86_64/windows.json"]
dependencies = { hello = "1.0" }
❯ cargo +stable metadata
<snip>
❯ cargo +beta metadata 
error: failed to parse manifest at `/tmp/Cargo.toml`

Caused by:
  failed to parse `x86_64/windows.json` as a cfg expression: invalid target specifier: unexpected character / in target name
@ehuss
Copy link
Contributor

ehuss commented Nov 21, 2019

This is a result of a change in the parsing code in #7375. However, this hasn't worked in a long time (if ever?), as the target was an absolute, canonicalized path. Then, in #7425 it was changed to be the file stem.

So a target like --target foo/bar.json now matches the platform in [target.bar.dependencies].

@alexcrichton Does the current behavior (file_stem) seem like the correct choice? If so, this seems like a documentation change (and adding a test).

@alexcrichton
Copy link
Member

Yeah I believe the filestem is what we've always intended as the general identifier for the target

@ordian
Copy link
Contributor Author

ordian commented Nov 21, 2019

Thanks for confirming this is a documentation bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants