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

gitconfig gpgsign = true should only sign signed-off-by commits #64

Open
thardeck opened this issue Apr 19, 2018 · 8 comments
Open

gitconfig gpgsign = true should only sign signed-off-by commits #64

thardeck opened this issue Apr 19, 2018 · 8 comments

Comments

@thardeck
Copy link

thardeck commented Apr 19, 2018

By default all my my git commits are signed by gpg. I have added the following snippet to my .gitconfig:

[commit]
  gpgsign = true

The issue is when using git-duet all commits are signed, but then Github shows the one which are committed by me, but signed-off by my colleague as unverified because he does not have my gpg key in his github settings and he should not have.

So I suppose the right approach would be to only sign commits which are "Signed-off-by" by the user mentioned in the .gitconfig.

Or github changes their behavior and also verifies the commits signed-off by somebody else with the gpg key of the committer.

@jszwedko
Copy link
Member

Thanks for the report!

I don't regularly use the gpgsigning feature of git, so I may be missing some bits, but:

I do see that commit.gpgsign = true does attempt to sign the commit using the credentials of the committer and not the author. I think, when using git duet you will want to ensure that the committer (the second initials in the pair) has their gpg credentials locally. In your case, when pairing on your machine, you will want to always have yourself as the second pair.

Alternatively, you can also consider using the recently added Co-authored-by trailer support which does not make use of the committer field and, in your case, would sign commits with whomever the primary pair is (the author) while secondary pairs would simply appear in the commit message trailers (which are supported by Github's UI).

I'm not currently seeing a better workaround for this, but let me know your thoughts!

@thardeck
Copy link
Author

thardeck commented May 8, 2018

Thanks for your reply. The problem with having a static succession is that the author tag never changes. We used git-duet specifically to prevent this.

Another workaround would be to disable the gpgsign option and use the -S option for commits where it makes sense but of course it is not as comfortable.

@jszwedko
Copy link
Member

jszwedko commented May 9, 2018

Aha, I see, yes.

A couple of ideas:

  • We could add logic to git-duet-commit to handle this case explicitly by looking at the author's email and determining if there is a matching gpg key. This adds more complexity to git-duet-commit and I'm don't immediately see how it could be integrated with the new Co-authored-by behavior which allows git commit to be used directly.
  • We could add an additional section to the .git-authors file that allows specification of additional flags to be run when the individual is the author. In this case, it would allow you to unset commit.gpgsign and add -S to your user in the .git-authors file. This is more flexible.

Open to other implementation approaches!

@WilHall
Copy link

WilHall commented Jul 13, 2019

@jszwedko

Alternatively, you can also consider using the recently added Co-authored-by trailer support which does not make use of the committer field and, in your case, would sign commits with whomever the primary pair is (the author) while secondary pairs would simply appear in the commit message trailers (which are supported by Github's UI).

I am having the same issue as @thardeck , however I wanted to use the Co-authored-by support so I turned that on, but git duet still sets the committer name and email:

$ echo $GIT_DUET_CO_AUTHORED_BY
true

$ git duet foo bar
GIT_AUTHOR_NAME='Foo'
GIT_AUTHOR_EMAIL='foo@foo.com'
GIT_COMMITTER_NAME='Bar'
GIT_COMMITTER_EMAIL='bar@bar.com'

With this setup, I also noticed another strange behavior:

$ git duet foo bar
GIT_AUTHOR_NAME='Foo'
GIT_AUTHOR_EMAIL='foo@foo.com'
GIT_COMMITTER_NAME='Bar'
GIT_COMMITTER_EMAIL='bar@bar.com'

$ git solo
GIT_AUTHOR_NAME='Foo'
GIT_AUTHOR_EMAIL='foo@foo.com'

$ tail .gitconfig
[duet "env"]
        git-author-initials = foo
        git-author-name = Foo
        git-author-email = foo@foo.com
        mtime = 1563030232
        git-committer-initials = bar
        git-committer-name = Bar
        git-committer-email = bar@bar.com

So it appears that the committer gets set, but not unset.

Is this behavior intended? I'm happy to test more with my setup.

@jszwedko
Copy link
Member

jszwedko commented Sep 8, 2019

Apologies for the delayed response!

What you are seeing is correct behavior, git-duet will still store both the given user and committer as configuration when using GIT_DUET_CO_AUTHORED_BY; it's just the usage of these values that differs:

  • With GIT_DUET_CO_AUTHORED_BY unset, it to sets both the author and commiter fields of the commit (e.g. here the author would be set to foo and the committer to bar).
  • With GIT_DUET_CO_AUTHORED_BY=1, instead of using the committer field of the commit, the given committer is added as a trailer to the commit (Co-authored-by). The author is still used as the author field of the commit.

You can run git show --pretty=full with commits made in each mode to see the difference.

Example:

$ cat ~/.git-authors 
pairs:
  jd: Jane Doe
  fb: Frances Bar
$ git-duet-commit -m test
[master bca4fd3] test
 Author: Jane Doe <jane-doe@hamster.local>
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 b
$ git show --pretty=full
commit bca4fd3a3e5d6ec225aba65612002d149619f046 (HEAD -> master)
Author: Jane Doe <jane-doe@hamster.local>
Commit: Frances Bar <frances-bar@hamster.local>

    test
    
    Signed-off-by: Frances Bar <frances-bar@hamster.local>

diff --git a/b b/b
new file mode 100644
index 0000000..e69de29
$ export GIT_DUET_CO_AUTHORED_BY=1
$ git duet jd fb
GIT_AUTHOR_NAME='Jane Doe'
GIT_AUTHOR_EMAIL='jane-doe@hamster.local'
GIT_COMMITTER_NAME='Frances Bar'
GIT_COMMITTER_EMAIL='frances-bar@hamster.local'
git-duet-install-hook: Installed hook to /tmp/tmp2/.git/hooks/prepare-commit-msg
$ git commit -m test # note the usage of `git commit` here rather than `git-duet-commit`
[master 85a94d6] test
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 e
$ git show --pretty=full
commit 85a94d62d107998da9973c1de23e860a7d6e9804 (HEAD -> master)
Author: Jane Doe <jane-doe@hamster.local>
Commit: Jane Doe <jane-doe@hamster.local>

    test
    
    Co-authored-by: Frances Bar <frances-bar@hamster.local>

diff --git a/e b/e
new file mode 100644
index 0000000..e69de29

@WilHall
Copy link

WilHall commented Sep 8, 2019

Hey @jszwedko no problem - thanks for the reply!

Here is a session illustrating the behavior i believe to be incorrect:

$ echo $GIT_DUET_CO_AUTHORED_BY
1

$ echo $GIT_DUET_GLOBAL
1

$ cat ~/.git-authors
authors:
    wh: Wil Hall
    fb: Foo Bar
email_addresses:
    wh: wil@wilhall.com
    fb: foo@bar.com

$ git duet wh fb
GIT_AUTHOR_NAME='Wil Hall'
GIT_AUTHOR_EMAIL='wil@wilhall.com'
GIT_COMMITTER_NAME='Foo Bar'
GIT_COMMITTER_EMAIL='foo@bar.com'

$ tail ~/.gitconfig
[duet "env"]
        git-author-initials = wh
        git-author-name = Wil Hall
        git-author-email = wil@wilhall.com
        git-committer-initials = fb
        git-committer-name = Foo Bar
        git-committer-email = foo@bar.com


$ git duet-commit -m "Duet test" --allow-empty
[develop de7be0d] Duet test
 Author: Wil Hall <wil@wilhall.com>


$ git log

. . .

commit de7be0da359f3c42701d280791643d63bfef1371 (HEAD -> develop)
Author: Wil Hall <wil@wilhall.com>
Date:   Sun Sep 8 19:27:54 2019 -0400

    Duet test

    Signed-off-by: Foo Bar <foo@bar.com>
    Co-authored-by: Foo Bar <foo@bar.com>

$ git solo
GIT_AUTHOR_NAME='Wil Hall'
GIT_AUTHOR_EMAIL='wil@wilhall.com'

$ tail ~/.gitconfig
[duet "env"]
        git-author-initials = wh
        git-author-name = Wil Hall
        git-author-email = wil@wilhall.com
        git-committer-initials = fb
        git-committer-name = Foo Bar
        git-committer-email = foo@bar.com


$ git duet-commit -m "Solo test" --allow-empty
[develop 857d834] Solo test
 Author: Wil Hall <wil@wilhall.com>

$ git log

. . .

commit 857d8347cecba19aa7a1ff4e6cc8ddf75c3e30c6 (HEAD -> develop)
Author: Wil Hall <wil@wilhall.com>
Date:   Sun Sep 8 19:28:44 2019 -0400

    Solo test

    Signed-off-by: Foo Bar <foo@bar.com>
    Co-authored-by: Foo Bar <foo@bar.com>


Using commit instead of duet-commit produces a slightly different result in the example above; it doesn't ever include the Signed-off-by trailer. However, both git duet and git solo result in commits that include Co-authored-by, so it's impossible to make solo commits.

@jszwedko
Copy link
Member

Aha! I see. This seems very similar to #81 .

git solo wh should get the behavior you want, but I agree that git solo's behavior of only printing out the author, even when a separate committer will be used, is confusing. I'm curious what you think of #81 (comment)

@WilHall
Copy link

WilHall commented Sep 21, 2019

@jszwedko Thanks for pointing that out! I totally missed that. It seems that my issue is the same as #81. I'll comment my thoughts on that issue 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants