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

How to detect workspace root? #4933

Closed
ehuss opened this issue Jan 11, 2018 · 3 comments · Fixed by #4938
Closed

How to detect workspace root? #4933

ehuss opened this issue Jan 11, 2018 · 3 comments · Fixed by #4938
Labels
Command-metadata

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 11, 2018

A recent change (#4788) changed it so that when you run cargo from within a workspace member, the paths in messages are now relative to the workspace root instead of the member package. I work on the Sublime integration, and the tool needs to know how to resolve the relative paths so that you can jump to the appropriate file when there is an error/warning. Currently it uses a simple logic of basing paths off the package's root. However, now it needs to know the workspace root, but I am uncertain how to find that. cargo metadata doesn't seem to offer any help. Is there some heuristic I can use?

Note: Currently the build tool doesn't know whether or not it is in a workspace. It blindly runs cargo build in the directory of the package.
@alexcrichton
Copy link
Member

Hm a good point! I actually don't think we've got a great way to learn about the workspace root, but for sure we should add it to cargo metadata!

@ehuss
Copy link
Contributor Author

ehuss commented Jan 12, 2018 •
edited
Loading

I can open a PR to add that. I have a few questions:

  • What do you think of just adding a new key "workspace_root", or should it be something else?
  • Does the metadata version need to change when adding keys? I would assume no, but the versioning rules don't seem to be defined.
  • Would it be feasible to backport this to beta? Without it I would need to recreate all the logic in parsing manifest files which sounds unpleasant. I assume all editors/IDEs/tools which read paths will need this (if they don't already know the workspace root).

@alexcrichton
Copy link
Member

Perhaps something like workspace_manifest: String? And yeah no need to bump the version, we'd only do that if we changed the existing version.

And sure yeah, we can backport to beta!

ehuss added a commit to ehuss/cargo that referenced this issue Jan 13, 2018
@ehuss
Add workspace root to metadata command.
fef215d 
Fixes rust-lang#4933
@ehuss ehuss mentioned this issue Jan 13, 2018
Merged
bors added a commit that referenced this issue Jan 13, 2018
@bors
Auto merge of #4938 - ehuss:ws-meta, r=alexcrichton
6a8eb71 
Add workspace root to metadata command.

Fixes #4933

@alexcrichton, you mentioned using `"workspace_manifest"`, but the `Workspace.root` function already strips off `Cargo.toml`.  It would be easy to append it back, though I'm uncertain if that's really necessary since I think for most use cases it will just need to be stripped off again.  Also, I feel like it might be confusing for non-workspace packages since `workspace_manifest` would be the same as the package `manifest_path` (and in that case it isn't really a workspace manifest).  I can easily change it, just let me know.
ehuss added a commit to ehuss/cargo that referenced this issue Jan 14, 2018
@ehuss
Add workspace root to metadata command.
cc37b90 
Fixes rust-lang#4933
@ehuss ehuss mentioned this issue Jan 14, 2018
Merged
bors added a commit that referenced this issue Jan 14, 2018
@bors
Auto merge of #4940 - ehuss:ws-meta-beta, r=alexcrichton
64326d7 
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?
@alexcrichton alexcrichton mentioned this issue Jan 15, 2018
Closed
@fmdkdd fmdkdd mentioned this issue Feb 3, 2018
Closed
@dpc dpc mentioned this issue Sep 9, 2018
Closed
@andreypopp andreypopp mentioned this issue Jan 23, 2019
Closed
petervdonovan added a commit to lf-lang/lingua-franca that referenced this issue Jan 13, 2022
@petervdonovan
Rust: Ask cargo what the package root is.
ca212f2 
This patches a tiny edge case that might not show up in practice. The problem is that the Rust code is being generated into an existing project (i.e., there is already a Cargo.toml file in the src-gen directory), which means that sometimes srcGenPkgPath != the actual package root (according to cargo). The cost of collecting and parsing the metadata is probably not negligible, so it would be nice if we didn't have to do this.

See rust-lang/cargo#4933.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Command-metadata
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add workspace root to metadata command. ehuss/cargo
2 participants
@ehuss @alexcrichton