--signoff commits

[arunsathiya]

I am not very certain but wanted to surface the idea of signing off commits with patch2pr-created commits.

I don't see that feature at the moment, perhaps is a GitHub limitation?

• https://docs.github.com/en/graphql/reference/mutations#createcommitonbranch
• https://docs.github.com/en/graphql/reference/input-objects#createcommitonbranchinput

[bluekeyes]

The Signed-off-by line added by Git's --signoff flag, like other trailers, is nothing more than a part of the commit message that is formatted in a specific way. That means that today, the easiest way to get this behavior with patch2pr is to use the --message flag to pass a commit message that includes the desired Signed-off-by line.

That said, I'm not opposed to adding a --signoff flag. But unlike with Git (which uses your configured Git identity), it's not immediately clear how to get the identity to put in the Signed-off-by line. It could:

• Copy the author information from the patch
• Use the GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL environment variables
• Take the identity as an argument to the flag
• Look up the user and email associated with the GitHub token and use that

There are probably other options too, but I think one of these would be the easiest to implement.

[arunsathiya]

The Signed-off-by line added by Git's --signoff flag, like other trailers, is nothing more than a part of the commit message that is formatted in a specific way.

Thank you for this tip! I dug in a bit more into how commits and patches are structured, and it does seem that Signed-off-by: Author <Email> syntax just needs to be in the commit body. With that understanding, I have this setup:

sha, err := graphqlApplier.Commit(
	context.Background(),
	"refs/heads/"+*fork.DefaultBranch,
	&gitdiff.PatchHeader{
		Author: &gitdiff.PatchIdentity{
			Name:  "Arun",
			Email: "[email protected]",
		},
		AuthorDate: time.Now(),
		Committer: &gitdiff.PatchIdentity{
			Name:  "Arun",
			Email: "[email protected]",
		},
		CommitterDate: time.Now(),
		Title:         prTitle,
		Body:          prBody + "\n\nSigned-off-by: Arun <[email protected]>",
	},
)

And it appears okay in the commit message.

Unrelated issue

But for some reason, IBM's checks don't seem to identify those commits as signed off. I had to run git rebase HEAD~n --signoff and force-push in these two examples:

• IBM/gevent-for-z@ff5ec74
• IBM/grpc-for-Z@cbe5f2f

I'll keep digging into that issue separately because it's not a patch2pr issue.

[bluekeyes]

I'd try adding a trailing newline on your body, e.g.

Body: prBody + "\n\nSigned-off-by: Arun <[email protected]>\n",

When you create a commit using normal git commands, it will make sure the message ends with a newline, but this is not enforced by patch2pr or the GitHub APIs. Tools that are looking for trailers in a message may expect this final newline.

If that doesn't help, another debugging tip is to compare the raw commits created by patch2pr with those created by git. You can do this using git cat-file commit <sha> or git log -1 --format=%B <sha> if you only want to see the commit message. Piping the output to a hex dump tool like xxd can make it easier to identify differences in whitespace characters.

[arunsathiya]

Thanks for all the tips! I haven't found a conclusive answer to what's happening just yet. But my findings are below:

Tools that are looking for trailers in a message may expect this final newline

No change with adding this newline.

If that doesn't help, another debugging tip is to compare the raw commits created by patch2pr with those created by git. You can do this using git cat-file commit or git log -1 --format=%B

With the latter command, I found that the newline actually appears as a %. I am still researching more on why that's happening.

Piping the output to a hex dump tool like xxd can make it easier to identify differences in whitespace characters.

On piping to xxd seems to show the same thing both in before (just appending the S-O-B line to commit body) and after (re-signing with --signoff) conditions.

So, I am not sure what's happening just yet, but I am going to keep looking tomorrow.