Revision aba91192ae39cd1a2f79e7ed91e966df3cfe10b7 authored by Carlos Rica on 09 September 2007, 00:39:29 UTC, committed by Junio C Hamano on 10 September 2007, 04:30:54 UTC
Most of this patch code and message was written by Shawn O. Pearce.
I made some tests to know what the problem was, and then I changed
the code related with the SIGPIPE signal.

If the user has misconfigured `user.signingkey` in their .git/config
or just doesn't have any secret keys on their keyring and they ask
for a signed tag with `git tag -s` we better make sure the resulting
tag was actually signed by gpg.

Prior versions of builtin git-tag allowed this failure to slip
by without error as they were not checking the return value of
the finish_command() so they did not notice when gpg exited with
an error exit status.  They also did not fail if gpg produced an
empty output or if read_in_full received an error from the read
system call while trying to read the pipe back from gpg.

Finally, we did not actually honor any return value from the do_sign
function as it returns ssize_t but was being stored into an unsigned
long.  This caused the compiler to optimize out the die condition,
allowing git-tag to continue along and create the tag object.

However, when gpg gets a wrong username, it exits before any read was done
and then the writing process receives SIGPIPE and program is terminated.
By ignoring this signal, anyway, the function write_or_die gets EPIPE from
write_in_full and exits returning 0 to the system without a message.
Here we better call to write_in_full directly so we can fail
printing a message and return safely to the caller.

With these issues fixed `git-tag -s` will now fail to create the
tag and will report a non-zero exit status to its caller, thereby
allowing automated helper scripts to detect (and recover from)
failure if gpg is not working properly.

Proposed-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Carlos Rica <jasampler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 7b02b85
Raw File
builtin-commit-tree.c
/*
 * GIT - The information manager from hell
 *
 * Copyright (C) Linus Torvalds, 2005
 */
#include "cache.h"
#include "commit.h"
#include "tree.h"
#include "builtin.h"
#include "utf8.h"

#define BLOCKING (1ul << 14)

/*
 * FIXME! Share the code with "write-tree.c"
 */
static void init_buffer(char **bufp, unsigned int *sizep)
{
	*bufp = xmalloc(BLOCKING);
	*sizep = 0;
}

static void add_buffer(char **bufp, unsigned int *sizep, const char *fmt, ...)
{
	char one_line[2048];
	va_list args;
	int len;
	unsigned long alloc, size, newsize;
	char *buf;

	va_start(args, fmt);
	len = vsnprintf(one_line, sizeof(one_line), fmt, args);
	va_end(args);
	size = *sizep;
	newsize = size + len + 1;
	alloc = (size + 32767) & ~32767;
	buf = *bufp;
	if (newsize > alloc) {
		alloc = (newsize + 32767) & ~32767;
		buf = xrealloc(buf, alloc);
		*bufp = buf;
	}
	*sizep = newsize - 1;
	memcpy(buf + size, one_line, len);
}

static void check_valid(unsigned char *sha1, enum object_type expect)
{
	enum object_type type = sha1_object_info(sha1, NULL);
	if (type < 0)
		die("%s is not a valid object", sha1_to_hex(sha1));
	if (type != expect)
		die("%s is not a valid '%s' object", sha1_to_hex(sha1),
		    typename(expect));
}

/*
 * Having more than two parents is not strange at all, and this is
 * how multi-way merges are represented.
 */
#define MAXPARENT (16)
static unsigned char parent_sha1[MAXPARENT][20];

static const char commit_tree_usage[] = "git-commit-tree <sha1> [-p <sha1>]* < changelog";

static int new_parent(int idx)
{
	int i;
	unsigned char *sha1 = parent_sha1[idx];
	for (i = 0; i < idx; i++) {
		if (!hashcmp(parent_sha1[i], sha1)) {
			error("duplicate parent %s ignored", sha1_to_hex(sha1));
			return 0;
		}
	}
	return 1;
}

static const char commit_utf8_warn[] =
"Warning: commit message does not conform to UTF-8.\n"
"You may want to amend it after fixing the message, or set the config\n"
"variable i18n.commitencoding to the encoding your project uses.\n";

int cmd_commit_tree(int argc, const char **argv, const char *prefix)
{
	int i;
	int parents = 0;
	unsigned char tree_sha1[20];
	unsigned char commit_sha1[20];
	char comment[1000];
	char *buffer;
	unsigned int size;
	int encoding_is_utf8;

	git_config(git_default_config);

	if (argc < 2)
		usage(commit_tree_usage);
	if (get_sha1(argv[1], tree_sha1))
		die("Not a valid object name %s", argv[1]);

	check_valid(tree_sha1, OBJ_TREE);
	for (i = 2; i < argc; i += 2) {
		const char *a, *b;
		a = argv[i]; b = argv[i+1];
		if (!b || strcmp(a, "-p"))
			usage(commit_tree_usage);

		if (parents >= MAXPARENT)
			die("Too many parents (%d max)", MAXPARENT);
		if (get_sha1(b, parent_sha1[parents]))
			die("Not a valid object name %s", b);
		check_valid(parent_sha1[parents], OBJ_COMMIT);
		if (new_parent(parents))
			parents++;
	}

	/* Not having i18n.commitencoding is the same as having utf-8 */
	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);

	init_buffer(&buffer, &size);
	add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));

	/*
	 * NOTE! This ordering means that the same exact tree merged with a
	 * different order of parents will be a _different_ changeset even
	 * if everything else stays the same.
	 */
	for (i = 0; i < parents; i++)
		add_buffer(&buffer, &size, "parent %s\n", sha1_to_hex(parent_sha1[i]));

	/* Person/date information */
	add_buffer(&buffer, &size, "author %s\n", git_author_info(1));
	add_buffer(&buffer, &size, "committer %s\n", git_committer_info(1));
	if (!encoding_is_utf8)
		add_buffer(&buffer, &size,
				"encoding %s\n", git_commit_encoding);
	add_buffer(&buffer, &size, "\n");

	/* And add the comment */
	while (fgets(comment, sizeof(comment), stdin) != NULL)
		add_buffer(&buffer, &size, "%s", comment);

	/* And check the encoding */
	buffer[size] = '\0';
	if (encoding_is_utf8 && !is_utf8(buffer))
		fprintf(stderr, commit_utf8_warn);

	if (!write_sha1_file(buffer, size, commit_type, commit_sha1)) {
		printf("%s\n", sha1_to_hex(commit_sha1));
		return 0;
	}
	else
		return 1;
}
back to top