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 structs output by --message-format=json? #45

Closed
alecmocatta opened this issue Jul 26, 2018 · 5 comments
Closed

Add structs output by --message-format=json? #45

alecmocatta opened this issue Jul 26, 2018 · 5 comments

Comments

@alecmocatta
Copy link
Contributor

I'm using the following to parse cargo build --message-format=json. I see other crates have a similar redefinition of this, for example assert_cmd.

They are apparently stable. I'm wondering if it's worth making cargo_metadata the canonical place for these?

use cargo_metadata::Target;

// https://github.com/rust-lang/cargo/blob/c24a09772c2c1cb315970dbc721f2a42d4515f21/src/cargo/util/machine_message.rs
#[derive(Deserialize, Debug)]
#[serde(tag = "reason", rename_all = "kebab-case")]
pub enum Message {
	CompilerArtifact {
		#[serde(flatten)]
		artifact: Artifact,
	},
	CompilerMessage {},
	BuildScriptExecuted {},
	#[serde(skip)]
	Unknown, // TODO https://github.com/serde-rs/serde/issues/912
}

#[derive(Deserialize, Debug)]
pub struct Artifact {
	pub package_id: String,
	pub target: Target,
	pub profile: ArtifactProfile,
	pub features: Vec<String>,
	pub filenames: Vec<PathBuf>,
	pub fresh: bool,
}

#[derive(Deserialize, Debug)]
pub struct ArtifactProfile {
	pub opt_level: String,
	pub debuginfo: Option<u32>,
	pub debug_assertions: bool,
	pub overflow_checks: bool,
	pub test: bool,
}
@oli-obk
Copy link
Owner

oli-obk commented Jul 26, 2018

Feels right. We'd need to update all the docs to mention that it's not just about cargo metadata anymore, but any machine processable metadata that cargo outputs

@epage
Copy link

epage commented Sep 10, 2018

Author of escargot here, a crate that provides an API over various cargo commands

Just so I know where we might fall on collaboration, consolidation, or orthogonality of effort, @oli-obk what is your expected scope of this kind of work? Just defining the structs? Also pulling in an API that calls cargo and returns the data for the structs?

I'm using the following to parse cargo build --message-format=json. I see other crates have a similar redefinition of this, for example assert_cmd.

btw the reason for escargot does not include struct definitions is to ensure clients have the control they need for reducing overhead.

  • Clients can choose their allocation policy (Cow / &str vs String)
  • Clients can ignore deserializing any parts of the payload they don't care about.

Probably not worth it with all the other overhead in these kind of operations.

@oli-obk
Copy link
Owner

oli-obk commented Sep 11, 2018

what is your expected scope of this kind of work? Just defining the structs? Also pulling in an API that calls cargo and returns the data for the structs?

The full thing. Structures and a convenient API. I think the correct collaboration here would be to define the structures here, and use all the call-logic of escargot by adding it as a dependency and either exposing it directly or just using it and exposing a simpler API. Not sure what the correct course of action is here. I don't really want to go into extension trait territory.

Alternatively we could merge the two crates and put the structs and serde stuff behind a cargo feature.

@epage
Copy link

epage commented Sep 11, 2018

For background, when I created escargot, I didn't think of trying to contribute what I was doing to this crate because I thought the focus of this was just on cargo-metadata.

The advantage of cargo_metadata as-it-is is that it is smaller and more clearly defined. Providing a general purpose cargo API (without the problems of linking against the cargo crate) is a large problem which I'm assuming will take a lot of work and iteration to get to a happy, 1.0 state.

If you want to take that on, I won't stop you and will support you. For me personally, I think I'd keep these as separate and have this issue and relevant PR redirected over to escargot.

As a side note, I saw interest, at least from killercup, to modularize the cargo crate. I doubt it will be able to replace the use case for either of our crates but there might be interesting parts to integrate with.

@ehuss
Copy link
Contributor

ehuss commented Nov 30, 2019

I think this issue is essentially resolved by #64.

@oli-obk oli-obk closed this as completed Nov 30, 2019
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 a pull request may close this issue.

4 participants