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

Implement cargo print-source-root to expose where packages' sources live #1968

Closed

Conversation

carols10cents
Copy link
Member

Fixes #1098. This command will list all of this project's dependencies and where their source is on your file system.

The output is currently an alphabetical list of TOML-formatted dependency name/source location pairs, but that was just the first thing I could think of that would be useful to both people and programs. I'm totally open to changing the output to something else!

Looking forward to any other feedback yinz have!!! 🎉

So that other places can more easily use knowledge of overrides, not
just cargo compile.
@rust-highfive
Copy link

r? @huonw

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

@carols10cents
Copy link
Member Author

And of course I messed up the tests on windows :)

Fixes rust-lang#1098. This command will list all of this project's dependencies
and where their source is on your file system.

The output is an alphabetical list of TOML-formatted dependency
name/source location pairs.
@alexcrichton
Copy link
Member

I've been a little hesitant to push on this in the past because there's certainly a lot of design space here in terms of instrumenting Cargo to give back metadata about compilation. For example #1434 was a PR in the past to output even more metadata (but not the same set of data as this).

Originally Cargo was envisioned as being a set of disconnected subcommands that would sorta pipe output between one another to implement each step, and that way the internal plumbing could be reused externally as subcommands for tasks such as this (e.g. discovering where dependencies are located).

I think a good way to think about this may be to basically just take the function signatures of each of the functions in cargo::ops and have that be the output of a command like this. In theory we'd have a subcommand per layer, but we can always start off smaller! This is also just a vague idea about how we might approach metadata extraction from Cargo in a more principled manner.

Overall, on this PR specifically, my thoughts are:

  • The output should probably be something implementing RustcEncodable and then printed on stdout. That way we can switch the encoding between TOML and JSON easily. This was one of the original visions of Cargo (lots of machine readable output in various easily read formats) but we never really quite pushed on it hard enough to make it a reality.
  • I'm a little worried that you'll want more information than just the roots of all the dependencies. For example this is sorta the input to cargo::ops::cargo_rustc but there's a lot more contextual information in a Package which is left out here. I'd be hesitant to stabilize a command like this without the ability to extend it with more metadata in the future.
  • A good deal of the plumbing in this subcommand seems duplicated with other functions in ops, perhaps these can be refactored into just one function here and there?

cc @brson, @wycats, you guys may also have thoughts on this.

@brson
Copy link
Contributor

brson commented Sep 8, 2015

Not an opinion about this particular PR, but I have a use case just today where I need to know all the names of a crates dependencies so I can go cargo build -p {dependency} and build all the dependencies of a crate before building the crate itself. Is there such a way already? Seems like there might be overlap with this feature.

@alexcrichton
Copy link
Member

I don't think there's currently a great way of doing that today short of parsing Cargo.lock unfortunately

@carols10cents
Copy link
Member Author

Originally Cargo was envisioned as being a set of disconnected subcommands that would sorta pipe output between one another to implement each step, and that way the internal plumbing could be reused externally as subcommands for tasks such as this (e.g. discovering where dependencies are located).

I think a good way to think about this may be to basically just take the function signatures of each of the functions in cargo::ops and have that be the output of a command like this. In theory we'd have a subcommand per layer, but we can always start off smaller! This is also just a vague idea about how we might approach metadata extraction from Cargo in a more principled manner.

Ah I see! I'll take another look with this in mind... it feels like things are more tangled together than this right now, but I'll look for opportunities for different seams to make this more of a pipeline. I feel like I need to be able to understand and find all the pieces like:

  • Input: Cargo.toml and registry, Output: Cargo.lock
  • Input: Cargo.lock, Output: downloaded dependency sources into local locations
  • Input: Locations of downloaded dependencies, Output: Compiled dependencies
  • Input: Compiled dependencies, Output: rustc command for current package

I know this kind of nests and there are special cases in all of these, but reading through the open issues it sounds like there are use cases for being able to easily externally do each piece of this...

I'll work on understanding this better and potentially asking more specific questions/proposing specific, smaller changes to parts of this :)

The output should probably be something implementing RustcEncodable and then printed on stdout. That way we can switch the encoding between TOML and JSON easily.

OOOHH that would be cool! Definitely a piece I'm missing :)

A good deal of the plumbing in this subcommand seems duplicated with other functions in ops, perhaps these can be refactored into just one function here and there?

You caught me :) I definitely copy-pasted a bunch of stuff to get this to function. I'll look for more opportunities to reorganize and share code like in carols10cents@d1cd597.

I'm a little worried that you'll want more information than just the roots of all the dependencies. For example this is sorta the input to cargo::ops::cargo_rustc but there's a lot more contextual information in a Package which is left out here. I'd be hesitant to stabilize a command like this without the ability to extend it with more metadata in the future.

Yeah, I can see that. I'll keep that in mind working on the RustcEncodable part.

I think I'm going to close this PR for now and try making and submitting smaller refactoring changes to support this. Thank you so much for the comments and guidance, they're very helpful!!!

@alexcrichton
Copy link
Member

Ok, sounds good to me, and thanks for taking the initiative on this! I think that your general listing of inputs/outputs sounds pretty good to me, the Cargo types in the backend will also help reflect what's going on and what metadata can be provided (e.g. PackageId vs Package)

@mdinger
Copy link
Contributor

mdinger commented Sep 12, 2015

I think rustbyexample will be able to use something like the in the future. The rustbyexample TOC structure would be in the Cargo.toml and so cargo test would automatically test all examples. Then exporting the TOC via JSON or TOML to be input into the doc generator like gitbook or something similar would simplify both the build process and the be tested fairly simply.

Then the entire example set Cargo.toml contains would be of interest and exporting everything the Cargo.toml contains may end up being useful to someone.

bors added a commit that referenced this pull request Sep 22, 2015
Hi! This is an attempt to start refactoring some of the internals to be more like a pipeline, and eventually enable the kind of functionality I tried to add in #1968 without having to add as much duplication. Turns out there's a fair bit of duplication in the code today, I think this helps address it!

I may have totally gone against some abstractions... namely I made a way to create `Package`s from a `manifest_path` and a `config`, without needing a `Source`. I think it cleans up the code quite a bit, and I think makes things a bit more pipeliney in that the `Source` isn't updated until we really need it to be (as opposed to having to use `preload` to avoid updating it again). But I'm open to the possibility that I'm moving things around to where no one who knows the code well will be able to find them ;)

This *should* be a Real Refactor in the sense that these changes don't change behavior-- except in one test case, where the same error happens as did before, but it's going through a `chain_error` now so has a slightly different message.
@SimonSapin
Copy link
Contributor

#2193 adds a cargo metadata sub-command that contains among other things per-"target" src_path and per-package manifest_path keys.

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.

7 participants