[PATCH] Make verify-tag a builtin.

[PATCH] Make verify-tag a builtin.

[Carlos Rica]

This replaces "git-verify-tag.sh" with "builtin-verify-tag.c".

Signal SIGPIPE is ignored because the program sometimes was
stopped because that signal when writing the input for gpg.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---

   Comments are welcomed.

 Makefile                                           |    3 +-
 builtin-verify-tag.c                               |  113 ++++++++++++++++++++
 builtin.h                                          |    1 +
 .../examples/git-verify-tag.sh                     |    0
 git.c                                              |    1 +
 5 files changed, 117 insertions(+), 1 deletions(-)
 create mode 100644 builtin-verify-tag.c
 rename git-verify-tag.sh => contrib/examples/git-verify-tag.sh (100%)

diff --git a/Makefile b/Makefile
index 73b487f..c6ed79f 100644
--- a/Makefile
+++ b/Makefile
@@ -206,7 +206,7 @@ SCRIPT_SH = \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
 	git-sh-setup.sh \
-	git-tag.sh git-verify-tag.sh \
+	git-tag.sh \
 	git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh \
@@ -367,6 +367,7 @@ BUILTIN_OBJS = \
 	builtin-update-ref.o \
 	builtin-upload-archive.o \
 	builtin-verify-pack.o \
+	builtin-verify-tag.o \
 	builtin-write-tree.o \
 	builtin-show-ref.o \
 	builtin-pack-refs.o
diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
new file mode 100644
index 0000000..586f28a
--- /dev/null
+++ b/builtin-verify-tag.c
@@ -0,0 +1,113 @@
+/*
+ * Builtin "git verify-tag"
+ *
+ * Copyright (c) 2007 Carlos Rica <jasampler@gmail.com>
+ *
+ * Based on git-verify-tag.sh
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "tag.h"
+#include "run-command.h"
+#include <signal.h>
+
+static const char builtin_verify_tag_usage[] =
+		"git-verify-tag [-v|--verbose] <tag>...";
+
+#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
+
+static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
+{
+	struct child_process gpg;
+	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+	char *path, *eol;
+	size_t len;
+	int fd, ret;
+
+	path = xstrdup(git_path("VTAG_TMP"));
+	fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
+	if (fd < 0)
+		return error("could not create temporary file '%s': %s",
+						path, strerror(errno));
+	if (write_in_full(fd, buf, size) < 0)
+		return error("failed writing temporary file '%s': %s",
+						path, strerror(errno));
+	close(fd);
+
+	/* find the length without signature */
+	len = 0;
+	while (len < size && prefixcmp(buf + len, PGP_SIGNATURE "\n")) {
+		eol = memchr(buf + len, '\n', size - len);
+		len += eol ? eol - (buf + len) + 1 : size - len;
+	}
+	if (verbose)
+		write_in_full(1, buf, len);
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args_gpg;
+	gpg.in = -1;
+	gpg.out = 1;
+	args_gpg[2] = path;
+	if (start_command(&gpg))
+		return error("could not run gpg.");
+
+	write_in_full(gpg.in, buf, len);
+	close(gpg.in);
+	gpg.close_in = 0;
+	ret = finish_command(&gpg);
+
+	remove(path);
+	free(path);
+
+	return ret;
+}
+
+static int verify_tag(const char *name, int verbose)
+{
+	enum object_type type;
+	unsigned char sha1[20];
+	char *buf;
+	unsigned long size;
+	int ret;
+
+	if (get_sha1(name, sha1))
+		return error("tag '%s' not found.", name);
+
+	type = sha1_object_info(sha1, NULL);
+	if (type <= OBJ_NONE)
+		return error("%s: bad object type.", name);
+	if (type != OBJ_TAG)
+		return error("%s: cannot verify a non-tag object of type %s.",
+				name, typename(type));
+
+	buf = read_sha1_file(sha1, &type, &size);
+	if (!buf)
+		return error("%s: unable to read file.", name);
+
+	ret = run_gpg_verify(buf, size, verbose);
+
+	free(buf);
+	return ret;
+}
+
+int cmd_verify_tag(int argc, const char **argv, const char *prefix)
+{
+	int i, verbose = 0, had_error = 0;
+
+	git_config(git_default_config);
+
+	if (argc == 1)
+		usage(builtin_verify_tag_usage);
+
+	i = 1;
+	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
+		verbose = 1;
+		i++;
+	}
+
+	signal(SIGPIPE, SIG_IGN);
+	while (i < argc)
+		if (verify_tag(argv[i++], verbose))
+			had_error = 1;
+	return had_error;
+}
diff --git a/builtin.h b/builtin.h
index 4cc228d..cb860a0 100644
--- a/builtin.h
+++ b/builtin.h
@@ -76,6 +76,7 @@ extern int cmd_update_index(int argc, const char **argv, const char *prefix);
 extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_tar(int argc, const char **argv, const char *prefix);
+extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);
 extern int cmd_version(int argc, const char **argv, const char *prefix);
 extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);
 extern int cmd_write_tree(int argc, const char **argv, const char *prefix);
diff --git a/git-verify-tag.sh b/contrib/examples/git-verify-tag.sh
similarity index 100%
rename from git-verify-tag.sh
rename to contrib/examples/git-verify-tag.sh
diff --git a/git.c b/git.c
index a647f9c..1dfe120 100644
--- a/git.c
+++ b/git.c
@@ -368,6 +368,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "update-index", cmd_update_index, RUN_SETUP },
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
 		{ "upload-archive", cmd_upload_archive },
+		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
 		{ "version", cmd_version },
 		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
 		{ "write-tree", cmd_write_tree, RUN_SETUP },
-- 
1.5.0

Re: [PATCH] Make verify-tag a builtin.

[Johannes Schindelin]

Hi,

On Mon, 23 Jul 2007, Carlos Rica wrote:

> +static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
> +{
> +	struct child_process gpg;
> +	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
> +	char *path, *eol;
> +	size_t len;
> +	int fd, ret;
> +
> +	path = xstrdup(git_path("VTAG_TMP"));
> +	fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
> +	if (fd < 0)
> +		return error("could not create temporary file '%s': %s",
> +						path, strerror(errno));
> +	if (write_in_full(fd, buf, size) < 0)
> +		return error("failed writing temporary file '%s': %s",
> +						path, strerror(errno));
> +	close(fd);

I just tested something like "gpg --verify - - < <filename>" and it 
worked...

> +	/* find the length without signature */
> +	len = 0;
> +	while (len < size && prefixcmp(buf + len, PGP_SIGNATURE "\n")) {
> +		eol = memchr(buf + len, '\n', size - len);
> +		len += eol ? eol - (buf + len) + 1 : size - len;
> +	}

How about this instead:

	const char *sig = strstr(buf, "\n" PGP_SIGNATURE "\n"));
	if (!sig)
		sig = buf + len;

Hmm?

But of course only if gpg does not grok "git cat-file tag <signed-tag> |
gpg --verify - -".

> +	remove(path);

I think we use unlink() consistently, since we are sure it is no 
directory.

> +	if (type <= OBJ_NONE)
> +		return error("%s: bad object type.", name);
> +	if (type != OBJ_TAG)
> +		return error("%s: cannot verify a non-tag object of type %s.",
> +				name, typename(type));

These two can be unified into one, right?

> +int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> +{
> +	int i, verbose = 0, had_error = 0;
> +
> +	git_config(git_default_config);
> +
> +	if (argc == 1)
> +		usage(builtin_verify_tag_usage);
> +
> +	i = 1;

You can initialise i above.

> +	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
> +		verbose = 1;
> +		i++;
> +	}
> +
> +	signal(SIGPIPE, SIG_IGN);
> +	while (i < argc)
> +		if (verify_tag(argv[i++], verbose))

Heh.  I thought that you only made this a separate function to help 
building it into builtin-tag...

That was quick!

Ciao,
Dscho

Re: [PATCH] Make verify-tag a builtin.

[Carlos Rica]

2007/7/23, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> On Mon, 23 Jul 2007, Carlos Rica wrote:
> > +static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
> > +{
> > +     struct child_process gpg;
> > +     const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
> > +     char *path, *eol;
> > +     size_t len;
> > +     int fd, ret;
> > +
> > +     path = xstrdup(git_path("VTAG_TMP"));
> > +     fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
> > +     if (fd < 0)
> > +             return error("could not create temporary file '%s': %s",
> > +                                             path, strerror(errno));
> > +     if (write_in_full(fd, buf, size) < 0)
> > +             return error("failed writing temporary file '%s': %s",
> > +                                             path, strerror(errno));
> > +     close(fd);
>
> I just tested something like "gpg --verify - - < <filename>" and it
> worked...

I couldn't verify tags using "gpg --verify  -  -  <file" as you
pointed in your response.

In my tests, it only allowed me to run:
"gpg --verify file.asc" or
"gpg --verify <file.asc" or
 "gpg --verify - <file.asc"
when file.asc was generated using the command "gpg --clearsign file",
in which it inserts a header:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
<empty-line>

I even tested to insert the header myself after creating a signed file
as git does:
   cp file otherfile
   gpg -bsa otherfile
   ...
   cat otherfile.asc >>otherfile
   ( echo -e "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n" ; cat
otherfile ) \
     | gpg --verify

Note that sending them to "diff file.asc -" also shows different signatures,
but anyway it also says "gpg: BAD signature from ...", the same message as in:
   gpg --verify - - <otherfile

and different from the "gpg: Good signature from ..." when I run:
  gpg --verify file.asc

Re: [PATCH] Make verify-tag a builtin.

[Johannes Schindelin]

Hi,

On Mon, 23 Jul 2007, Carlos Rica wrote:

> 2007/7/23, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> > On Mon, 23 Jul 2007, Carlos Rica wrote:
> > > +static int run_gpg_verify(const char *buf, unsigned long size, int
> > verbose)
> > > +{
> > > +     struct child_process gpg;
> > > +     const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
> > > +     char *path, *eol;
> > > +     size_t len;
> > > +     int fd, ret;
> > > +
> > > +     path = xstrdup(git_path("VTAG_TMP"));
> > > +     fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
> > > +     if (fd < 0)
> > > +             return error("could not create temporary file '%s': %s",
> > > +                                             path, strerror(errno));
> > > +     if (write_in_full(fd, buf, size) < 0)
> > > +             return error("failed writing temporary file '%s': %s",
> > > +                                             path, strerror(errno));
> > > +     close(fd);
> > 
> > I just tested something like "gpg --verify - - < <filename>" and it
> > worked...
> 
> I couldn't verify tags using "gpg --verify  -  -  <file" as you
> pointed in your response.

As discussed on IRC, my test did not use a detached signature, while tags 
_do_ use a detached signature.

Oh, well.  I retract my suggestion.  But I slightly disagree that having 
to write a temporary file is good for security, as the gpg man page 
suggests:

--verify [[sigfile]  [signed-files]]
         [...] To read the signed stuff from stdin, use - as the second 
	filename.  For security reasons a detached signature cannot read 
	the signed material from stdin without denoting it in the above 
	way.

So we have to live, or die, with the temporary file.

Just one request: please use lock_file, to prevent multiple instances from 
getting confused...  Even if that cannot be 100% secure: if you write to 
the file, and only then start gpg, there is still the odd chance for an 
attacker.  Even if you created the file with 0600 (very wise, BTW).

Ciao,
Dscho

Re: [PATCH] Make verify-tag a builtin.

[Carlos Rica]

2007/7/23, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Just one request: please use lock_file, to prevent multiple instances from
> getting confused...  Even if that cannot be 100% secure: if you write to
> the file, and only then start gpg, there is still the odd chance for an
> attacker.  Even if you created the file with 0600 (very wise, BTW).

Why not using also a random name for the temporary file?
Something like "VTAG_TMP30946".

Re: [PATCH] Make verify-tag a builtin.

[Johannes Schindelin]

Hi,

On Tue, 24 Jul 2007, Carlos Rica wrote:

> 2007/7/23, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> > Just one request: please use lock_file, to prevent multiple instances from
> > getting confused...  Even if that cannot be 100% secure: if you write to
> > the file, and only then start gpg, there is still the odd chance for an
> > attacker.  Even if you created the file with 0600 (very wise, BTW).
> 
> Why not using also a random name for the temporary file?
> Something like "VTAG_TMP30946".

Yes, makes sense.  One call to mkstemp() away.

Ciao,
Dscho

[PATCH] Make verify-tag a builtin.

[Carlos Rica]

This replaces "git-verify-tag.sh" with "builtin-verify-tag.c".

Testing relies on the "git tag -v" tests calling this command.

A temporary file is needed when calling to gpg, because git is
already creating detached signatures (gpg option -b) to sign tags
(instead of leaving gpg to add the signature to the file by itself),
and those signatures need to be supplied in a separate file to be
verified by gpg.

The program uses git_mkstemp to create that temporary file needed by
gpg, instead of the previously used "$GIT_DIR/.tmp-vtag", in order to
allow the command to be used in read-only repositories, and also
prevent other instances of git to read or remove the same file.

Signal SIGPIPE is ignored because the program sometimes was
terminated because that signal when writing the input for gpg.

The command now can receive many tag names to be verified.
Documentation is also updated here to reflect this new behaviour.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---

   This resend is addressing the questions commented in:
   http://thread.gmane.org/gmane.comp.version-control.git/53376

   Function verify_tag is designed to be easily reused in the
   upcoming "builtin-tag.c", although it is declared static now.

 Documentation/git-verify-tag.txt                   |    4 +-
 Makefile                                           |    3 +-
 builtin-verify-tag.c                               |  111 ++++++++++++++++++++
 builtin.h                                          |    1 +
 .../examples/git-verify-tag.sh                     |    0
 git.c                                              |    1 +
 6 files changed, 117 insertions(+), 3 deletions(-)
 create mode 100644 builtin-verify-tag.c
 rename git-verify-tag.sh => contrib/examples/git-verify-tag.sh (100%)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index 48d17fd..ac7fb19 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -3,11 +3,11 @@ git-verify-tag(1)

 NAME
 ----
-git-verify-tag - Check the GPG signature of tag
+git-verify-tag - Check the GPG signature of tags

 SYNOPSIS
 --------
-'git-verify-tag' <tag>
+'git-verify-tag' <tag>...

 DESCRIPTION
 -----------
diff --git a/Makefile b/Makefile
index 73b487f..c6ed79f 100644
--- a/Makefile
+++ b/Makefile
@@ -206,7 +206,7 @@ SCRIPT_SH = \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
 	git-sh-setup.sh \
-	git-tag.sh git-verify-tag.sh \
+	git-tag.sh \
 	git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh \
@@ -367,6 +367,7 @@ BUILTIN_OBJS = \
 	builtin-update-ref.o \
 	builtin-upload-archive.o \
 	builtin-verify-pack.o \
+	builtin-verify-tag.o \
 	builtin-write-tree.o \
 	builtin-show-ref.o \
 	builtin-pack-refs.o
diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
new file mode 100644
index 0000000..11b971e
--- /dev/null
+++ b/builtin-verify-tag.c
@@ -0,0 +1,111 @@
+/*
+ * Builtin "git verify-tag"
+ *
+ * Copyright (c) 2007 Carlos Rica <jasampler@gmail.com>
+ *
+ * Based on git-verify-tag.sh
+ */
+#include "cache.h"
+#include "builtin.h"
+#include "tag.h"
+#include "run-command.h"
+#include <signal.h>
+
+static const char builtin_verify_tag_usage[] =
+		"git-verify-tag [-v|--verbose] <tag>...";
+
+#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
+
+static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
+{
+	struct child_process gpg;
+	const char *args_gpg[] = {"gpg", "--verify", "FILE", "-", NULL};
+	char path[PATH_MAX], *eol;
+	size_t len;
+	int fd, ret;
+
+	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+	if (fd < 0)
+		return error("could not create temporary file '%s': %s",
+						path, strerror(errno));
+	if (write_in_full(fd, buf, size) < 0)
+		return error("failed writing temporary file '%s': %s",
+						path, strerror(errno));
+	close(fd);
+
+	/* find the length without signature */
+	len = 0;
+	while (len < size && prefixcmp(buf + len, PGP_SIGNATURE "\n")) {
+		eol = memchr(buf + len, '\n', size - len);
+		len += eol ? eol - (buf + len) + 1 : size - len;
+	}
+	if (verbose)
+		write_in_full(1, buf, len);
+
+	memset(&gpg, 0, sizeof(gpg));
+	gpg.argv = args_gpg;
+	gpg.in = -1;
+	gpg.out = 1;
+	args_gpg[2] = path;
+	if (start_command(&gpg))
+		return error("could not run gpg.");
+
+	write_in_full(gpg.in, buf, len);
+	close(gpg.in);
+	gpg.close_in = 0;
+	ret = finish_command(&gpg);
+
+	unlink(path);
+	free(path);
+
+	return ret;
+}
+
+static int verify_tag(const char *name, int verbose)
+{
+	enum object_type type;
+	unsigned char sha1[20];
+	char *buf;
+	unsigned long size;
+	int ret;
+
+	if (get_sha1(name, sha1))
+		return error("tag '%s' not found.", name);
+
+	type = sha1_object_info(sha1, NULL);
+	if (type != OBJ_TAG)
+		return error("%s: cannot verify a non-tag object of type %s.",
+				name, typename(type));
+
+	buf = read_sha1_file(sha1, &type, &size);
+	if (!buf)
+		return error("%s: unable to read file.", name);
+
+	ret = run_gpg_verify(buf, size, verbose);
+
+	free(buf);
+	return ret;
+}
+
+int cmd_verify_tag(int argc, const char **argv, const char *prefix)
+{
+	int i = 1, verbose = 0, had_error = 0;
+
+	git_config(git_default_config);
+
+	if (argc == 1)
+		usage(builtin_verify_tag_usage);
+
+	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
+		verbose = 1;
+		i++;
+	}
+
+	/* sometimes the program was terminated because this signal
+	 * was received in the process of writing the gpg input: */
+	signal(SIGPIPE, SIG_IGN);
+	while (i < argc)
+		if (verify_tag(argv[i++], verbose))
+			had_error = 1;
+	return had_error;
+}
diff --git a/builtin.h b/builtin.h
index 4cc228d..cb860a0 100644
--- a/builtin.h
+++ b/builtin.h
@@ -76,6 +76,7 @@ extern int cmd_update_index(int argc, const char **argv, const char *prefix);
 extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_tar(int argc, const char **argv, const char *prefix);
+extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);
 extern int cmd_version(int argc, const char **argv, const char *prefix);
 extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);
 extern int cmd_write_tree(int argc, const char **argv, const char *prefix);
diff --git a/git-verify-tag.sh b/contrib/examples/git-verify-tag.sh
similarity index 100%
rename from git-verify-tag.sh
rename to contrib/examples/git-verify-tag.sh
diff --git a/git.c b/git.c
index a647f9c..1dfe120 100644
--- a/git.c
+++ b/git.c
@@ -368,6 +368,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "update-index", cmd_update_index, RUN_SETUP },
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
 		{ "upload-archive", cmd_upload_archive },
+		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
 		{ "version", cmd_version },
 		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
 		{ "write-tree", cmd_write_tree, RUN_SETUP },
-- 
1.5.0

Re: [PATCH] Make verify-tag a builtin.

[Junio C Hamano]

Carlos Rica <jasampler@gmail.com> writes:

> This replaces "git-verify-tag.sh" with "builtin-verify-tag.c".

This is better overall, but you need to drop free(path) as you
are using git_mkstemp() now on a on-stack buffer.

> Signal SIGPIPE is ignored because the program sometimes was
> terminated because that signal when writing the input for gpg.

This "sometimes" does not give confidence to readers, I am
afraid.

What is happening is perfectly normal, not "sometimes it gets
the signal mysteriously, so we need to paper it over by ignoring
it".

You invoke gpg, giving it a file that is supposed to contain a
detached signature, and start feeding the payload that ought to
verify Ok with the signature.  If the tag is not signed, after
gpg has read the detached signature, it already knows that the
signature will not verify and it can exit without reading the
payload from its standard input.  When you try to write the
payload to the pipe, you would get SIGPIPE.

Anyway, I'll replace the tip of cr/tag topic with this version,
and merge it to 'next'.

Re: [PATCH] Make verify-tag a builtin.

[Carlos Rica]

2007/7/27, Junio C Hamano <gitster@pobox.com>:
> Carlos Rica <jasampler@gmail.com> writes:
> > Signal SIGPIPE is ignored because the program sometimes was
> > terminated because that signal when writing the input for gpg.
>
> This "sometimes" does not give confidence to readers, I am
> afraid.
>
> What is happening is perfectly normal, not "sometimes it gets
> the signal mysteriously, so we need to paper it over by ignoring
> it".
>
> You invoke gpg, giving it a file that is supposed to contain a
> detached signature, and start feeding the payload that ought to
> verify Ok with the signature.  If the tag is not signed, after
> gpg has read the detached signature, it already knows that the
> signature will not verify and it can exit without reading the
> payload from its standard input.  When you try to write the
> payload to the pipe, you would get SIGPIPE.

By using "sometimes" I was requesting an answer to this strange
behaviour (for me) to you and the mailing list. Now I could reproduce it
removing the call to signal and running this:

#!/bin/sh
echo "creating vtag1..."
git tag -s -m "a signed tag" vtag1
echo "creating vtag2..."
git tag -m "an annotated non-signed tag" vtag2
echo "creating vtag3..."
git tag -m "another annotated non-signed tag is " vtag3
# 141 returned sometimes when gpg got
# a non-signed file as detached signature:
# (it refuses to read from the input when
# no signature is given, sometimes)
for i in 1 2 3 4 5 6 7 8 9 10
do
        ./git verify-tag vtag1 vtag2 vtag3
        echo "git verify-tag vtag1 vtag2 vtag3 exit code: $?"
        echo
done
git tag -d vtag1
git tag -d vtag2
git tag -d vtag3

In my system, some of the tests give 141 and others give 1 as exit code.
Dscho said that it could depend on the CPU current load of the computer,
since he got always 141 as you said, so perhaps it's me.

Re: [PATCH] Make verify-tag a builtin.

[Junio C Hamano]

"Carlos Rica" <jasampler@gmail.com> writes:

> In my system, some of the tests give 141 and others give 1 as exit code.
> Dscho said that it could depend on the CPU current load of the computer,
> since he got always 141 as you said, so perhaps it's me.

This is expected.  It depends on how processess are scheduled.

What is happening is:

 1. Your process prepares the whole thing in vtag-tmp, to hand
    to gpg;

 2. You make a pipe and start gpg with the above file telling it
    "here is a detached signature file, the payload will be fed
    through your stdin";

 3-a. You feed the payload to the pipe, expecting gpg to read it.

 3-b. gpg reads the detached signature file, finds no signature in
    the vtag-tmp file because the tag in question is not signed,
    and exits without reading a single byte from the pipe;

Now, 3-a and 3-b run in parallel.  If 3-a is scheduled before
3-b happens, because payload is very often much smaller than the
in-kernel pipe buffer, your write(2) succeeds before gpg gives up
and exits without reading from the pipe.  If 3-b is scheduled
before 3-a, then gpg exits and when 3-a gets around to write(2)
to the pipe, write notices that there is nobody on the other end
of the pipe, and you get SIGPIPE.

Re: [PATCH] Make verify-tag a builtin.

[Carlos Rica]

Your explanation about the I/O in parallel sounds much better to me,
now I understand, thank you Junio.

2007/7/27, Junio C Hamano <gitster@pobox.com>:
> What is happening is:
>
>  1. Your process prepares the whole thing in vtag-tmp, to hand
>     to gpg;
>
>  2. You make a pipe and start gpg with the above file telling it
>     "here is a detached signature file, the payload will be fed
>     through your stdin";
>
>  3-a. You feed the payload to the pipe, expecting gpg to read it.
>
>  3-b. gpg reads the detached signature file, finds no signature in
>     the vtag-tmp file because the tag in question is not signed,
>     and exits without reading a single byte from the pipe;
>
> Now, 3-a and 3-b run in parallel.  If 3-a is scheduled before
> 3-b happens, because payload is very often much smaller than the
> in-kernel pipe buffer, your write(2) succeeds before gpg gives up
> and exits without reading from the pipe.  If 3-b is scheduled
> before 3-a, then gpg exits and when 3-a gets around to write(2)
> to the pipe, write notices that there is nobody on the other end
> of the pipe, and you get SIGPIPE.