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

Emit more info on --message-format=json #3319

Merged
merged 1 commit into from
Dec 6, 2016

Conversation

alexcrichton
Copy link
Member

This adds more output on Cargo's behalf to the output of
--message-format=json. Cargo will now emit a message when a crate is finished
compiling with a list of all files that were just generated along with a message
when a build script finishes executing with linked libraries and linked library
paths.

Closes #3212

@rust-highfive
Copy link

r? @brson

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

@alexcrichton
Copy link
Member Author

r? @brson

cc @jsgf, was there any other build metadata you were thinking of in particular for #3212? I left out the metadata/extra-filename bits as in theory those shouldn't be relevant so long as you have the filename itself I believe, but I'm curious if you have other purposes for them!

@jsgf
Copy link
Contributor

jsgf commented Nov 30, 2016

The full path should be fine - I don't have any separate need for the metadata hash.

I haven't tried this yet - does it also emit the set of paths to native libraries (and frameworks, I guess, for generality) that the crate depends on?

As a separate thing, is it possible to tell rustc not to incorporate a .a file's .os in the .rlib?

@jsgf
Copy link
Contributor

jsgf commented Nov 30, 2016

OK, I'm trying it out. It looks pretty close! I think I can probably replace my current set of hacks with this as-is.

That said, some notes:

It's a little odd I also need to specify -q to make it not emit the "Compiling foo" messages. I would think --message-format json would imply "don't emit any human-oriented stuff" (I didn't check to see if they were coming out of stdout vs stderr).

It might be nice to include the build mode (release/debug), features and cfgs and/or the complete set of flags passed to rustc.

Is the intent that the reason key in the record is to be used as a "record type identifier" - ie, every record will have a distinct reason field for distinct types of info? Is it also the case that the same key will have the same meaning in all record types (like package_id)?

It would be helpful if "lib" type targets also included the paths to any native libraries they're dependent on:

{
    "filenames": [
        "/Users/jsgf/git/cargo/target/release/deps/libopenssl_sys-5201f798d8034d0e.rlib"
    ],
    "package_id": "openssl-sys 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)",
    "reason": "compiler-artifact",
    "target": {
        "kind": [
            "lib"
        ],
        "name": "openssl-sys",
        "src_path": "/Users/jsgf/.cargo/registry/src/github.com-1ecc6299db9ec823/openssl-sys-0.9.1/src/lib.rs"
    }
}

I think this information is available if I:

  1. find the corresponding reason: build-script-executed record
  2. get the linked_libs and linked_paths fields and construct pathnames for the libraries from that.

But that depends on there having been a build-script. Do crates with native dependencies always have a build script?

It would be more straightforward if the compiler-artifact record just explicitly listed all the link-time dependencies it has (ideally, including to other rlibs).

When I build cargo itself with --message-format json, the final thing it emits is:

{
    "filenames": [
        "/Users/jsgf/git/cargo/target/debug/deps/cargo-a8eb8e92f78bce89"
    ],
    "package_id": "cargo 0.16.0 (path+file:///Users/jsgf/git/cargo)",
    "reason": "compiler-artifact",
    "target": {
        "kind": [
            "bin"
        ],
        "name": "cargo",
        "src_path": "src/bin/cargo.rs"
    }
}

The deps/cargo-a8eb8e92f78bce89 filename is a little unexpected; I would expect it to at least also include the target/debug/cargo hardlink path as well, since that's the conventional name for a normal executable.

@jsgf
Copy link
Contributor

jsgf commented Nov 30, 2016

Also, when I get a compilation failure, I get a "reason: compiler-message" record, but then its followed by error: Could not compile cargo, complete with ANSI colouring.

@brson
Copy link
Contributor

brson commented Dec 1, 2016

Seems ok to me once @jsgf is happy.

@alexcrichton
Copy link
Member Author

does it also emit the set of paths to native libraries (and frameworks, I guess, for generality) that the crate depends on?

I believe so, yes

It might be nice to include the build mode (release/debug), features and cfgs and/or the complete set of flags passed to rustc.

Ah yeah I can definitely include that. I'll try to think something through here.

Is the intent that the reason key in the record is to be used as a "record type identifier" - ie, every record will have a distinct reason field for distinct types of info?

Indeed! (sorry for the almost nonexistent docs about this right now)

Is it also the case that the same key will have the same meaning in all record types (like package_id)?

I hadn't explicitly thought of this, but yeah I would want this to be true in any case.

It would be helpful if "lib" type targets also included the paths to any native libraries they're dependent on:

That should come up with the build script messages, but were those dropped? If there's library linkage included in the source then unfortunately Cargo doesn't have that information to print back :(

Do crates with native dependencies always have a build script?

Most of the time, yes, although #[link] the source still comes up from time to time.

The deps/cargo-a8eb8e92f78bce89 filename is a little unexpected

Yeah this was a recent change which helps us cache a bit more (but we update the hardlinks still. I can update the path to print out the hardlink location, though, if you'd prefer.

@bors
Copy link
Collaborator

bors commented Dec 2, 2016

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

This adds more output on Cargo's behalf to the output of
`--message-format=json`. Cargo will now emit a message when a crate is finished
compiling with a list of all files that were just generated along with a message
when a build script finishes executing with linked libraries and linked library
paths.

Closes rust-lang#3212
@alexcrichton
Copy link
Member Author

Ok I've updated with profile information (debuginfo, debug asserts, opt level, etc) and features. @jsgf was there more info you're thinking of that you'd like to see?

@jsgf
Copy link
Contributor

jsgf commented Dec 2, 2016

Ooh, looks good. I'll try to update my script to use it this afternoon.

"debuginfo":false - isn't debuginfo a number to set different levels?

@alexcrichton
Copy link
Member Author

In theory, yeah, although Cargo doesn't quite have support for that just yet. I'll switch that though I think as it's more future proof to use a number.

@jsgf
Copy link
Contributor

jsgf commented Dec 3, 2016

Yeah, good. I got a successful build out of scripts modified to use this.

@alexcrichton
Copy link
Member Author

Awesome! Let's land this then

@bors: r=brson

@bors
Copy link
Collaborator

bors commented Dec 5, 2016

📌 Commit 9f6d798 has been approved by brson

@bors
Copy link
Collaborator

bors commented Dec 5, 2016

⌛ Testing commit 9f6d798 with merge 0116caa...

@bors
Copy link
Collaborator

bors commented Dec 5, 2016

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member Author

alexcrichton commented Dec 5, 2016 via email

@bors
Copy link
Collaborator

bors commented Dec 5, 2016

⌛ Testing commit 9f6d798 with merge 75ab8ff...

@bors
Copy link
Collaborator

bors commented Dec 5, 2016

💔 Test failed - status-travis

@jsgf
Copy link
Contributor

jsgf commented Dec 5, 2016

Looks like that failure was just a timeout.

@jsgf
Copy link
Contributor

jsgf commented Dec 5, 2016

@bors: retry

@alexcrichton
Copy link
Member Author

alexcrichton commented Dec 6, 2016 via email

@bors
Copy link
Collaborator

bors commented Dec 6, 2016

⌛ Testing commit 9f6d798 with merge 16cc83e...

bors added a commit that referenced this pull request Dec 6, 2016
Emit more info on --message-format=json

This adds more output on Cargo's behalf to the output of
`--message-format=json`. Cargo will now emit a message when a crate is finished
compiling with a list of all files that were just generated along with a message
when a build script finishes executing with linked libraries and linked library
paths.

Closes #3212
@bors
Copy link
Collaborator

bors commented Dec 6, 2016

☀️ Test successful - status-appveyor, status-travis
Approved by: brson
Pushing 16cc83e to master...

@bors bors merged commit 9f6d798 into rust-lang:master Dec 6, 2016
@alexcrichton alexcrichton deleted the more-info branch December 8, 2016 19:48
bors added a commit that referenced this pull request Feb 27, 2017
Always produce artifact messages

This changes `artifact` messages in several ways:

* They are produced even for fresh builds

* They used the path after hard linking (@jsgf talked about it in the end of #3319 (comment))

* Don't produce filenames if the compiler has not actually produced the binaries (`-Z-no-trans`).
@ehuss ehuss added this to the 1.15.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.

Export more build metadata from cargo build
6 participants