Skip to content

Fix panic in cargo metadata with dep on bin #5558

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

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

This fixes an accidental unwrap on a package which doesn't actually have a lib
target in cargo metadata. It also refactors along the way to return a Result
to propagate the error from packages.get

Closes #5548

@rust-highfive
Copy link

r? @matklad

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

@alexcrichton
Copy link
Member Author

This'll also need to be backported to beta as it's there as well

@matklad
Copy link
Member

matklad commented May 24, 2018

Interesting! I am not quite sure that this is the best behavior: ideally, each dependency should correspond to an extern crate declaration, so manufacturing some arbitray name for a binary-only crate does not feel like a right solution here. Perhaps the better option is to just omit this dependency altogether?

However, I am actually not happy about this new dependencies format at all: #5461 (comment).

Perhaps we should just revert #5461 for now? I am not sure I'll have bandwidth to handle this for the next couple of weeks.

This fixes an accidental `unwrap` on a package which doesn't actually have a lib
target in `cargo metadata`. It also refactors along the way to return a `Result`
to propagate the error from `packages.get`

Closes rust-lang#5548
@alexcrichton
Copy link
Member Author

Sure yeah seems reasonable at least in isolation for this to drop the dependency edge. I don't have too many thoughts about whether or not we should revert, I'd be fine either way

matklad added a commit to matklad/cargo that referenced this pull request May 27, 2018
…chton"

This reverts commit d0d3cb5, reversing
changes made to 757112c.

It is unclear if the design is right, see
rust-lang#5558 (comment)
matklad added a commit to matklad/cargo that referenced this pull request May 27, 2018
…chton"

This reverts commit d0d3cb5, reversing
changes made to 757112c.

It is not clear that the design is right, see
rust-lang#5558 (comment)
bors added a commit that referenced this pull request May 27, 2018
Revert "Auto merge of #5461 - matklad:meta-rename, r=alexcrichton"

This reverts commit d0d3cb5, reversing
changes made to 757112c.

It is unclear if the design is right, see
#5558 (comment)
bors added a commit that referenced this pull request May 27, 2018
Revert "Auto merge of #5461 - matklad:meta-rename, r=alexcrichton"

This reverts commit d0d3cb5, reversing
changes made to 757112c.

It is not clear that the design is right, see
#5558 (comment)

this is the backport sibling of #5576
@bors
Copy link
Contributor

bors commented May 27, 2018

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

@alexcrichton
Copy link
Member Author

No need for this any more!

@alexcrichton alexcrichton deleted the fix2 branch May 27, 2018 18:27
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.

cargo metadata panics when you have a dependency to a bin crate
4 participants