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

Implement cfg-based target-specific dependencies #2328

Merged
merged 1 commit into from
Feb 16, 2016

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of [RFC 1361][rfc] which is an extension of
Cargo's target section in Cargo.toml to allow the use of #[cfg]-like
expressions for target-specific dependencies. Now that the compiler has been
extended with --print cfg each invocation of Cargo will scrape this output and
learn about the relevant #[cfg] directives in play for the target being
compiled. Cargo will then use these directives to decide whether a dependency
should be activated or not.

This should allow definition of dependencies along the lines of:

[target.'cfg(unix)'.dependencies]
[target.'cfg(target_os = "linux")'.dependencies]
[target.'cfg(windows)'.dependencies]

Which is much more ergonomic and robust than listing all the triples out!

@rust-highfive
Copy link

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

Note that this will require rust-lang/rust#31278 to land first to get the CI/tests to pass, but I figured I'd at least put this up for review?

r? @brson or @wycats

@rust-highfive rust-highfive assigned brson and unassigned huonw Jan 29, 2016
@durka
Copy link
Contributor

durka commented Feb 1, 2016

This will be great. Maybe we can knock winapi off the most popular crates list!

@brson
Copy link
Contributor

brson commented Feb 2, 2016

This only supports a subset of Rust's attribute syntax. In particular, it doesn't support the 'list' meta item type, instead hardcoding 'all', 'not', and 'any'. ISTM that we should keep them at parity and support the same grammar in both places.

@alexcrichton
Copy link
Member Author

Ah that's because right now Rust doesn't actually allow #[cfg] directives defined by the platform to take the form of list meta items. I believe the syntax accepted here is the same as any valid #[cfg] attribute in Rust.

Would you prefer that the grammar itself is represented the same way, though, and then there's a pass afterwards to make sure it's a valid meta item by parsing but error'ing on strings like foo(bar)?

@alexcrichton alexcrichton force-pushed the target-specific-deps branch 2 times, most recently from a191553 to af294cf Compare February 9, 2016 21:24
@bors
Copy link
Collaborator

bors commented Feb 11, 2016

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

@alexcrichton alexcrichton force-pushed the target-specific-deps branch 2 times, most recently from e823157 to 5ec35c9 Compare February 12, 2016 23:27
@alexcrichton
Copy link
Member Author

Two more notes I have since realized:

  • When using a newer Cargo with an older Compiler, we won't be able to learn about --print cfg, so these new kinds of target-specific dependencies won't work. Cargo will just always return false indicating that the dependencies are not applicable, but there's also in theory room to do something like emit a warning.
  • Various #[cfg] lines sometimes get out of hand and are split across multiple lines, but the syntax here does not support that. TOML only allows keys to be defined on one line.

@bors
Copy link
Collaborator

bors commented Feb 14, 2016

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

This commit is an implementation of [RFC 1361][rfc] which is an extension of
Cargo's `target` section in `Cargo.toml` to allow the use of `#[cfg]`-like
expressions for target-specific dependencies. Now that the compiler has been
extended with `--print cfg` each invocation of Cargo will scrape this output and
learn about the relevant `#[cfg]` directives in play for the target being
compiled. Cargo will then use these directives to decide whether a dependency
should be activated or not.

This should allow definition of dependencies along the lines of:

    [target.'cfg(unix)'.dependencies]
    [target.'cfg(target_os = "linux")'.dependencies]
    [target.'cfg(windows)'.dependencies]

Which is much more ergonomic and robust than listing all the triples out!
@brson
Copy link
Contributor

brson commented Feb 16, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 16, 2016

📌 Commit f5d786e has been approved by brson

@bors
Copy link
Collaborator

bors commented Feb 16, 2016

⌛ Testing commit f5d786e with merge 4739ba1...

bors added a commit that referenced this pull request Feb 16, 2016
This commit is an implementation of [RFC 1361][rfc] which is an extension of
Cargo's `target` section in `Cargo.toml` to allow the use of `#[cfg]`-like
expressions for target-specific dependencies. Now that the compiler has been
extended with `--print cfg` each invocation of Cargo will scrape this output and
learn about the relevant `#[cfg]` directives in play for the target being
compiled. Cargo will then use these directives to decide whether a dependency
should be activated or not.

This should allow definition of dependencies along the lines of:

    [target.'cfg(unix)'.dependencies]
    [target.'cfg(target_os = "linux")'.dependencies]
    [target.'cfg(windows)'.dependencies]

Which is much more ergonomic and robust than listing all the triples out!
@bors
Copy link
Collaborator

bors commented Feb 16, 2016

SimonSapin added a commit to servo/servo that referenced this pull request Apr 26, 2016
* Sections like `[dependencies.foo]` can be entries in a `[dependencies]`
  section with the `{key = value}` syntax.
* Per-target dependencies can be expressed with more general `cfg(…)`
  conditions instead of exact target triples:
  rust-lang/cargo#2328
SimonSapin added a commit to servo/servo that referenced this pull request Apr 26, 2016
* Sections like `[dependencies.foo]` can be entries in a `[dependencies]`
  section with the `{key = value}` syntax.
* Per-target dependencies can be expressed with more general `cfg(…)`
  conditions instead of exact target triples:
  rust-lang/cargo#2328
bors-servo pushed a commit to servo/servo that referenced this pull request Apr 26, 2016
Simplify TOML syntax

* Sections like `[dependencies.foo]` can be entries in a `[dependencies]` section with the `{key = value}` syntax.
* Per-target dependencies can be expressed with more general `cfg(…)` conditions instead of exact target triples: rust-lang/cargo#2328

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10857)
<!-- Reviewable:end -->
@lilianmoraru
Copy link

Would of been nice if this syntax was supported on all of the options...
Example:

[workspace.'cfg(target_os = "linux")']
#...

# Example for the Cargo.toml present in the Rust source "rust/src/Cargo.toml"
[profile.release.'cfg(windows)']
opt-level = 2

[dependencies.'cfg(windows)']
winapi = "0.2"

[dev-dependencies.'cfg(target_os = "linux")']
libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer-sys.git" }

@ehuss ehuss mentioned this pull request Jul 26, 2019
bors added a commit that referenced this pull request Jul 26, 2019
Clean up TargetInfo

- The main motivation here is to provide a better error message when collecting information from rustc fails (it now shows the command and the output).
- Remove `has_cfg_and_sysroot`. I think this dates back to when it was introduced in #2328, as a guard for older versions of rustc that did not know about `--print=cfg`. Unless I'm missing something, I don't think we need to retain this backwards compatibility.
- Add some documentation.
- Demote the rustc cache log messages to `debug` level. I haven't really seen any caching issues, so I don't think it needs to be info level.
- Some other misc cleanup (remove unused function, etc.).
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
* Sections like `[dependencies.foo]` can be entries in a `[dependencies]` section with the `{key = value}` syntax.
* Per-target dependencies can be expressed with more general `cfg(…)` conditions instead of exact target triples: rust-lang/cargo#2328

Source-Repo: https://github.com/servo/servo
Source-Revision: 2729864af73d62719ea0fd55cef417c43bdd951e

UltraBlame original commit: 6710bf1dbafec084574fd82064e1f7896091bacf
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
* Sections like `[dependencies.foo]` can be entries in a `[dependencies]` section with the `{key = value}` syntax.
* Per-target dependencies can be expressed with more general `cfg(…)` conditions instead of exact target triples: rust-lang/cargo#2328

Source-Repo: https://github.com/servo/servo
Source-Revision: 2729864af73d62719ea0fd55cef417c43bdd951e

UltraBlame original commit: 6710bf1dbafec084574fd82064e1f7896091bacf
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
* Sections like `[dependencies.foo]` can be entries in a `[dependencies]` section with the `{key = value}` syntax.
* Per-target dependencies can be expressed with more general `cfg(…)` conditions instead of exact target triples: rust-lang/cargo#2328

Source-Repo: https://github.com/servo/servo
Source-Revision: 2729864af73d62719ea0fd55cef417c43bdd951e

UltraBlame original commit: 6710bf1dbafec084574fd82064e1f7896091bacf
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.

7 participants