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

Add a warning when packaging crates with wildcard dependencies #2005

Merged
merged 2 commits into from Sep 29, 2015

Conversation

Projects
None yet
4 participants
@sfackler
Copy link
Member

sfackler commented Sep 28, 2015

r? @alexcrichton

Any ideas on how to make a reliable test for this? Making a test with multiple dependencies on stuff off of crates.io seems to make output pretty nondeterministic wrt download and compilation ordering.

@achanda

This comment has been minimized.

Copy link
Contributor

achanda commented Sep 28, 2015

Shouldn't we let the end user know as soon as possible (i.e. on a cargo build)?

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented Sep 28, 2015

I'd lean towards no. Just like the checks that already exist for missing descriptions, licenses, documentation links, etc, these limitations only really matter when you're about to publish, and they're easy to fix at that point.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 28, 2015

This actually explicitly doesn't want to warn on a cargo build because only crates.io will be rejecting * dependencies, not Cargo itself. This means that applications and such are free to use * still, it's just libraries on crates.io that won't be allowed to use it.

There should be existing instances in tests/test_cargo_registry.rs about how to make a fake registry which doesn't do anything on the network, so perhaps one of those could be re-used to add a test?

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented Sep 28, 2015

Cool. Will take a look tonight.

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented Sep 29, 2015

Updated with test.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 29, 2015

@bors: r+ 28126c1

Thanks!

bors added a commit that referenced this pull request Sep 29, 2015

Auto merge of #2005 - sfackler:wildcard-warnings, r=alexcrichton
r? @alexcrichton 

Any ideas on how to make a reliable test for this? Making a test with multiple dependencies on stuff off of crates.io seems to make output pretty nondeterministic wrt download and compilation ordering.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 29, 2015

⌛️ Testing commit 28126c1 with merge ec9398e...

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 29, 2015

💔 Test failed - cargo-linux-32

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 29, 2015

Fascinating! That's the first time I've ever seen that failure outside of rust-lang/rust's bots...

@bors: retry

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 29, 2015

⚡️ Previous build results for cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64 are reusable. Rebuilding only cargo-linux-32...

@bors

This comment has been minimized.

@bors bors merged commit 28126c1 into rust-lang:master Sep 29, 2015

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.