x/review/git-codereview: set git config commit.gpgsign to false

hnakamur commented 2 weeks ago

Go version

go version go1.22.2 linux/amd64

Output of `go env` in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/hnakamur/.cache/go-build'
GOENV='/home/hnakamur/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/hnakamur/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/hnakamur/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/hnakamur/ghq/go.googlesource.com/review/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2151546368=/tmp/go-build -gno-record-gcc-switches'

What did you do?

git config --global commit.gpgsign true git clone https://go.googlesource.com/review cd review go test -v

What did you see happen?

Errors like below occurred:

=== RUN   TestSubmit
    submit_test.go:131: in git-client/, ran [git tag -f work.mailed] with git version 2.39.2
        :
        exit status 1
        error: There was a problem with the editor 'false'.
        Please supply the message using either -m or -F option.
--- FAIL: TestSubmit (0.43s)

What did you expect to see?

All test passes.

dmitshur commented 2 weeks ago

Thanks for reporting this.

The git configuration surface is quite large, and this appears to be a case where one of the configuration values conflicts with the expectations of git-codereview, causing test errors.

Can you clarify if this is causing a problem for your workflow? Or are you simply reporting it because you spotted it?

While git-codereview can be updated for this particular case, it might not be viable to do that holistically. An alternative approach is to not to use this configuration when working with Go repositories. For example, that can be done by running git config --local commit.gpgsign false inside Go repositories where you need to use git-codereview. Would that work?

hnakamur commented 2 weeks ago

Thanks for your comment.

I reported this simply I spotted it. Sorry I was not accurate about the description in "What did you do?" section. Actually I happen to set commit.gpgsign and tag.gpgsign to true in my ~/.gitconfig.

I tried again with setting commit.gpgsign and tag.gpgsign to false locally:

$ git config --local commig.gpgsign false
$ git config --local tag.gpgsign false

Verify the configs:

$ git config --get-all commit.gpgsign
true
false
$ git config --get-all tag.gpgsign
true
false

Test still fails, for example:

$ go test -v -run TestHookPreCommitEnv
=== RUN   TestHookPreCommitEnv
    hook_test.go:328: in git-client/, ran [git tag work] with git version 2.39.2
        :
        exit status 1
        error: There was a problem with the editor 'false'.
        Please supply the message using either -m or -F option.
--- FAIL: TestHookPreCommitEnv (0.30s)
FAIL
exit status 1
FAIL    golang.org/x/review/git-codereview      0.304s

The commit used for test is:

$ git rev-parse HEAD
c91ae924997076a8e6e6f16d1d9fb75f812e0cdd

However with the following diff (in my local git branch):

$ git diff HEAD~
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 931d017..932e7ab 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -200,6 +200,7 @@ func newGitTest(t *testing.T) (gt *gitTest) {
                write(t, client+"/.git/hooks/"+h, "#!/bin/sh\nexit 0\n", 0755)
        }

+       trun(t, client, "git", "config", "tag.gpgsign", "false")
        trun(t, client, "git", "config", "core.editor", "false")
        pwd, err := os.Getwd()
        if err != nil {

Now the test passes:

$ go test -v -run TestHookPreCommitEnv
=== RUN   TestHookPreCommitEnv
    hook_test.go:335: git-codereview hook-invoke pre-commit
    hook_test.go:335: stderr:
        git-codereview pre-commit gofmt hook disabled by $GIT_GOFMT_HOOK=off
--- PASS: TestHookPreCommitEnv (0.35s)
PASS
ok      golang.org/x/review/git-codereview      0.354s

golang / go