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

Automatic features. #1787

Closed
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions 0000-conditional_dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
- Feature Name: `conditional_dependencies`
- Start Date: 2016-11-05
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Support "conditional" dependencies, which will be used only if that dependency
is already present. Users will be able to write code to integrate with another
library (for example, serializations for their type) without introducing a
*necessary* dependency on that library for users who don't need it.

# Motivation
[motivation]: #motivation

Currently, a common pattern exists for libraries to have feature flags which
turn on a dependency. For example, if I have a library which defines a new
type, I might want users to be able to serialize it with `serde`, but I don't
want users who don't care about this functionality to have to add `serde` to
their dependency graph. So I add a `serialization` feature to my Cargo.toml,
and only if you turn on this feature do you get the feature for serialization.

Ultimately, the source of this problem is the orphan rules - because only *I*
can define the impls for my type, I have to add this dependency. For every
commonly used trait defining library, I have to add a new feature, and all of
my clients have to turn on that feature.

cargo features have many uses, but this particular pattern ends up being a lot
of fiddling for a very common use case. Instead of requiring users go through
the feature flag rigmarole, we can provide first class support for this
pattern through "conditional dependencies" - dependencies that are turned on
automatically if they were already present.

# Detailed design
[design]: #detailed-design

## Conditional dependencies in `Cargo.toml`

When declaring a dependency in a Cargo.toml file, if that dependency is an
object, it can contain the `conditional` member, which is a boolean, similar to
the `optional` dependency. For example:

```toml
[dependencies]
foobar = { version = "1.0.0", conditional = true }
```

Or:

```toml
[dependencies.foobar]
version = "1.0.0"
conditional = true
```

A single dependency objecy cannot contain both a `conditional` key and an
Copy link
Contributor

Choose a reason for hiding this comment

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

s/objecy/object

`optional` key.

When generating the dependency graph, cargo will not include dependencies
tagged conditional. It will then traverse the graph looking for conditional
dependencies; if a matching source for a conditional dependency (e.g. a
matching version number) is already present in the graph, cargo will
automatically add that dependency.

## `#[cfg(dependency=)]`
Copy link
Member

@Manishearth Manishearth Nov 6, 2016

Choose a reason for hiding this comment

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

Could we continue to use cfg(feature=) and specify conditional deps in Cargo as

[features.serialization]
crates = ["serde", "blah"]
magic = true #please bikeshed the name

instead? I don't really like creating a new system which works mostly like cargo features but is incompatible with them.

This also lets you specify features that should be brought in only when more than one crate is included, which the current proposal does not. It also means that there can be a straightforward transition -- folks can continue to specify optional dependencies via explicit features for crates which have switched to magic dependencies. It's all nice and interoperable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought a bit about this and there are two syntaxes which are equivalent that we can decide between, will write up more in a comment but it probably won't be today.

Copy link
Contributor

Choose a reason for hiding this comment

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

The magic route is what Cabal uses for its flags, basically.

Somewhat relatedly, I think target.cfg(....).foo syntax is really good---way better than optional syntax. I would love:

[targets.'cfg(and(or(feature = "foo", feature = "bar"), create = "baz"))'.asdf]
#...


A new kind of cfg attribute will be added to rustc. The `dependency` attribute
will only be compiled when a certain dependency has been explicitly passed to
rustc using the --extern flag.

Because cargo automatically passes dependencies explicitly with this command,
Copy link
Member

Choose a reason for hiding this comment

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

I think cargo build -p foo may need to be tweaked to support this case. Minor issue.

this code will be compiled only when cargo triggers the conditional dependency.

## Convention for using conditional dependencies

The above to features can be composed in any way, but to assist in adopting
this functionality, we can define an encouraged convention.

If you want to add a new conditional dependency, you should add a top level
submodule (that is, mounted directly under lib, main, etc) with the same name
as the conditional dependency. This module will be tagged with the cfg, and the
extern crate declaration and all code that uses symbols from that crate will
go inside of that module.

For example:

```rust
/// lib.rs
Copy link
Member

Choose a reason for hiding this comment

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

/// is definitely not what you wanted.


#[cfg(dependency=foobar]
Copy link
Member

Choose a reason for hiding this comment

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

#[cfg(dependency = "foobar")]

mod foobar;
```

```rust
/// foobar.rs

extern crate foobar;
Copy link
Member

Choose a reason for hiding this comment

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

Note that this is a departure from the very strong convention of only using extern crate in the crate root. It also has substantial potential to be confusing, because now for to import something from that crate you’d need use self::foobar::Bar; or use foobar::foobar::Bar; rather than use foobar::Bar;.


impl foobar::Bar for MyType {
...
}
```

## Preventing cyclic dependencies on upload to crates.io

When packaging and uploading a crate to crates.io, some strategy will be used
to ensure that its conditional dependencies could not introduce a cycle into
the dependency graph of your crate.

## `cargo doc` compiles with all dependencies

Unlike other forms of compilation, the `cargo doc` command will treat
conditional dependencies as present by default, in order to document the APIs
which exist only when conditional dependencies are present. The conditional
dependency may be provided somehow.
Copy link
Member

Choose a reason for hiding this comment

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

The conditional dependency may be provided somehow.

I don’t know what you mean. The sentence doesn’t make sense. Do you mean documented, i.e. some kind of banner, like with the stability attributes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is supposed to be documented, yes, will update


# Drawbacks
[drawbacks]: #drawbacks

This adds more functionality to both cargo and rustc. The distinction between
"conditional" dependencies and "optional" dependencies is sort of subtle and
will have to be explained to users. More complexity always has a downside.

# Alternatives
[alternatives]: #alternatives

We considered options like allowing the orphan rules to be broken by certain
Copy link
Member

Choose a reason for hiding this comment

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

FWIW the application of sibling crates is much more than just optional dependencies.

One example is Servo. Servo is nobody's dependency, and has a bunch of crates, split up for compile times. Between these crates it would be lovely if we could bypass the orphan rules -- the main reason for the orphan rules is so that downstream users shouldn't get conflict issues that they're powerless to fix (without editing their dependencies directly), but in this case we have no downstream users -- the downstream crates are all Servo crates, and if a change in layout forces me to make a change in style to avoid an ambiguity, so be it. All the Servo crates should really be considered as a single whole wrt orphan rules.

I recently was forced to merge three crates to get around orphan rule issues (and other things).

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, this isn't going to solve this problem. The solution to this problem is incremental compilation and then don't break up your projects into crates just to get coarse grained incremental compilation.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's a bit extreme though. We still want there to be crates for code organization and code reuse (for example now Firefox uses our style crate), but it would be nice if orphan rules could be broken amongst these crates.

(Yes, in this case that means we do have downstream users, but that's C++ code)

Copy link
Contributor Author

@withoutboats withoutboats Nov 6, 2016

Choose a reason for hiding this comment

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

In my opinion any solution that means that rustc is no longer able to enforce the orphan rules without cargo is hard stop untenable. I'm open to any way to resolve the papercuts of the orphan rules that doesn't break that rule.

The other approach to this problem I've been taking is to push specialization's rules further so that we can define flexible blanket impls, so that hopefully you can get several trait impls for your type for free. Hopefully this would apply to Servo also.

Copy link
Member

Choose a reason for hiding this comment

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

I'm open to any way to resolve the papercuts of the orphan rules that doesn't break that rule.

Yeah, I think we can design this to work without cargo. But this is pretty off topic, just wanted to point out (like sgrif) that the orphan rules have a few more problems that this can't help with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think we can design this to work without cargo.

Cool. :-) I'm always very excited to hear new ideas to make the orphan rules less of a burden. I'd love to continue this conversation off of this RFC thread.

crates that were recognized as "sibling" crates by a parent, as a way to get
around the orphan rule issue. However, this alternative would create a
soundness hole in the orphan rules and violate good layering, by making them
only fully enforced by cargo and not by rustc. Ultimately, conditionally
altering the shape of code compiled by the flags passed by cargo seemed like
a better solution to the problem.

# Unresolved questions
[unresolved]: #unresolved-questions

The specific mechanisms used by cargo and crates.io to determine conditional
dependencies and to prevent cycles are rather unspecified in this RFC. It may
have interactions with other cargo features that haven't been fully thought
through.