Carlos Rica <jasampler@gmail.com> wrote:
quoted text
>    I'm not sure how to send this
>    right, because the patch was almost entirely from Shawn and I
>    only changed a few things.

It depends on how you want the attribution to look.  In this case
where the bulk of the patch was actually written by me you could
insert a line as the very first line of the message body, so the
overall email message (including headers) looked like this:

  From: Carlos Rica <jasampler@gmail.com>
  To: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
  Cc: "Shawn O. Pearce" <spearce@spearce.org>, Johannes Schindelin <Johannes.Schindelin@gmx.de>
  Subject: [PATCH] git-tag -s must fail if gpg cannot sign the tag.

  From: Shawn O. Pearce <spearce@spearce.org>
  
  ...rest of the commit message...

  Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
  Cleaned-up-by: Carlos Rica <jasampler@gmail.com>

In such a case git-am will replace your own From header with the
From header that starts the message body, thus allowing me to be
made the author when Junio applies the patch.

Of you could write it out differently and make yourself the author:

  From: Carlos Rica <jasampler@gmail.com>
  To: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
  Cc: "Shawn O. Pearce" <spearce@spearce.org>, Johannes Schindelin <Johannes.Schindelin@gmx.de>
  Subject: [PATCH] git-tag -s must fail if gpg cannot sign the tag.

  ...rest of the commit message...

  Proposed-by: Shawn O. Pearce <spearce@spearce.org>
  Signed-off-by: Carlos Rica <jasampler@gmail.com>

Either approach on a patch like this fine with me.  I think what is
important is that its clear from the commit author and message body
who was involved with considering/developing/testing the change so
everyone knows later on who to go back to if there are questions
concerning that code.

quoted text
> diff --git a/builtin-tag.c b/builtin-tag.c
> index 348919c..c9be69a 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -200,6 +200,10 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
>  			bracket[1] = '__PLACEHOLDER__1_';
>  	}
> 
> +	/* When the username signingkey is bad, program could be terminated
> +	 * because gpg exits without reading and then write gets SIGNPIPE. */
> +	signal(SIGPIPE, SIG_IGN);
> +

Don't you mean SIGPIPE in the comment above?  (It says SIGNPIPE).

quoted text
> @@ -212,12 +216,13 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
>  	if (start_command(&gpg))
>  		return error("could not run gpg.");
> 
> -	write_or_die(gpg.in, buffer, size);
> +	write_in_full(gpg.in, buffer, size);

write_in_full returns the size written.  I would prefer that this
be handled correctly:

	if (write_in_full(gpg.in, buffer, size) != size) {
		close(gpg.in);
		finish_command(&gpg);
		return error("gpg did not accept the tag data");
	}

We might fail to send all bytes to gpg and in such a condition
gpg would sign part of the tag, and we might then actually get
back a signature for just that leading part.  Such a tag would
never verify...

-- 
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html