Generate .cargo_vcs_info.json and include in cargo package (take 2)
#5886
Conversation
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
src/cargo/ops/cargo_package.rs
Outdated
| format!("failed to add to archive: `{}`", fnd) | ||
| })?; | ||
| let mut buff = Cursor::new(Vec::with_capacity(256)); | ||
| writeln!(buff, "{}", serde_json::to_string_pretty(json)?)?; |
There was a problem hiding this comment.
I think the Cursor isn't necessary here, right? This could just use to_string and then get out a Vec<u8> which could be passed in below as a slice?
|
Looks good to me, thanks! Just one small nit but otherwise r=me |
Per github review.
|
@alexcrichton Fixed the ´nit, which does simplify that portion, thanks for pointing that out. Also fixed a new clippy lint associated with this PR's changes. Cross review as of 06dcc20? |
|
@bors: r+ Thanks again for this! |
|
Thank you! Bors is kind of slow at moment then, I must assume. |
|
📌 Commit 06dcc20 has been approved by |
Generate .cargo_vcs_info.json and include in `cargo package` (take 2) Implements #5629 and supersedes #5786, with the following improvements: * With an upstream git2-rs fix (tracked #5823, validated and min version update in: #5858), no longer requires windows/unix split tests. * Per review in #5786, drops file system output and locks for .cargo_vcs_info.json. * Now uses serde `json!` macro for json production with appropriate escaping. * Now includes a test of the output json format. Possible followup: * Per discussion in #5786, we could improve reliability of both the VCS dirty check and the git head sha1 recording by preferring (and/or warning otherwise) the local repository bytes of each source file, at the same head commit. This makes the process more appropriately like an atomic snapshot, with no sentry file or other locking required. However given my lack of a window's license and dev setup, as exhibited by troubles of #5823, this feel intuitively like too much to attempt to change in one iteration here. I accept the "best effort" concept of this feature as suggested in #5786, and think it acceptable progress if merged in this form. @alexcrichton r? @joshtriplett cc
|
☀️ Test successful - status-appveyor, status-travis |
Cargo has gained a feature where it records the package-time VCS commit: * rust-lang/cargo#5886 * rust-lang/cargo#5629
Implements #5629 and supersedes #5786, with the following improvements:
With an upstream git2-rs fix (tracked No git repo found on windows (only) for package_verbose test #5823, validated and min version update in: Improve verbose console and log for finding git repo in package check #5858), no longer requires windows/unix split tests.
Per review in Generate .cargo_vcs_info.json and include in
cargo package#5786, drops file system output and locks for .cargo_vcs_info.json.Now uses serde
json!macro for json production with appropriate escaping.Now includes a test of the output json format.
Possible followup:
cargo package#5786, we could improve reliability of both the VCS dirty check and the git head sha1 recording by preferring (and/or warning otherwise) the local repository bytes of each source file, at the same head commit. This makes the process more appropriately like an atomic snapshot, with no sentry file or other locking required. However given my lack of a window's license and dev setup, as exhibited by troubles of No git repo found on windows (only) for package_verbose test #5823, this feel intuitively like too much to attempt to change in one iteration here. I accept the "best effort" concept of this feature as suggested in Generate .cargo_vcs_info.json and include incargo package#5786, and think it acceptable progress if merged in this form.@alexcrichton r?
@joshtriplett cc