Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit
From: Jeff King @ 2011-10-22 19:17 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4EA2FC63.9060602@ramsay1.demon.co.uk>

On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote:

> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  credential-cache--daemon.c |   23 ++++++++++++-----------
>  1 files changed, 12 insertions(+), 11 deletions(-)

Looks sane, and I'll probably squash it in. Alternatively, we could also
set a signal/exit handler to clean up our socket when we die. That would
also cover the error exit cases.

In either case, I think we need to handle stale sockets better. They
will happen eventually due to power loss or kill -9, anyway.

-Peff

^ permalink raw reply

* [PATCH 5/5] revert: simplify communicating command-line arguments
From: Ramkumar Ramachandra @ 2011-10-22 19:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319310826-508-1-git-send-email-artagnon@gmail.com>

From: Jonathan Nieder <jrnieder@gmail.com>

Currently, command-line arguments are communicated using (argc, argv)
until a prepare_revs() turns it into a terse structure.  However,
since we plan to expose the cherry-picking machinery through a public
API in the future, we want callers to be able to call in with a
filled-in structure.  For the revert builtin, this means that the
chief argument parser, parse_args(), should parse into such a
structure.  Make this change.

[rr: minor improvements, commit message]

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c |   53 +++++++++++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 8c3e4fc..df9459b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,14 @@ struct replay_opts {
 	int allow_rerere_auto;
 
 	int mainline;
-	int commit_argc;
-	const char **commit_argv;
 
 	/* Merge strategy */
 	const char *strategy;
 	const char **xopts;
 	size_t xopts_nr, xopts_alloc;
+
+	/* Only used by REPLAY_NONE */
+	struct rev_info *revs;
 };
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -161,9 +162,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			die(_("program error"));
 	}
 
-	opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
-					PARSE_OPT_KEEP_ARGV0 |
-					PARSE_OPT_KEEP_UNKNOWN);
+	argc = parse_options(argc, argv, NULL, options, usage_str,
+			PARSE_OPT_KEEP_ARGV0 |
+			PARSE_OPT_KEEP_UNKNOWN);
 
 	/* Check for incompatible subcommands */
 	verify_opt_mutually_compatible(me,
@@ -198,9 +199,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				NULL);
 	}
 
-	else if (opts->commit_argc < 2)
-		usage_with_options(usage_str, options);
-
 	if (opts->allow_ff)
 		verify_opt_compatible(me, "--ff",
 				"--signoff", opts->signoff,
@@ -208,7 +206,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				"-x", opts->record_origin,
 				"--edit", opts->edit,
 				NULL);
-	opts->commit_argv = argv;
+
+	if (opts->subcommand == REPLAY_NONE) {
+		opts->revs = xmalloc(sizeof(*opts->revs));
+		init_revisions(opts->revs, NULL);
+		opts->revs->no_walk = 1;
+		if (argc < 2)
+			usage_with_options(usage_str, options);
+		argc = setup_revisions(argc, argv, opts->revs, NULL);
+	} else
+		opts->revs = NULL;
+
+	/* Forbid stray command-line arguments */
+	if (argc > 1)
+		usage_with_options(usage_str, options);
 }
 
 struct commit_message {
@@ -614,23 +625,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
 	return res;
 }
 
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
 {
-	int argc;
-
-	init_revisions(revs, NULL);
-	revs->no_walk = 1;
 	if (opts->action != REPLAY_REVERT)
-		revs->reverse = 1;
+		opts->revs->reverse ^= 1;
 
-	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
-	if (argc > 1)
-		usage(*revert_or_cherry_pick_usage(opts));
-
-	if (prepare_revision_walk(revs))
+	if (prepare_revision_walk(opts->revs))
 		die(_("revision walk setup failed"));
 
-	if (!revs->commits)
+	if (!opts->revs->commits)
 		die(_("empty commit set passed"));
 }
 
@@ -818,14 +821,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
-	struct rev_info revs;
 	struct commit *commit;
 	struct replay_insn_list **next;
 
-	prepare_revs(&revs, opts);
+	prepare_revs(opts);
 
 	next = todo_list;
-	while ((commit = get_revision(&revs)))
+	while ((commit = get_revision(opts->revs)))
 		next = replay_insn_list_append(opts->action, commit, next);
 }
 
@@ -949,6 +951,9 @@ static int pick_revisions(struct replay_opts *opts)
 	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
+	if (opts->subcommand == REPLAY_NONE)
+		assert(opts->revs);
+
 	read_and_refresh_cache(opts);
 
 	/*
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 4/5] revert: allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-10-22 19:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319310826-508-1-git-send-email-artagnon@gmail.com>

Parse the instruction sheet in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming that all instructions use
the same action.  Now you can do:

  pick fdc0b12 picked
  revert 965fed4 anotherpick

For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
This patch lays the foundation for extending the parser to support
more actions so 'git rebase -i' can reuse this machinery in the
future.  While at it, also improve the error messages reported by the
insn sheet parser.

Helped-by: Jonathan Nieder <jrnider@gmail.com>
Acked-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c                |  132 +++++++++++++++++++--------------------
 sequencer.h                     |    8 +++
 t/t3510-cherry-pick-sequence.sh |   58 +++++++++++++++++
 3 files changed, 130 insertions(+), 68 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 14462ca..8c3e4fc 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-enum replay_action { REVERT, CHERRY_PICK };
 enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
 
 struct replay_opts {
@@ -68,14 +67,14 @@ struct replay_opts {
 
 static const char *action_name(const struct replay_opts *opts)
 {
-	return opts->action == REVERT ? "revert" : "cherry-pick";
+	return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
 }
 
 static char *get_encoding(const char *message);
 
 static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
 {
-	return opts->action == REVERT ? revert_usage : cherry_pick_usage;
+	return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
 }
 
 static int option_parse_x(const struct option *opt,
@@ -152,7 +151,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		OPT_END(),
 	};
 
-	if (opts->action == CHERRY_PICK) {
+	if (opts->action == REPLAY_PICK) {
 		struct option cp_extra[] = {
 			OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
 			OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -348,7 +347,7 @@ static int error_dirty_index(struct replay_opts *opts)
 		return error_resolve_conflict(action_name(opts));
 
 	/* Different translation strings for cherry-pick and revert */
-	if (opts->action == CHERRY_PICK)
+	if (opts->action == REPLAY_PICK)
 		error(_("Your local changes would be overwritten by cherry-pick."));
 	else
 		error(_("Your local changes would be overwritten by revert."));
@@ -452,7 +451,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
 	return run_command_v_opt(args, RUN_GIT_CMD);
 }
 
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+			struct replay_opts *opts)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
@@ -527,7 +527,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 
 	defmsg = git_pathdup("MERGE_MSG");
 
-	if (opts->action == REVERT) {
+	if (action == REPLAY_REVERT) {
 		base = commit;
 		base_label = msg.label;
 		next = parent;
@@ -568,7 +568,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		}
 	}
 
-	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+	if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
 					 head, &msgbuf, opts);
 		write_message(&msgbuf, defmsg);
@@ -592,11 +592,11 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	 * However, if the merge did not even start, then we don't want to
 	 * write it at all.
 	 */
-	if (opts->action == CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
+	if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
 		write_cherry_pick_head(commit);
 
 	if (res) {
-		error(opts->action == REVERT
+		error(action == REPLAY_REVERT
 		      ? _("could not revert %s... %s")
 		      : _("could not apply %s... %s"),
 		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -620,7 +620,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
 
 	init_revisions(revs, NULL);
 	revs->no_walk = 1;
-	if (opts->action != REVERT)
+	if (opts->action != REPLAY_REVERT)
 		revs->reverse = 1;
 
 	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -666,49 +666,53 @@ static void read_and_refresh_cache(struct replay_opts *opts)
  *     assert(commit_list_count(list) == 2);
  *     return list;
  */
-static struct commit_list **commit_list_append(struct commit *commit,
-					       struct commit_list **next)
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+						struct commit *operand,
+						struct replay_insn_list **next)
 {
-	struct commit_list *new = xmalloc(sizeof(struct commit_list));
-	new->item = commit;
+	struct replay_insn_list *new = xmalloc(sizeof(*new));
+	new->action = action;
+	new->operand = operand;
 	*next = new;
 	new->next = NULL;
 	return &new->next;
 }
 
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
-		struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
 {
-	struct commit_list *cur = NULL;
-	const char *sha1_abbrev = NULL;
-	const char *action_str = opts->action == REVERT ? "revert" : "pick";
-	const char *subject;
-	int subject_len;
+	struct replay_insn_list *cur;
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
-		subject_len = find_commit_subject(cur->item->buffer, &subject);
+		const char *sha1_abbrev, *action_str, *subject;
+		int subject_len;
+
+		action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+		sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+		subject_len = find_commit_subject(cur->operand->buffer, &subject);
 		strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
 			subject_len, subject);
 	}
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
 {
 	unsigned char commit_sha1[20];
-	enum replay_action action;
 	char *end_of_object_name;
 	int saved, status;
 
 	if (!prefixcmp(bol, "pick ")) {
-		action = CHERRY_PICK;
+		item->action = REPLAY_PICK;
 		bol += strlen("pick ");
 	} else if (!prefixcmp(bol, "revert ")) {
-		action = REVERT;
+		item->action = REPLAY_REVERT;
 		bol += strlen("revert ");
-	} else
-		return NULL;
+	} else {
+		size_t len = strchrnul(bol, '\n') - bol;
+		if (len > 255)
+			len = 255;
+		return error(_("Unrecognized action: %.*s"), (int)len, bol);
+	}
 
 	end_of_object_name = bol + strcspn(bol, " \n");
 	saved = *end_of_object_name;
@@ -716,37 +720,29 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
 	status = get_sha1(bol, commit_sha1);
 	*end_of_object_name = saved;
 
-	/*
-	 * Verify that the action matches up with the one in
-	 * opts; we don't support arbitrary instructions
-	 */
-	if (action != opts->action) {
-		const char *action_str;
-		action_str = action == REVERT ? "revert" : "cherry-pick";
-		error(_("Cannot %s during a %s"), action_str, action_name(opts));
-		return NULL;
-	}
-
 	if (status < 0)
-		return NULL;
+		return error(_("Malformed object name: %s"), bol);
 
-	return lookup_commit_reference(commit_sha1);
+	item->operand = lookup_commit_reference(commit_sha1);
+	if (!item->operand)
+		return error(_("Not a valid commit: %s"), bol);
+
+	item->next = NULL;
+	return 0;
 }
 
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
-			struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
 {
-	struct commit_list **next = todo_list;
-	struct commit *commit;
+	struct replay_insn_list **next = todo_list;
+	struct replay_insn_list item = {0, NULL, NULL};
 	char *p = buf;
 	int i;
 
 	for (i = 1; *p; i++) {
 		char *eol = strchrnul(p, '\n');
-		commit = parse_insn_line(p, eol, opts);
-		if (!commit)
-			return error(_("Could not parse line %d."), i);
-		next = commit_list_append(commit, next);
+		if (parse_insn_line(p, eol, &item) < 0)
+			return error(_("on line %d."), i);
+		next = replay_insn_list_append(item.action, item.operand, next);
 		p = *eol ? eol + 1 : eol;
 	}
 	if (!*todo_list)
@@ -754,8 +750,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 	return 0;
 }
 
-static void read_populate_todo(struct commit_list **todo_list,
-			struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	struct strbuf buf = STRBUF_INIT;
@@ -771,7 +766,7 @@ static void read_populate_todo(struct commit_list **todo_list,
 	}
 	close(fd);
 
-	res = parse_insn_buffer(buf.buf, todo_list, opts);
+	res = parse_insn_buffer(buf.buf, todo_list);
 	strbuf_release(&buf);
 	if (res)
 		die(_("Unusable instruction sheet: %s"), todo_file);
@@ -820,18 +815,18 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 		die(_("Malformed options sheet: %s"), opts_file);
 }
 
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
 	struct rev_info revs;
 	struct commit *commit;
-	struct commit_list **next;
+	struct replay_insn_list **next;
 
 	prepare_revs(&revs, opts);
 
 	next = todo_list;
 	while ((commit = get_revision(&revs)))
-		next = commit_list_append(commit, next);
+		next = replay_insn_list_append(opts->action, commit, next);
 }
 
 static int create_seq_dir(void)
@@ -860,7 +855,7 @@ static void save_head(const char *head)
 		die(_("Error wrapping up %s."), head_file);
 }
 
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	static struct lock_file todo_lock;
@@ -868,7 +863,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 	int fd;
 
 	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
-	if (format_todo(&buf, todo_list, opts) < 0)
+	if (format_todo(&buf, todo_list) < 0)
 		die(_("Could not format %s."), todo_file);
 	if (write_in_full(fd, buf.buf, buf.len) < 0) {
 		strbuf_release(&buf);
@@ -912,9 +907,10 @@ static void save_opts(struct replay_opts *opts)
 	}
 }
 
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list,
+			struct replay_opts *opts)
 {
-	struct commit_list *cur;
+	struct replay_insn_list *cur;
 	int res;
 
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -924,8 +920,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 	read_and_refresh_cache(opts);
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		save_todo(cur, opts);
-		res = do_pick_commit(cur->item, opts);
+		save_todo(cur);
+		res = do_pick_commit(cur->operand, cur->action, opts);
 		if (res) {
 			if (!cur->next)
 				/*
@@ -950,7 +946,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 
 static int pick_revisions(struct replay_opts *opts)
 {
-	struct commit_list *todo_list = NULL;
+	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
 	read_and_refresh_cache(opts);
@@ -967,7 +963,7 @@ static int pick_revisions(struct replay_opts *opts)
 		if (!file_exists(git_path(SEQ_TODO_FILE)))
 			goto error;
 		read_populate_opts(&opts);
-		read_populate_todo(&todo_list, opts);
+		read_populate_todo(&todo_list);
 
 		/* Verify that the conflict has been resolved */
 		if (!index_differs_from("HEAD", 0))
@@ -987,7 +983,7 @@ static int pick_revisions(struct replay_opts *opts)
 			return -1;
 		}
 		if (get_sha1("HEAD", sha1)) {
-			if (opts->action == REVERT)
+			if (opts->action == REPLAY_REVERT)
 				return error(_("Can't revert as initial commit"));
 			return error(_("Can't cherry-pick into empty head"));
 		}
@@ -1007,7 +1003,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	if (isatty(0))
 		opts.edit = 1;
-	opts.action = REVERT;
+	opts.action = REPLAY_REVERT;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
 	res = pick_revisions(&opts);
@@ -1022,7 +1018,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	int res;
 
 	memset(&opts, 0, sizeof(opts));
-	opts.action = CHERRY_PICK;
+	opts.action = REPLAY_PICK;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
 	res = pick_revisions(&opts);
diff --git a/sequencer.h b/sequencer.h
index 905d295..f4db257 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,14 @@
 #define SEQ_TODO_FILE	"sequencer/todo"
 #define SEQ_OPTS_FILE	"sequencer/opts"
 
+enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+
+struct replay_insn_list {
+	enum replay_action action;
+	struct commit *operand;
+	struct replay_insn_list *next;
+};
+
 /*
  * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
  * any errors.  Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 39b55c1..4b12244 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -239,4 +239,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
 	test_line_count = 4 commits
 '
 
+test_expect_success 'revert --continue continues after cherry-pick' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	git revert --continue &&
+	test_path_is_missing .git/sequencer &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	oldsha=`git rev-parse --short HEAD~1` &&
+	echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	foo
+	OBJID
+	:100644 100644 OBJID OBJID M	unrelated
+	OBJID
+	:000000 100644 OBJID OBJID A	foo
+	:000000 100644 OBJID OBJID A	unrelated
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 3/5] revert: make commit subjects in insn sheet optional
From: Ramkumar Ramachandra @ 2011-10-22 19:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319310826-508-1-git-send-email-artagnon@gmail.com>

Change the instruction sheet format subtly so that the subject of the
commit message that follows the object name is optional.  As a result,
an instruction sheet like this is now perfectly valid:

  pick 35b0426
  pick fbd5bbcbc2e
  pick 7362160f

While at it, also fix a bug: currently, we use a commit-id-shaped
buffer to store the word after "pick" in '.git/sequencer/todo'.  This
is both wasteful and wrong because it places an artificial limit on
the line length.  Eliminate the need for the buffer altogether, and
add a test demonstrating this.

[jc: simplify parsing]

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c                |   37 ++++++++++++++++---------------------
 t/t3510-cherry-pick-sequence.sh |   28 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index efa8d00..14462ca 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -694,31 +694,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
 {
 	unsigned char commit_sha1[20];
-	char sha1_abbrev[40];
 	enum replay_action action;
-	int insn_len = 0;
-	char *p, *q;
+	char *end_of_object_name;
+	int saved, status;
 
-	if (!prefixcmp(start, "pick ")) {
+	if (!prefixcmp(bol, "pick ")) {
 		action = CHERRY_PICK;
-		insn_len = strlen("pick");
-		p = start + insn_len + 1;
-	} else if (!prefixcmp(start, "revert ")) {
+		bol += strlen("pick ");
+	} else if (!prefixcmp(bol, "revert ")) {
 		action = REVERT;
-		insn_len = strlen("revert");
-		p = start + insn_len + 1;
+		bol += strlen("revert ");
 	} else
 		return NULL;
 
-	q = strchr(p, ' ');
-	if (!q)
-		return NULL;
-	q++;
-
-	strlcpy(sha1_abbrev, p, q - p);
+	end_of_object_name = bol + strcspn(bol, " \n");
+	saved = *end_of_object_name;
+	*end_of_object_name = '\0';
+	status = get_sha1(bol, commit_sha1);
+	*end_of_object_name = saved;
 
 	/*
 	 * Verify that the action matches up with the one in
@@ -731,7 +727,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 		return NULL;
 	}
 
-	if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+	if (status < 0)
 		return NULL;
 
 	return lookup_commit_reference(commit_sha1);
@@ -746,13 +742,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 	int i;
 
 	for (i = 1; *p; i++) {
-		commit = parse_insn_line(p, opts);
+		char *eol = strchrnul(p, '\n');
+		commit = parse_insn_line(p, eol, opts);
 		if (!commit)
 			return error(_("Could not parse line %d."), i);
 		next = commit_list_append(commit, next);
-		p = strchrnul(p, '\n');
-		if (*p)
-			p++;
+		p = *eol ? eol + 1 : eol;
 	}
 	if (!*todo_list)
 		return error(_("No commits parsed."));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..39b55c1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -12,6 +12,9 @@ test_description='Test cherry-pick continuation features
 
 . ./test-lib.sh
 
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
 pristine_detach () {
 	git cherry-pick --reset &&
 	git checkout -f "$1^0" &&
@@ -211,4 +214,29 @@ test_expect_success 'malformed instruction sheet 2' '
 	test_must_fail git cherry-pick --continue
 '
 
+test_expect_success 'malformed instruction sheet 3' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "resolved" >foo &&
+	git add foo &&
+	git commit &&
+	sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	test_must_fail git cherry-pick --continue
+'
+
+test_expect_success 'commit descriptions in insn sheet are optional' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	git rev-list HEAD >commits
+	test_line_count = 4 commits
+'
+
 test_done
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 2/5] revert: simplify getting commit subject in format_todo()
From: Ramkumar Ramachandra @ 2011-10-22 19:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319310826-508-1-git-send-email-artagnon@gmail.com>

format_todo() calls get_message(), but uses only the subject line of
the commit message.  Save work and unnecessary memory allocations by
using find_commit_subject() instead.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index a6f2ea7..efa8d00 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -680,16 +680,16 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 		struct replay_opts *opts)
 {
 	struct commit_list *cur = NULL;
-	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	const char *sha1_abbrev = NULL;
 	const char *action_str = opts->action == REVERT ? "revert" : "pick";
+	const char *subject;
+	int subject_len;
 
 	for (cur = todo_list; cur; cur = cur->next) {
 		sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
-		if (get_message(cur->item, &msg))
-			return error(_("Cannot get commit message for %s"), sha1_abbrev);
-		strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
-		free_message(&msg);
+		subject_len = find_commit_subject(cur->item->buffer, &subject);
+		strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+			subject_len, subject);
 	}
 	return 0;
 }
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH v3 0/5] Sequencer fixups mini-series
From: Ramkumar Ramachandra @ 2011-10-22 19:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder

Hi,

This is the third iteration of the series, with one change since the
previous round: the 3rd and 4th parts have been squashed together as
suggested by Junio and Jonathan.  As a result, it's a five-part series
instead of a six-part series now.

Second iteration: $gmane/183962
First iteration: $gmane/183157

Thanks.

Jonathan Nieder (1):
  revert: simplify communicating command-line arguments

Ramkumar Ramachandra (4):
  revert: free msg in format_todo()
  revert: simplify getting commit subject in format_todo()
  revert: make commit subjects in insn sheet optional
  revert: allow mixed pick and revert instructions

 builtin/revert.c                |  221 +++++++++++++++++++--------------------
 sequencer.h                     |    8 ++
 t/t3510-cherry-pick-sequence.sh |   86 +++++++++++++++
 3 files changed, 203 insertions(+), 112 deletions(-)

-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply

* [PATCH 1/5] revert: free msg in format_todo()
From: Ramkumar Ramachandra @ 2011-10-22 19:13 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319310826-508-1-git-send-email-artagnon@gmail.com>

Memory allocated to the fields of msg by get_message() isn't freed.
This is potentially a big leak, because fresh memory is allocated to
store the commit message for each commit.  Fix this using
free_message().

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 87df70e..a6f2ea7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -689,6 +689,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 		if (get_message(cur->item, &msg))
 			return error(_("Cannot get commit message for %s"), sha1_abbrev);
 		strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+		free_message(&msg);
 	}
 	return 0;
 }
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* Re: [PATCH 0/2] git-credential-cache--daemon on Cygwin
From: Jeff King @ 2011-10-22 19:15 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4EA2FC0D.5060404@ramsay1.demon.co.uk>

On Sat, Oct 22, 2011 at 06:23:25PM +0100, Ramsay Jones wrote:

> I had a quick look, and found that unix_stream_listen() was failing to
> bind() to a stale unix socket. The code looked OK to me, and should have
> handled this just fine, but didn't ...
> 
> On a hunch, I found that the following "belt-n-braces" approach fixed the
> the test for me:
> 
> -- >8 --
> diff --git a/unix-socket.c b/unix-socket.c
> index cf9890f..d974e01 100644
> --- a/unix-socket.c
> +++ b/unix-socket.c
> @@ -42,7 +42,9 @@ int unix_stream_listen(const char *path)
>  	fd = unix_stream_socket();
>  
>  	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
> +		close(fd);
>  		unlink(path);
> +		fd = unix_stream_socket();
>  		if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
>  			close(fd);
>  			return -1;
> -- 8< --

Wow, that seems horribly broken on the part of cygwin.

I think your solution to just unlink first() is the right way to go.  I
have a few comments, though so I'll respond to each of the patches
individually.

> Assuming that a modified http-auth-keyring series will make a return to pu
> sometime, could you please squash these patches into (the patch corresponding to)
> commit 2d6874d (credentials: add "cache" helper, 18-07-2011). Thanks!

I'm planning a reroll, so I'll squash them (or something similar) in.

> Also, note that this series breaks the build on MinGW. The patch to fix the build
> is rather simple, but the result doesn't work, of course, because winsock does
> not know about AF_UNIX (or AF_LOCAL). I started to code an win32 emulation of unix
> sockets, but stopped working on it when this branch dropped from next. If you
> intend to re-submit this work, then I can pick this up again.

Right. I sort of expected that, and figured we could just drop the cache
helper on mingw until somebody felt like writing such an emulation
layer.

It's definitely coming back, so if you feel like working on it, please
do. Also note that if it would be easier to have an alternate
abstraction for inter-process communication on windows, I'm open to
doing that in the cache daemon.

-Peff

^ permalink raw reply

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Jeff King @ 2011-10-22 19:09 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Ævar Arnfjörð
In-Reply-To: <1319277881-4128-1-git-send-email-pclouds@gmail.com>

On Sat, Oct 22, 2011 at 09:04:19PM +1100, Nguyen Thai Ngoc Duy wrote:

> This series helps pass commit message size up to output functions,
> though it does not change any output functions to print ^@.

Can we take a step back for a second and discuss what git _should_ do
with commits that contain NUL?

If all of the pretty-print functions are just going consider "foo\0bar"
to be "foo^@bar", then maybe it would be much simpler to just
"normalize" the commit message into a C string at a lower level, and
pass it around as a string as we currently do.

On the other hand, if we are eventually looking to add an option like
"--include-NUL-in-commit-message", then it would make sense for the
real contents and size to get passed around.

> All functions up to the last patch learn to accept a string as a pair
> <const char *start, const char *end> as a preparation step. These
> changes are relatively simple. Or it could have been so if I did not
> attempt to reduce some code duplication found while working on this
> series.

Great. Reducing code duplication is always a plus.

> The last patch turns commit_buffer field in struct commit to "struct
> strbuf *". This approach costs us 12 bytes more each commit. We can
> choose not to use strbuf to save memory.

I think 12 bytes in the commit struct might be noticeable. But it looks
like you've done the sane thing, and replaced the pointer-to-char with a
pointer-to-strbuf. And that I don't think should be a big deal. The
buffer itself is way bigger than 12 bytes, so we don't care so much
about the "we have a buffer" case, but more about the 100,000 other
commits that we're not currently printing right now.

Of course, some timings on things like "rev-list" and "pack-objects"
would be nice to double-check.

-Peff

^ permalink raw reply

* Re: [PATCH 3/6] revert: fix buffer overflow in insn sheet parser
From: Ramkumar Ramachandra @ 2011-10-22 18:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, Git List, Christian Couder
In-Reply-To: <7vaa8va3xm.fsf@alter.siamese.dyndns.org>

Hi Junio and Jonathan,

Thanks for the good suggestions.  Will post the next iteration in a
few minutes (some tests running now).

Junio C Hamano writes:
> [...]
> When the log message justifies the cause and the approach in the right
> way, the actual patch becomes self evident. Also I often find myself
> coming up with a _better_ solution than the patch I originally prepared
> while writing the commit log message to explain it, and redoing the patch
> text to match the description.

Wow.  It looks like I have a long way to go :/
Maybe I should practice writing more Haskell.

-- Ram

^ permalink raw reply

* Re: [PATCH 7/5] pretty: %G[?GS] placeholders
From: Junio C Hamano @ 2011-10-22 17:55 UTC (permalink / raw)
  To: Elia Pinto; +Cc: git
In-Reply-To: <CA+EOSB=EowzV5B9jjq9D9rshPt8LmO9K_GbwNWo_x3Uv9+kwxg@mail.gmail.com>

Elia Pinto <gitter.spiros@gmail.com> writes:

> Can you suggest what do you think can be useful placeholders ? Thanks.

That is a weird question.

> 2011/10/22, Junio C Hamano <gitster@pobox.com>:
>> Add new placeholders related to the GPG signature on signed commits.
>>
>>  - %GG to show the raw verification message from GPG;
>>  - %G? to show either "G" for Good, "B" for Bad;
>>  - %GS to show the name of the signer.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  * The 6th is the one that works with a bogus commit with NUL in it I sent
>>    out previously.
>>
>>    This concludes the series; I'll leave the design and implementation of
>>    other useful placeholders to the list for now.

I can think of random other placeholders off the top of my head purely by
speculation without having real need [*1*], but they won't be much useful.

People on the list who *want* to use this feature in their projects may
have specific needs and they would be closer to what is needed in the real
world use cases than what comes out of thin air by imagination.

That is the reason why I left the enhancement to the list.

If you have to ask that question because you do not have any specific need
yourself, and especially if you have to ask it to *me*, then you should
wait for others to come up with their real needs, just like what I am
doing right now ;-).


[Footnote]

*1*

 - %GC that is replaced with COLOR_GREEN when Good signature is found,
   COLOR_RED when BAD signature is found, and COLOR_RESET when there is no
   signature;

 - %GD for the date the signature was made on (with date format variants);

 - %Gk for the type of the key and the Key-ID.
 

^ permalink raw reply

* Re: [PATCH] make the sample pre-commit hook script reject names with newlines, too
From: Jim Meyering @ 2011-10-22 17:44 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: git list
In-Reply-To: <m262jhoro1.fsf@igel.home>

Andreas Schwab wrote:
> Jim Meyering <jim@meyering.net> writes:
>
>> @@ -26,7 +29,7 @@ if [ "$allownonascii" != "true" ] &&
>>  	# even required, for portability to Solaris 10's /usr/bin/tr), since
>>  	# the square bracket bytes happen to fall in the designated range.
>>  	test "$(git diff --cached --name-only --diff-filter=A -z $against |
>> -	  LC_ALL=C tr -d '[ -~]\0')"
>> +	  LC_ALL=C tr -d '[ -~]\0' | wc -c)" != 0
>
> This will fail if the output of wc contains leading spaces.

Good point.  Thanks.  That's a portability bug.
GNU wc outputs no leading spaces, but others certainly do.

Removing the double quotes fixes that:

-- >8 --
Subject: [PATCH] make the sample pre-commit hook script reject names with
 newlines, too

The sample pre-commit hook script would fail to reject a file name
like "a\nb" because of the way newlines are handled in "$(...)".
Adjust the test to count filtered bytes and require there be 0.
Also print all diagnostics to standard error, not stdout, so they
will actually be seen.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 templates/hooks--pre-commit.sample |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index b187c4b..18c4829 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -18,6 +18,9 @@ fi
 # If you want to allow non-ascii filenames set this variable to true.
 allownonascii=$(git config hooks.allownonascii)

+# Redirect output to stderr.
+exec 1>&2
+
 # Cross platform projects tend to avoid non-ascii filenames; prevent
 # them from being added to the repository. We exploit the fact that the
 # printable range starts at the space character and ends with tilde.
@@ -25,8 +28,8 @@ if [ "$allownonascii" != "true" ] &&
 	# Note that the use of brackets around a tr range is ok here, (it's
 	# even required, for portability to Solaris 10's /usr/bin/tr), since
 	# the square bracket bytes happen to fall in the designated range.
-	test "$(git diff --cached --name-only --diff-filter=A -z $against |
-	  LC_ALL=C tr -d '[ -~]\0')"
+	test $(git diff --cached --name-only --diff-filter=A -z $against |
+	  LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0
 then
 	echo "Error: Attempt to add a non-ascii file name."
 	echo
@@ -43,4 +46,5 @@ then
 	exit 1
 fi

+# If there are whitespace errors, print the offending file names and fail.
 exec git diff-index --check --cached $against --
--
1.7.7.419.g87009

^ permalink raw reply related

* Re: [PATCH] make the sample pre-commit hook script reject names with newlines, too
From: Andreas Schwab @ 2011-10-22 17:35 UTC (permalink / raw)
  To: Jim Meyering; +Cc: git list
In-Reply-To: <87obx9eygk.fsf@rho.meyering.net>

Jim Meyering <jim@meyering.net> writes:

> @@ -26,7 +29,7 @@ if [ "$allownonascii" != "true" ] &&
>  	# even required, for portability to Solaris 10's /usr/bin/tr), since
>  	# the square bracket bytes happen to fall in the designated range.
>  	test "$(git diff --cached --name-only --diff-filter=A -z $against |
> -	  LC_ALL=C tr -d '[ -~]\0')"
> +	  LC_ALL=C tr -d '[ -~]\0' | wc -c)" != 0

This will fail if the output of wc contains leading spaces.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* [PATCH 2/2] credential-cache--daemon.c: unlink() a potentially stale unix socket
From: Ramsay Jones @ 2011-10-22 17:25 UTC (permalink / raw)
  To: Jeff King; +Cc: GIT Mailing-list



Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 credential-cache--daemon.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index ee2c15a..c5fb1b2 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -230,6 +230,7 @@ static void serve_cache(const char *socket_path)
 {
 	int fd;
 
+	unlink(socket_path);
 	fd = unix_stream_listen(socket_path);
 	if (fd < 0)
 		die_errno("unable to bind to '%s'", socket_path);
-- 
1.7.7

^ permalink raw reply related

* [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit
From: Ramsay Jones @ 2011-10-22 17:24 UTC (permalink / raw)
  To: Jeff King; +Cc: GIT Mailing-list



Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 credential-cache--daemon.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 128c5ce..ee2c15a 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -137,22 +137,22 @@ static int read_credential_request(FILE *fh, struct credential *c,
 	return 0;
 }
 
-static void serve_one_client(FILE *in, FILE *out)
+static int serve_one_client(FILE *in, FILE *out)
 {
 	struct credential c = { NULL };
 	int timeout = -1;
 	char *action = NULL;
 
 	if (read_credential_request(in, &c, &action, &timeout) < 0)
-		return;
+		return 1;
 
 	if (!action) {
 		warning("cache client didn't specify an action");
-		return;
+		return 1;
 	}
 
 	if (!strcmp(action, "exit"))
-		exit(0);
+		return 0;
 
 	if (!strcmp(action, "get")) {
 		struct credential_cache_entry *e = lookup_credential(&c);
@@ -160,27 +160,27 @@ static void serve_one_client(FILE *in, FILE *out)
 			fprintf(out, "username=%s\n", e->item.username);
 			fprintf(out, "password=%s\n", e->item.password);
 		}
-		return;
+		return 1;
 	}
 
 	if (!strcmp(action, "erase")) {
 		remove_credential(&c);
-		return;
+		return 1;
 	}
 
 	if (!strcmp(action, "store")) {
 		if (timeout < 0) {
 			warning("cache client didn't specify a timeout");
-			return;
+			return 1;
 		}
 
 		remove_credential(&c);
 		cache_credential(&c, timeout);
-		return;
+		return 1;
 	}
 
 	warning("cache client sent unknown action: %s", action);
-	return;
+	return 1;
 }
 
 static int serve_cache_loop(int fd)
@@ -201,7 +201,7 @@ static int serve_cache_loop(int fd)
 	}
 
 	if (pfd.revents & POLLIN) {
-		int client, client2;
+		int client, client2, ret;
 		FILE *in, *out;
 
 		client = accept(fd, NULL, NULL);
@@ -218,9 +218,10 @@ static int serve_cache_loop(int fd)
 
 		in = xfdopen(client, "r");
 		out = xfdopen(client2, "w");
-		serve_one_client(in, out);
+		ret = serve_one_client(in, out);
 		fclose(in);
 		fclose(out);
+		return ret;
 	}
 	return 1;
 }
-- 
1.7.7

^ permalink raw reply related

* [PATCH 0/2] git-credential-cache--daemon on Cygwin
From: Ramsay Jones @ 2011-10-22 17:23 UTC (permalink / raw)
  To: Jeff King; +Cc: GIT Mailing-list

Hi Jeff,

When the 'jk/http-auth-keyring' branch was in next (and later back
in pu), I noticed that the t0300-credentials.sh test failing on cygwin.

I had a quick look, and found that unix_stream_listen() was failing to
bind() to a stale unix socket. The code looked OK to me, and should have
handled this just fine, but didn't ...

On a hunch, I found that the following "belt-n-braces" approach fixed the
the test for me:

-- >8 --
diff --git a/unix-socket.c b/unix-socket.c
index cf9890f..d974e01 100644
--- a/unix-socket.c
+++ b/unix-socket.c
@@ -42,7 +42,9 @@ int unix_stream_listen(const char *path)
 	fd = unix_stream_socket();
 
 	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+		close(fd);
 		unlink(path);
+		fd = unix_stream_socket();
 		if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
 			close(fd);
 			return -1;
-- 8< --

However, I don't particularly like the above solution, so I decided to take
a look at credential-cache--daemon to see why it was leaving a stale socket
in the first place. The following patches are the result:

    [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit
    [PATCH 2/2] credential-cache--daemon.c: unlink() a potentially stale unix socket

Note that, in serve_one_client(), the "--exit" action results in the server
exit(0)-ing immediately without close()-ing and unlink()-ing the socket.
So, patch #1 changes serve_one_client() to return a value to serve_cache_loop()
which indicates whether it's caller (serve_cache()) should exit from the main
server loop. This results in the socket being cleaned up correctly in the "--exit"
case.

Note that this does not eliminate all early-exit code paths (for example, we note
an die_errno() call), so it is still possible for the server to exit without
having cleaned up the socket, so patch #2 adds on unlink() call to the beginning of
serve_cache().

Note that each of these patches, separately and together, fix the test on cygwin.

Assuming that a modified http-auth-keyring series will make a return to pu
sometime, could you please squash these patches into (the patch corresponding to)
commit 2d6874d (credentials: add "cache" helper, 18-07-2011). Thanks!

Also, note that this series breaks the build on MinGW. The patch to fix the build
is rather simple, but the result doesn't work, of course, because winsock does
not know about AF_UNIX (or AF_LOCAL). I started to code an win32 emulation of unix
sockets, but stopped working on it when this branch dropped from next. If you
intend to re-submit this work, then I can pick this up again.

HTH.

ATB,
Ramsay Jones

^ permalink raw reply related

* [PATCH] make the sample pre-commit hook script reject names with newlines, too
From: Jim Meyering @ 2011-10-22 17:19 UTC (permalink / raw)
  To: git list


The sample pre-commit hook script would fail to reject a file name
like "a\nb" because of the way newlines are handled in "$(...)".
Adjust the test to count filtered bytes and require there be 0.
Also print all diagnostics to standard error, not stdout, so they
will actually be seen.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 templates/hooks--pre-commit.sample |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index b187c4b..1addec5 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -18,6 +18,9 @@ fi
 # If you want to allow non-ascii filenames set this variable to true.
 allownonascii=$(git config hooks.allownonascii)

+# Redirect output to stderr.
+exec 1>&2
+
 # Cross platform projects tend to avoid non-ascii filenames; prevent
 # them from being added to the repository. We exploit the fact that the
 # printable range starts at the space character and ends with tilde.
@@ -26,7 +29,7 @@ if [ "$allownonascii" != "true" ] &&
 	# even required, for portability to Solaris 10's /usr/bin/tr), since
 	# the square bracket bytes happen to fall in the designated range.
 	test "$(git diff --cached --name-only --diff-filter=A -z $against |
-	  LC_ALL=C tr -d '[ -~]\0')"
+	  LC_ALL=C tr -d '[ -~]\0' | wc -c)" != 0
 then
 	echo "Error: Attempt to add a non-ascii file name."
 	echo
@@ -43,4 +46,5 @@ then
 	exit 1
 fi

+# If there are whitespace errors, print the offending file names and fail.
 exec git diff-index --check --cached $against --
--
1.7.7.419.g87009

^ permalink raw reply related

* Re: [PATCH] git-gui: delegate selection from gutter columns to text output
From: Bert Wesarg @ 2011-10-22 15:41 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git
In-Reply-To: <87ehy63rvx.fsf@fox.patthoyts.tk>

On Sat, Oct 22, 2011 at 00:24, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>>Selecting in the gutter columns of the blame view should make no sense,
>>so delegate any selection action in these columns to the text output
>>by selecting whole lines there.
>>
>>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>---
>> git-gui.sh    |   20 ++++++++++++++++++++
>> lib/blame.tcl |    4 +++-
>> 2 files changed, 23 insertions(+), 1 deletions(-)
>>
>>diff --git a/git-gui.sh b/git-gui.sh
>>index 21033cb..cf5ed79 100755
>>--- a/git-gui.sh
>>+++ b/git-gui.sh
>>@@ -2077,6 +2077,26 @@ proc many2scrollbar {list mode sb top bottom} {
>>       foreach w $list {$w $mode moveto $top}
>> }
>>
>>+proc delegate_sel_to {w from} {
>>+      set bind_list [list \
>>+              <Button-1> \
>>+              <B1-Motion> \
>>+              <Double-Button-1> \
>>+              <Triple-Button-1> \
>>+              <Shift-Button-1> \
>>+              <Double-Shift-Button-1> \
>>+              <Triple-Shift-Button-1> \
>>+      ]
>>+
>>+      foreach seq $bind_list {
>>+              set script [bind Text $seq]
>>+              set new_script [string map [list %W $w %x 0 word line] $script]
>>+              foreach f $from {
>>+                      bind $f $seq "$new_script; break"
>>+              }
>>+      }
>>+}
>>+
>> proc incr_font_size {font {amt 1}} {
>>       set sz [font configure $font -size]
>>       incr sz $amt
>>diff --git a/lib/blame.tcl b/lib/blame.tcl
>>index 49eae19..9ab0da5 100644
>>--- a/lib/blame.tcl
>>+++ b/lib/blame.tcl
>>@@ -210,6 +210,8 @@ constructor new {i_commit i_path i_jump} {
>>
>>       set w_columns [list $w_amov $w_asim $w_line $w_file]
>>
>>+      delegate_sel_to $w_file [list $w_amov $w_asim $w_line]
>>+
>>       ${NS}::scrollbar $w.file_pane.out.sbx \
>>               -orient h \
>>               -command [list $w_file xview]
>>@@ -315,7 +317,7 @@ constructor new {i_commit i_path i_jump} {
>>               $i conf -yscrollcommand \
>>                       "[list ::searchbar::scrolled $finder]
>>                        [list many2scrollbar $w_columns yview $w.file_pane.out.sby]"
>>-              bind $i <Button-1> "
>>+              bind $i <Button-1> "+
>>                       [cb _hide_tooltip]
>>                       [cb _click $i @%x,%y]
>>                       focus $i
>
> The patch seems to be fine but I don't think I agree with the intention
> here. Currently clicking anywhere that is not marked as a link (blue
> underlined text) selects a commit and shows information in the lower
> pane. With this change, the left hand columns become inactive in regards
> to selecting a commit. I don't see why that is desirable.

Sorry, this was not intended. And I thought I took care for it with
the last hunk. I have a look at it again.

Bert

>
> --
> Pat Thoyts                            http://www.patthoyts.tk/
> PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
>

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2011, #08; Fri, 21)
From: Jakub Narebski @ 2011-10-22 13:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kato Kazuyoshi
In-Reply-To: <m3r5256h76.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:

> > * kk/gitweb-side-by-side-diff (2011-10-17) 2 commits
> >  - gitweb: add a feature to show side-by-side diff
> >  - gitweb: change format_diff_line() to remove leading SP from $diff_class
> > 
> > Fun.
> > Will keep in 'pu' until the planned re-roll comes.
> 
> I think this needs some more work, not only re-roll...
> 
> 
> BTW. the bottom commit could be I think replaced by mine
> 
>    - gitweb: Refactor diff body line classification

Nb. that needs a easy but non-trivial merge conflict resolution,
see e.g.

-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
The following changes since commit 8963314c77af9a4eda5dcbdbab3d4001af83ad81:

  Sync with maint (2011-10-21 11:24:34 -0700)

are available in the git repository at:

  git://repo.or.cz/git/jnareb-git.git gitweb/side-by-side-diff

Jakub Narebski (1):
      gitweb: Refactor diff body line classification

Kato Kazuyoshi (1):
      gitweb: add a feature to show side-by-side diff

 gitweb/gitweb.perl       |  144 ++++++++++++++++++++++++++++++++++------------
 gitweb/static/gitweb.css |   15 +++++
 2 files changed, 123 insertions(+), 36 deletions(-)

-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD
From: Jonathan Nieder @ 2011-10-22 11:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Alex Riesen, Christopher M. Fuhrman, Greg Troxel,
	Stefan Sperling
In-Reply-To: <20111003064120.GA24396@elie>

(people cc-ed: your input would be welcome on [*] below.  See commit
81a24b52, "Do not use GUID on dir in git init --shared=all on FreeBSD"
for context)

Hi Junio,

>From Documentation/RelNotes/1.7.7.1.txt:

 * On some BSD systems, adding +s bit on directories is detrimental
   (it is not necessary on BSD to begin with). The installation
   procedure has been updated to take this into account.

I assume this is referring to 0b20dd8f (Makefile: do not set setgid
bit on directories on GNU/kFreeBSD, 2011-10-03), which admittedly
does have a subject line that suggests it would be about that (sorry
about that).  The change was actually about "git init -s" which sets
the setgid bit on SysV-style systems to allow shared access to a
repository (and can provoke errors on BSD-style systems, depending on
how permissive the filesystem in use wants to be).

More to the point, the patch was just taking a fix that arrived for
FreeBSD in v1.5.5 days and making it also apply to machines using an
(obscure) GNU userland/FreeBSD kernel mixture.

By the way, maybe other BSD-style ports (NetBSD, OpenBSD) should be
setting DIR_HAS_BSD_GROUP_SEMANTICS to get this fix, too[*]?  Then the
release notes could look something like this:

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/RelNotes/1.7.7.1.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git i/Documentation/RelNotes/1.7.7.1.txt w/Documentation/RelNotes/1.7.7.1.txt
index fecfac8a..e3c29ff0 100644
--- i/Documentation/RelNotes/1.7.7.1.txt
+++ w/Documentation/RelNotes/1.7.7.1.txt
@@ -5,8 +5,9 @@ Fixes since v1.7.7
 ------------------
 
  * On some BSD systems, adding +s bit on directories is detrimental
-   (it is not necessary on BSD to begin with). The installation
-   procedure has been updated to take this into account.
+   (it is not necessary on BSD to begin with). "git init --shared"
+   has been updated to take this into account without extra makefile
+   settings on platforms the Makefile knows about.
 
  * After incorrectly written third-party tools store a tag object in
    HEAD, git diagnosed it as a repository corruption and refused to
-- 

^ permalink raw reply related

* Re: [PATCH 7/5] pretty: %G[?GS] placeholders
From: Elia Pinto @ 2011-10-22 10:47 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <7v7h3x7h6j.fsf_-_@alter.siamese.dyndns.org>

Can you suggest what do you think can be useful placeholders ? Thanks.

2011/10/22, Junio C Hamano <gitster@pobox.com>:
> Add new placeholders related to the GPG signature on signed commits.
>
>  - %GG to show the raw verification message from GPG;
>  - %G? to show either "G" for Good, "B" for Bad;
>  - %GS to show the name of the signer.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * The 6th is the one that works with a bogus commit with NUL in it I sent
>    out previously.
>
>    This concludes the series; I'll leave the design and implementation of
>    other useful placeholders to the list for now.
>
>  pretty.c |   86
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 86 insertions(+), 0 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index f45eb54..392d656 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -9,6 +9,7 @@
>  #include "notes.h"
>  #include "color.h"
>  #include "reflog-walk.h"
> +#include "gpg-interface.h"
>
>  static char *user_format;
>  static struct cmt_fmt_map {
> @@ -640,6 +641,12 @@ struct format_commit_context {
>  	const struct pretty_print_context *pretty_ctx;
>  	unsigned commit_header_parsed:1;
>  	unsigned commit_message_parsed:1;
> +	unsigned commit_signature_parsed:1;
> +	struct {
> +		char *gpg_output;
> +		char good_bad;
> +		char *signer;
> +	} signature;
>  	char *message;
>  	size_t width, indent1, indent2;
>
> @@ -822,6 +829,59 @@ static void rewrap_message_tail(struct strbuf *sb,
>  	c->indent2 = new_indent2;
>  }
>
> +static struct {
> +	char result;
> +	const char *check;
> +} signature_check[] = {
> +	{ 'G', ": Good signature from " },
> +	{ 'B', ": BAD signature from " },
> +};
> +
> +static void parse_signature_lines(struct format_commit_context *ctx)
> +{
> +	const char *buf = ctx->signature.gpg_output;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(signature_check); i++) {
> +		const char *found = strstr(buf, signature_check[i].check);
> +		const char *next;
> +		if (!found)
> +			continue;
> +		ctx->signature.good_bad = signature_check[i].result;
> +		found += strlen(signature_check[i].check);
> +		next = strchrnul(found, '\n');
> +		ctx->signature.signer = xmemdupz(found, next - found);
> +		break;
> +	}
> +}
> +
> +static void parse_commit_signature(struct format_commit_context *ctx)
> +{
> +	struct strbuf payload = STRBUF_INIT;
> +	struct strbuf signature = STRBUF_INIT;
> +	struct strbuf gpg_output = STRBUF_INIT;
> +	int status;
> +
> +	ctx->commit_signature_parsed = 1;
> +
> +	if (parse_signed_commit(ctx->commit->object.sha1,
> +				&payload, &signature) <= 0)
> +		goto out;
> +	status = verify_signed_buffer(payload.buf, payload.len,
> +				      signature.buf, signature.len,
> +				      &gpg_output);
> +	if (status && !gpg_output.len)
> +		goto out;
> +	ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL);
> +	parse_signature_lines(ctx);
> +
> + out:
> +	strbuf_release(&gpg_output);
> +	strbuf_release(&payload);
> +	strbuf_release(&signature);
> +}
> +
> +
>  static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
>  				void *context)
>  {
> @@ -974,6 +1034,30 @@ static size_t format_commit_one(struct strbuf *sb,
> const char *placeholder,
>  		return 0;
>  	}
>
> +	if (placeholder[0] == 'G') {
> +		if (!c->commit_signature_parsed)
> +			parse_commit_signature(c);
> +		switch (placeholder[1]) {
> +		case 'G':
> +			if (c->signature.gpg_output)
> +				strbuf_addstr(sb, c->signature.gpg_output);
> +			break;
> +		case '?':
> +			switch (c->signature.good_bad) {
> +			case 'G':
> +			case 'B':
> +				strbuf_addch(sb, c->signature.good_bad);
> +			}
> +			break;
> +		case 'S':
> +			if (c->signature.signer)
> +				strbuf_addstr(sb, c->signature.signer);
> +			break;
> +		}
> +		return 2;
> +	}
> +
> +
>  	/* For the rest we have to parse the commit header. */
>  	if (!c->commit_header_parsed)
>  		parse_commit_header(c);
> @@ -1114,6 +1198,8 @@ void format_commit_message(const struct commit
> *commit,
>
>  	if (context.message != commit->buffer)
>  		free(context.message);
> +	free(context.signature.gpg_output);
> +	free(context.signature.signer);
>  }
>
>  static void pp_header(const struct pretty_print_context *pp,
> --
> 1.7.7.555.g02edb3
>
> --
> 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
>

-- 
Inviato dal mio dispositivo mobile

^ permalink raw reply

* Re: Breakage in master since 6d4bb3833c
From: Michael Haggerty @ 2011-10-22  5:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER Gábor, git discussion list
In-Reply-To: <7vehy68ejp.fsf@alter.siamese.dyndns.org>

On 10/21/2011 07:01 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> Yes, you are right.  Setting GIT=$(pwd)/bin-wrappers/git fixes the problem.
> 
> So in short, this was a false alarm crying wolf, and there was no problem?

Correct, it was my fault for not running the locally-compiled version of
git the correct way to get consistent invocation of subprocesses.  So
there was no problem.

...but there arguably *is* a metaproblem, namely that the obvious naive
way to invoke a locally-compiled test version of git without installing
it neither works correctly nor fails loudly.  It sometimes works (if the
main process doesn't need to call any subprocesses), sometimes works
accidentally (if the subprocess that is used happens to have the
behavior expected by the test version) and sometimes fails bizarrely (as
in my case and other cases recently mentioned on the mailing list).

I can think of a few ugly hacks that could improve the situation:

1. Don't compile executables into the project root directory, but rather
into a subdirectory named something awful like
"do-not-run-commands-from-this-directory" with a big README.txt in the
directory explaining how the commands *should* be run.  Even knowing
that one has to RTFM would be a help.

2. Include a special file like "GIT-DEV-SETUP" in the directory to which
the executables are compiled, but don't copy this file to $BINDIR when
git is installed.  Teach git commands to check for the presence of
$(dirname $0)/GIT-DEV-SETUP, and if found, do the equivalent of "exec
GIT-DEV-SETUP "$@"" or maybe just read and use some values out of
GIT-DEV-SETUP to set up the correct environment.  There could be some
environment variable like GIT_SETUP_DONE to prevent recursion and/or
allow this step to be bypassed.

3. Have git commands tell git subcommands what version they are
expecting (either via an environment variable or via a hidden
command-line parameter), and have the subcommand barf if the version
does not match its own internal version.  This approach is more
intrusive but would also help defend against inconsistently-installed
versions (like having one version installed in /usr/bin and fragments of
another version installed in $HOME/bin).

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* [PATCH 7/5] pretty: %G[?GS] placeholders
From: Junio C Hamano @ 2011-10-22  5:01 UTC (permalink / raw)
  To: git
In-Reply-To: <1319071023-31919-1-git-send-email-gitster@pobox.com>

Add new placeholders related to the GPG signature on signed commits.

 - %GG to show the raw verification message from GPG;
 - %G? to show either "G" for Good, "B" for Bad;
 - %GS to show the name of the signer.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * The 6th is the one that works with a bogus commit with NUL in it I sent
   out previously.

   This concludes the series; I'll leave the design and implementation of
   other useful placeholders to the list for now.

 pretty.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/pretty.c b/pretty.c
index f45eb54..392d656 100644
--- a/pretty.c
+++ b/pretty.c
@@ -9,6 +9,7 @@
 #include "notes.h"
 #include "color.h"
 #include "reflog-walk.h"
+#include "gpg-interface.h"
 
 static char *user_format;
 static struct cmt_fmt_map {
@@ -640,6 +641,12 @@ struct format_commit_context {
 	const struct pretty_print_context *pretty_ctx;
 	unsigned commit_header_parsed:1;
 	unsigned commit_message_parsed:1;
+	unsigned commit_signature_parsed:1;
+	struct {
+		char *gpg_output;
+		char good_bad;
+		char *signer;
+	} signature;
 	char *message;
 	size_t width, indent1, indent2;
 
@@ -822,6 +829,59 @@ static void rewrap_message_tail(struct strbuf *sb,
 	c->indent2 = new_indent2;
 }
 
+static struct {
+	char result;
+	const char *check;
+} signature_check[] = {
+	{ 'G', ": Good signature from " },
+	{ 'B', ": BAD signature from " },
+};
+
+static void parse_signature_lines(struct format_commit_context *ctx)
+{
+	const char *buf = ctx->signature.gpg_output;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(signature_check); i++) {
+		const char *found = strstr(buf, signature_check[i].check);
+		const char *next;
+		if (!found)
+			continue;
+		ctx->signature.good_bad = signature_check[i].result;
+		found += strlen(signature_check[i].check);
+		next = strchrnul(found, '\n');
+		ctx->signature.signer = xmemdupz(found, next - found);
+		break;
+	}
+}
+
+static void parse_commit_signature(struct format_commit_context *ctx)
+{
+	struct strbuf payload = STRBUF_INIT;
+	struct strbuf signature = STRBUF_INIT;
+	struct strbuf gpg_output = STRBUF_INIT;
+	int status;
+
+	ctx->commit_signature_parsed = 1;
+
+	if (parse_signed_commit(ctx->commit->object.sha1,
+				&payload, &signature) <= 0)
+		goto out;
+	status = verify_signed_buffer(payload.buf, payload.len,
+				      signature.buf, signature.len,
+				      &gpg_output);
+	if (status && !gpg_output.len)
+		goto out;
+	ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL);
+	parse_signature_lines(ctx);
+
+ out:
+	strbuf_release(&gpg_output);
+	strbuf_release(&payload);
+	strbuf_release(&signature);
+}
+
+
 static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 				void *context)
 {
@@ -974,6 +1034,30 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		return 0;
 	}
 
+	if (placeholder[0] == 'G') {
+		if (!c->commit_signature_parsed)
+			parse_commit_signature(c);
+		switch (placeholder[1]) {
+		case 'G':
+			if (c->signature.gpg_output)
+				strbuf_addstr(sb, c->signature.gpg_output);
+			break;
+		case '?':
+			switch (c->signature.good_bad) {
+			case 'G':
+			case 'B':
+				strbuf_addch(sb, c->signature.good_bad);
+			}
+			break;
+		case 'S':
+			if (c->signature.signer)
+				strbuf_addstr(sb, c->signature.signer);
+			break;
+		}
+		return 2;
+	}
+
+
 	/* For the rest we have to parse the commit header. */
 	if (!c->commit_header_parsed)
 		parse_commit_header(c);
@@ -1114,6 +1198,8 @@ void format_commit_message(const struct commit *commit,
 
 	if (context.message != commit->buffer)
 		free(context.message);
+	free(context.signature.gpg_output);
+	free(context.signature.signer);
 }
 
 static void pp_header(const struct pretty_print_context *pp,
-- 
1.7.7.555.g02edb3

^ permalink raw reply related

* Re: [PATCH 1/3] MSVC: Compile fix by not including sys/resources.h
From: Junio C Hamano @ 2011-10-22  0:56 UTC (permalink / raw)
  To: Vincent van Ravesteijn; +Cc: git
In-Reply-To: <4EA1C9C9.9010904@lyx.org>

>> ...
>> Subject: [PATCH 1/3] MSVC: Compile fix by not including sys/resources.h
>> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
........................................................^^^^^^

Could you refrain from doing that?  It breaks the patch text.

Vincent van Ravesteijn <vfr@lyx.org> writes:

> Fix compilation when compiling with MSVC because sys/resource.h
> is not available. This patch causes a number of other headerfiles
> that are not available to be excluded as well.
>
> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>

Instead of current

	#ifndef mingw32 is the only one that is strange
        ... everything for systems that is not strainge ...
        #else
        ... include mingw specific tweaks ...
        #endif
        #ifdef msvc is also strange
        ... include msvc specific tweaks ...
        #endif

it turns things around and says what it wants to achieve in a more direct
way, i.e.

	#if mingw32
        #include "compat/mingw.h"
	#elif msvc
        #include "compat/msvc.h"
	#else
        ... all the others ...
	#endif

which makes it look simpler.

^ permalink raw reply

* a bug when execute "git status" in git version 1.7.7.431.g89633
From: John Hsing @ 2011-10-22  0:20 UTC (permalink / raw)
  To: git

the error:
git: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr)
(((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct
malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >=
(unsigned long)((((__builtin_offsetof (struct malloc_chunk,
fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) -
1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask)
== 0)' failed.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox