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.

Sign up for GitHub

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

Jump to bottom

Implement features #509

Merged
merged 1 commit into from
Sep 23, 2014
Merged

Implement features #509

merged 1 commit into from
Sep 23, 2014

Conversation

alexcrichton
Copy link
Member

This feature was outlined in #385 1, and documentation has been included as
part of this commit.

Closes #385

@alexcrichton alexcrichton changed the title wip Implement features Sep 4, 2014
@alexcrichton
Copy link
Member Author

Oops, wrong title!

sfackler
sfackler reviewed Sep 5, 2014
View reviewed changes
src/doc/source/manifest.md
6. When a feature is selected, Cargo will call `rustc` with
`--cfg feature="${feature_name}"`. If a feature group is included,
all of its individual features will be included. This can be
tested in code via `#[cfg(cargo = "foo")]`
Copy link
Member

Choose a reason for hiding this comment

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

cargo or feature?

@sfackler
Copy link
Member

sfackler commented Sep 5, 2014

Is the definition of a dependency-less feature allowed? For example, rust-openssl has support for SSLv2 hidden behind a cfg flag: https://github.com/sfackler/rust-openssl/blob/4a823242abd136417939a49ebb121533381c01ca/src/ssl/mod.rs#L51. That kind of use case might be worth mentioning in the docs.

@alexcrichton
Copy link
Member Author

Aha, thanks for reminding me @sfackler! That caught a few bugs here and there, and I've put an explicit mention in the documentation now as well.

Some interesting changes to resolve:

  • Cyclic dependencies are now explicitly disallowed.
  • A test for two dependencies having the same name has been modified, because I believe this case to be impossible. The dependencies map in TOML is a hash map, so it's impossible to have two keys with the same name.

@alexcrichton alexcrichton mentioned this pull request Sep 10, 2014
Closed
@jeremyletang jeremyletang mentioned this pull request Sep 14, 2014
Closed
@gilles-leblanc gilles-leblanc mentioned this pull request Sep 16, 2014
Closed
@andrew-d
Copy link

@alexcrichton Any update on this?

@alexcrichton
Copy link
Member Author

I need to work on getting this rebased past the implementation of the registry, which will require some finesse that is not currently implemented.

@alexcrichton
Copy link
Member Author

Rebased, r? @wycats

@wycats
Copy link
Contributor

wycats commented Sep 22, 2014

r+

I think this will need a bit of cleanup at some point, but I'm ok with it as is.

@alexcrichton
Implement features
2b46d03 
This feature was outlined in rust-lang#385 [1], and documentation has been included as
part of this commit.

[1]: rust-lang#385 (comment)
bors added a commit that referenced this pull request Sep 23, 2014
@bors
auto merge of #509 : alexcrichton/cargo/features, r=wycats
af066ef 
This feature was outlined in #385 [1], and documentation has been included as
part of this commit.

[1]: #385 (comment)

Closes #385
@bors bors merged commit 2b46d03 into rust-lang:master Sep 23, 2014
@alexcrichton alexcrichton deleted the features branch September 23, 2014 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Design document for "features"
5 participants
@alexcrichton @sfackler @andrew-d @wycats @bors