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

fix: parse commit hash of GPG signed commit #953

Merged
merged 2 commits into from Feb 6, 2019

Conversation

Projects
None yet
3 participants
@sublimino
Copy link
Contributor

sublimino commented Feb 5, 2019

This change will support parsing the commit hash from GPG-signed commits. It does so by disabling signing output from the git CLI using git -c log.showSignature=false prior to the git subcommand.

@controlplaneio security policy mandates GPG signed commits, and we'd like to use this tool.

Closes #952

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 5, 2019

Codecov Report

Merging #953 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #953      +/-   ##
==========================================
+ Coverage   88.41%   88.43%   +0.01%     
==========================================
  Files          51       51              
  Lines        2461     2465       +4     
==========================================
+ Hits         2176     2180       +4     
  Misses        229      229              
  Partials       56       56
Impacted Files Coverage Δ
internal/testlib/git.go 100% <100%> (ø) ⬆️
internal/git/git.go 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd692bd...5a63e0f. Read the comment docs.

@caarlos0

This comment has been minimized.

Copy link
Member

caarlos0 commented Feb 5, 2019

wouldn't this work too? https://github.com/goreleaser/goreleaser/compare/master...quiet?expand=1

GitHub
Deliver Go binaries as fast and easily as possible - goreleaser/goreleaser
@@ -48,6 +48,7 @@ func fakeGit(args ...string) (string, error) {
"-c", "user.name='GoReleaser'",
"-c", "user.email='test@goreleaser.github.com'",
"-c", "commit.gpgSign=false",
"-c", "log.showSignature=false",

This comment has been minimized.

Copy link
@caarlos0

caarlos0 Feb 5, 2019

Member

this is not needed I believe...

@sublimino

This comment has been minimized.

Copy link
Contributor Author

sublimino commented Feb 6, 2019

wouldn't this work too? goreleaser/goreleaser/compare/master...quiet?expand=1

Not on latest Debian with git version 2.20.1. The flag in this PR is needed.

-q can be added to suppress further lines (although it seems they're discarded anyway).

An example of each type:


$ git show --format='%H' HEAD
gpg: Signature made Fri 11 Jan 2019 16:50:03 CET
gpg:                using RSA key 7DCF335A3BB2EC440C516D4C7B9EA6DD705D5F09
gpg: Good signature from "Andrew Martin <sublimino@gmail.com>" [ultimate]
e0754ec9a9cfc8cd9fe2de6b918235b44da916d1

diff --git a/x b/y
similarity index 100%
rename from x
rename to y

$ git show --format='%H' HEAD -q
gpg: Signature made Fri 11 Jan 2019 16:50:03 CET
gpg:                using RSA key 7DCF335A3BB2EC440C516D4C7B9EA6DD705D5F09
gpg: Good signature from "Andrew Martin <sublimino@gmail.com>" [ultimate]
e0754ec9a9cfc8cd9fe2de6b918235b44da916d1

This is the proposition in this PR:

$ git -c log.showSignature=false show --format='%H' HEAD
e0754ec9a9cfc8cd9fe2de6b918235b44da916d1

diff --git a/x b/y
similarity index 100%
rename from x
rename to y

This is this PR plus the suggested flag from #953 (comment):

$ git -c log.showSignature=false show --format='%H' HEAD -q
e0754ec9a9cfc8cd9fe2de6b918235b44da916d1

@sublimino sublimino force-pushed the sublimino:master branch from c634485 to 96ac904 Feb 6, 2019

@caarlos0

This comment has been minimized.

Copy link
Member

caarlos0 commented Feb 6, 2019

oh, ok, thanks for testing it... I couldn't reproduce this locally, even with signed commits... anyway, will merge this in.

thanks for the PR :)

@caarlos0 caarlos0 merged commit a6b60ce into goreleaser:master Feb 6, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
WIP ready for review
Details
@sublimino

This comment has been minimized.

Copy link
Contributor Author

sublimino commented Feb 6, 2019

Ah yes, that's because my ~/.gitconfig is set to show signatures. Thanks for the merge 🙏

[log]              
  showSignature = true   
@caarlos0

This comment has been minimized.

Copy link
Member

caarlos0 commented Feb 6, 2019

hmmm, that makes sense... anyway, it safer to prevent that =D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.