Skip to content

Fix panic in cargo metadata with dep on bin - #5558

Closed
alexcrichton wants to merge 1 commit into
rust-lang:masterfrom
alexcrichton:fix2
Closed

Fix panic in cargo metadata with dep on bin#5558
alexcrichton wants to merge 1 commit into
rust-lang:masterfrom
alexcrichton:fix2

Conversation

@alexcrichton

Copy link
Copy Markdown
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
Copy Markdown

r? @matklad

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

@alexcrichton

Copy link
Copy Markdown
Member Author

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

@matklad

matklad commented May 24, 2018

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

bors commented May 27, 2018

Copy link
Copy Markdown
Contributor

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

@alexcrichton

Copy link
Copy Markdown
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