threads / patch / 9249

patchMake verify-tag a builtin.

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

## tl;dr

5 messages between Jul 27, 2007 and Jul 28, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Carlos Rica· Jul 27, 2007, 04:07 UTC · lore
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%)
Show changes to 6 files +117 −3

Documentation/git-verify-tag.txt, Makefile, builtin-verify-tag.c, builtin.h, git-verify-tag.sh, git.c

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
Junio C Hamano· Jul 27, 2007, 07:05 UTC · re: Carlos Rica · lore

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

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'.

Carlos Rica· Jul 27, 2007, 19:51 UTC · re: Junio C Hamano · lore

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

2007/7/27, Junio C Hamano <gitster@pobox.com>:
Show 18 quoted lines
> 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.

Junio C Hamano· Jul 27, 2007, 21:08 UTC · re: Carlos Rica · lore

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

"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.

Carlos Rica· Jul 28, 2007, 18:02 UTC · re: Junio C Hamano · lore

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

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>:
Show 22 quoted lines
> 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.

← back to recent threads