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

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

Closed
hnakamur opened this issue May 15, 2024 · 4 comments
Closed

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

hnakamur opened this issue May 15, 2024 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@hnakamur
Copy link
Contributor

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.

@gopherbot gopherbot added this to the Unreleased milestone May 15, 2024
@dmitshur
Copy link
Contributor

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?

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 17, 2024
@hnakamur
Copy link
Contributor Author

hnakamur commented May 17, 2024

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

@seankhliao
Copy link
Member

I think this is just about the unit test in git-codereview which creates new repositories?

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jun 15, 2024
@gopherbot
Copy link
Contributor

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants