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 workspace root to metadata command. #4940

Merged
merged 1 commit into from
Jan 14, 2018

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jan 14, 2018

Fixes #4933

Merge of #4938 for rust 1.24 beta.

@alexcrichton I'm uncertain about the process for merging in beta. I'm guessing after this I just need to open a PR on the rust beta branch to update the cargo submodule?

@rust-highfive
Copy link

r? @alexcrichton

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

@rust-highfive
Copy link

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against rust-1.24.0. Please double check that you specified the right target!

@alexcrichton
Copy link
Member

@ehuss indeed that should do it! Once this is merged we'll just need a PR for rust-lang/rust's beta branch.

Just to confirm before merging, @matklad you're ok w/ this right?

@matklad
Copy link
Member

matklad commented Jan 14, 2018

Yep, adding workspace root to metadata definitely seems useful!

@matklad
Copy link
Member

matklad commented Jan 14, 2018

Just to clarify, we use workspace dir and not the package did here because otherwise all packages will have src/lib.rs in debuginfo, and that would be ambiguous?

@alexcrichton
Copy link
Member

@matklad correct!

@bors: r+

@bors
Copy link
Collaborator

bors commented Jan 14, 2018

📌 Commit cc37b90 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Jan 14, 2018

⌛ Testing commit cc37b90 with merge 64326d7...

bors added a commit that referenced this pull request Jan 14, 2018
Add workspace root to metadata command.

Fixes #4933

Merge of #4938 for rust 1.24 beta.

@alexcrichton I'm uncertain about the process for merging in beta.  I'm guessing after this I just need to open a PR on the rust beta branch to update the cargo submodule?
@ehuss
Copy link
Contributor Author

ehuss commented Jan 14, 2018

Just to clarify, we use workspace dir and not the package did here because otherwise all packages will have src/lib.rs in debuginfo, and that would be ambiguous?

I don't think that was the original motivation, but you an Alex went back and forth about this in #4788 that introduced this change.

I just discovered that backtraces displayed on Windows are relative to the CWD when the panic happens (unless you have RUST_BACKTRACE="full" in which case they are absolute). Presumably this is because Windows debug symbols are stored absolute and RUST_BACKTRACE=1 wants to display a semi-compact backtrace (OSX is relative to the workspace in 1.24). The output from the panic! macro itself is relative to the workspace root, so I don't think this is too much of a problem, just something that was a little odd.

@bors
Copy link
Collaborator

bors commented Jan 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 64326d7 to rust-1.24.0...

@bors bors merged commit cc37b90 into rust-lang:rust-1.24.0 Jan 14, 2018
bors added a commit to rust-lang/rust that referenced this pull request Jan 18, 2018
Update cargo on beta.

This brings in rust-lang/cargo#4940 which adds `"workspace_root"` to `cargo metadata` so that tools can resolve the new relative paths in symbols and compiler messages.
bors added a commit to rust-lang/rust that referenced this pull request Jan 18, 2018
[beta] Update cargo on beta.

This brings in rust-lang/cargo#4940 which adds `"workspace_root"` to `cargo metadata` so that tools can resolve the new relative paths in symbols and compiler messages.
Elarnon added a commit to Elarnon/cbindgen that referenced this pull request Jul 26, 2018
cbindgen currently assumes that the `Cargo.lock` is in directly in the
crate directory as a sibling to `Cargo.toml`; however that is usually
not the case inside a workspace.

Instead, this PR extracts the workspace root from the output of `cargo
metadata` [1] (already used to get a list of packages) and uses the
`Cargo.lock` from there.

 1. This is available since Rust 1.24; see rust-lang/cargo#4940
Elarnon added a commit to Elarnon/cbindgen that referenced this pull request Jul 26, 2018
cbindgen currently assumes that the `Cargo.lock` is in directly in the
crate directory as a sibling to `Cargo.toml`; however that is usually
not the case inside a workspace.

Instead, this PR extracts the workspace root from the output of `cargo
metadata` [1] (already used to get a list of packages) and uses the
`Cargo.lock` from there.

 1. This is available since Rust 1.24; see rust-lang/cargo#4940
Elarnon added a commit to Elarnon/cbindgen that referenced this pull request Jul 26, 2018
cbindgen currently assumes that the `Cargo.lock` is in directly in the
crate directory as a sibling to `Cargo.toml`; however that is usually
not the case inside a workspace.

Instead, this PR extracts the workspace root from the output of `cargo
metadata` [1] (already used to get a list of packages) and uses the
`Cargo.lock` from there.

 1. This is available since Rust 1.24; see rust-lang/cargo#4940
eqrion pushed a commit to mozilla/cbindgen that referenced this pull request Jul 31, 2018
cbindgen currently assumes that the `Cargo.lock` is in directly in the
crate directory as a sibling to `Cargo.toml`; however that is usually
not the case inside a workspace.

Instead, this PR extracts the workspace root from the output of `cargo
metadata` [1] (already used to get a list of packages) and uses the
`Cargo.lock` from there.

 1. This is available since Rust 1.24; see rust-lang/cargo#4940
@ehuss ehuss added this to the 1.24.0 milestone Feb 6, 2022
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.

5 participants