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

Record git hash of source tree in package #5629

Closed
dekellum opened this issue Jun 11, 2018 · 6 comments
Closed

Record git hash of source tree in package #5629

dekellum opened this issue Jun 11, 2018 · 6 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-publish

Comments

@dekellum
Copy link
Contributor

The cargo package step now requires that the source tree is clean (has no uncommitted changes).

The automating of git tag on release is proposed in #841 with some WIP.

As (previously) proposed, recording the git hash is compatible with these and would be immediately useful. In a comment on #2640 @alexcrichton suggests that "we could record the SHA" after ensuring a commit is published, etc. But I don't think it is necessary to ensure that a commit is pushed to its canonical repo in order for the SHA hash to be useful.

For example, there's a crate with 30K downloads where the author doesn't bother with a change log or even release tags. With best efforts, manually created release tags are sometimes forgotten or delayed, raising the question of what was the commit that was released. Even with #841 in place, a recorded hash would be reassuring. Tags can be changed. It may not be practical to automatically push tags, given workflow differences.

Utility:

  • If a SHA hash can be extracted (or seen on crates.io) and it matches a signed git tag in the canonical repository, that is very reassuring. A+!

  • Otherwise if it disagrees with (any) tag, but matches a plausible commit in the canonical tree, then that is at least somewhat reassuring, and possibly worth followup with the author.

  • If it doesn't match any commit in the canonical repo, that's a definite warning flag, and I have a starting point to followup with the author.

Eventually something like crates.io (and/or deps.rs) could actually automate the comparison of the SHA hash with a commit/tag in the canonical repo, warning if there is any disparity. This woundn't obviate the need for other forms of security, like rust-lang/crates.io#75, but it would be complementary.

@matklad matklad added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 26, 2018
@Eh2406
Copy link
Contributor

Eh2406 commented Jun 26, 2018

Sorry for the slow reply, the decisive people that have final say on these things are on vacation or otherwise unavailable. My impression is that they will be back soon.

FWIIW, I like the suggestion.

@alexcrichton
Copy link
Member

This also sounds like a great idea to me! We could pretty easily have something like .cargo-git-info.json which we automatically add to all published crates (if available)

@dekellum
Copy link
Contributor Author

dekellum commented Jun 29, 2018

Good to hear, thanks! I'm happy to help, ideally with some pointers, if you feel its easy enough or the "help-wanted" label is appropriate.

@alexcrichton
Copy link
Member

Oh assistance in implementing this would be most welcome! This feature would go somewhere around here most likely I believe

@luser
Copy link
Contributor

luser commented Jul 12, 2018

This also sounds like a great idea to me! We could pretty easily have something like .cargo-git-info.json which we automatically add to all published crates (if available)

For future expansion's sake maybe name it .cargo-vcs-info.json and leave room for distinguishing the VCS type, so someone could add support for hg later.

bors added a commit that referenced this issue Aug 20, 2018
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
@dwijnand
Copy link
Member

dwijnand commented Nov 9, 2018

Fixed in #5886

@dwijnand dwijnand closed this as completed Nov 9, 2018
est31 added a commit to est31/cargo-local-serve that referenced this issue Jan 12, 2019
Cargo has gained a feature where it records the package-time VCS commit:

* rust-lang/cargo#5886
* rust-lang/cargo#5629
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-publish
Projects
None yet
Development

No branches or pull requests

6 participants