Git development
 help / color / mirror / Atom feed
* Re: a bug when execute "git status" in git version 1.7.7.431.g89633
From: John Hsing @ 2011-10-23  8:35 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqfwiknmh3.fsf@bauges.imag.fr>

ok,when i finish compiling git 1.7.7.431.g89633,I use it to check 
Cyanogenod(an Android mod source) by “git status”,it happend this 
error!but when i reuse git v1.7.7,it is OK!so i think it is a bug in 
git 1.7.7.431.g89633!My OS is Ubuntu Linux 10.10,sorry for my bad 
english! If you want to reproduce this error,please excute "git status" 
in https://github.com/CyanogenMod/android_packages_apps_DSPManager.git 
repo!

于2011年10月23日 星期日 16时25分28秒,Matthieu Moy写到:
> John Hsing <tsyj2007@gmail.com> writes:
>
>> the error:
>
> When does this error occur? is it reproducible on any repository? Which
> OS?
>
>> 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.
>
> The assertion is violated outside the source code of Git itself, so
> either Git calls malloc incorrectly, or this is a bug in your libc.
>

^ permalink raw reply

* Re: git mergetool ignores configured command line
From: Andreas Schwab @ 2011-10-23  8:37 UTC (permalink / raw)
  To: Alexander Vladimirov; +Cc: git
In-Reply-To: <CAARCrw6D7CKy2Jn43zUZ3EefyqdY6Tk4A39ZQ74H6hySA5eCBQ@mail.gmail.com>

Alexander Vladimirov <alexander.idkfa.vladimirov@gmail.com> writes:

> To configure diffuse as merge tool I set mergetool configuration using
> following commands:
>
> git config --global merge.tool diffuse
> git config --global mergetool.diffuse.cmd '/usr/bin/diffuse "$LOCAL"
> "$MERGED" "$REMOTE"'
>
> Then, when I invoke mergetool during merge, I get diffuse with four
> panes open as by default, instead of three specified in command line
> setting.
> Is this an expected behavior, and how can I force mergetool to use
> provided command line?

diffuse is already a predefined merge tool, so mergetool.diffuse.cmd is
ignored.  Try using a different name.

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

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Junio C Hamano @ 2011-10-23  9:46 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, peff, Ævar Arnfjörð
In-Reply-To: <CACsJy8CA2cqJqt7cUN1CdnOb3=qE6B2XTd1oQKZ7osVz09kSGg@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Sun, Oct 23, 2011 at 4:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> The low level object format of our commit is textual header fields, each
>> of which is terminated with a LF, followed by a LF to mark the end of
>> header fields, and then opaque payload that can contain any bytes. It does
>> not forbid a non-Git application to reuse the object store infrastructure
>> to store ASN.1 binary goo there, and the low level interface we give such
>> as cat-file is a perfectly valid way to inspect such a "commit" object.
>
> cat-file is fine, commit-tree (or any commands that call
> commit_tree()) cuts at NUL though.
> I wonder how git processes commit messages in utf-16.

That is exactly what I am saying.

Perhaps you didn't either read or understand what you omitted from your
quoting; otherwise you even wouldn't have brought up utf-16.

Let me requote that part for you.

> But when it comes to "Git" Porcelains (e.g. the log family of commands),
> we do assume people do not store random binary byte sequences in commits,
> and we do take advantage of that assumption by splitting each "line" at
> LF, indenting them with 4 spaces, etc. In other words, a commit log in the
> Git context _is_ pretty much text and not arbitrary byte sequence.

Think what would cutting at a byte whose value is 012 and adding four
bytes whose values are 040 to each of "lines" that formed with such
cutting do to UTF-16 goo, even if it does not contain any NUL byte. As far
as Git Porcelains are concerned, it is no different from random binary
byte sequences.

^ permalink raw reply

* Re: git mergetool ignores configured command line
From: Junio C Hamano @ 2011-10-23  9:52 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Alexander Vladimirov, git
In-Reply-To: <m21uu4hzmq.fsf@linux-m68k.org>

Andreas Schwab <schwab@linux-m68k.org> writes:

> diffuse is already a predefined merge tool, so mergetool.diffuse.cmd is
> ignored.  Try using a different name.

I wonder if we can improve this, though.

It is nice that we give canned definition of argument order to so many
obscure tools, but instead of ignoring mergetool.<tool>.cmd when the user
told us to use <tool>, couldn't we simply ignore what we have internally
as canned definition?

Even if such a change were too intrusive (I didn't check), could we have
a decency to at least warn that we are ignoring the configuration?

^ permalink raw reply

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Nguyen Thai Ngoc Duy @ 2011-10-23 10:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff, Ævar Arnfjörð
In-Reply-To: <7vehy459bg.fsf@alter.siamese.dyndns.org>

On Sun, Oct 23, 2011 at 8:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Sun, Oct 23, 2011 at 4:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> The low level object format of our commit is textual header fields, each
>>> of which is terminated with a LF, followed by a LF to mark the end of
>>> header fields, and then opaque payload that can contain any bytes. It does
>>> not forbid a non-Git application to reuse the object store infrastructure
>>> to store ASN.1 binary goo there, and the low level interface we give such
>>> as cat-file is a perfectly valid way to inspect such a "commit" object.
>>
>> cat-file is fine, commit-tree (or any commands that call
>> commit_tree()) cuts at NUL though.
>> I wonder how git processes commit messages in utf-16.
>
> That is exactly what I am saying.
>
> Perhaps you didn't either read or understand what you omitted from your
> quoting; otherwise you even wouldn't have brought up utf-16.
>
> Let me requote that part for you.
>
>> But when it comes to "Git" Porcelains (e.g. the log family of commands),
>> we do assume people do not store random binary byte sequences in commits,
>> and we do take advantage of that assumption by splitting each "line" at
>> LF, indenting them with 4 spaces, etc. In other words, a commit log in the
>> Git context _is_ pretty much text and not arbitrary byte sequence.
>
> Think what would cutting at a byte whose value is 012 and adding four
> bytes whose values are 040 to each of "lines" that formed with such
> cutting do to UTF-16 goo, even if it does not contain any NUL byte. As far
> as Git Porcelains are concerned, it is no different from random binary
> byte sequences.
>

I'm sorry. The utf-16 was an afterthought when I was nearly finished
with the reply and already cut that quote.

The assumption that people do not store random binary byte sequences
in commits sort of conflicts with "encoding" field in the commit
header though. The assumption is documented in i18n.txt. I guess it's
just me who did not read document carefully. But maybe it's good to
stop people from shooting themselves in this case (i.e. setting
encoding to utf-16 or similar).
-- 
Duy

^ permalink raw reply

* Re: git mergetool ignores configured command line
From: David Aguilar @ 2011-10-23 10:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Schwab, Alexander Vladimirov, git
In-Reply-To: <7vaa8s592k.fsf@alter.siamese.dyndns.org>

On Sun, Oct 23, 2011 at 02:52:19AM -0700, Junio C Hamano wrote:
> Andreas Schwab <schwab@linux-m68k.org> writes:
> 
> > diffuse is already a predefined merge tool, so mergetool.diffuse.cmd is
> > ignored.  Try using a different name.
> 
> I wonder if we can improve this, though.
> 
> It is nice that we give canned definition of argument order to so many
> obscure tools, but instead of ignoring mergetool.<tool>.cmd when the user
> told us to use <tool>, couldn't we simply ignore what we have internally
> as canned definition?
> 
> Even if such a change were too intrusive (I didn't check), could we have
> a decency to at least warn that we are ignoring the configuration?

I agree.  Ideally, we should honor their configuration.
I'll see if doing so is not too intrusive.
-- 
					David

^ permalink raw reply

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Robin Rosenberg @ 2011-10-23 10:44 UTC (permalink / raw)
  To: Jeff King
  Cc: Nguyễn Thái Ngọc Duy, git, Junio C Hamano,
	Ævar Arnfjörð
In-Reply-To: <20111022190914.GA1785@sigill.intra.peff.net>

Jeff King skrev 2011-10-22 21.09:
> 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?
Yes please. I don't think allowing NUL makes sense, but it makes sense
to state how NUL should be handled when anyone attempt it, so there
might be things to fix even if NUL is banned.

Are there any such commits in the wild?

-- robin

^ permalink raw reply

* [RFC/PATCH] sequencer: factor code out of revert builtin
From: Ramkumar Ramachandra @ 2011-10-23 11:09 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>

Start building the generalized sequencer by moving code from revert.c
into sequencer.c and sequencer.h.  Make the builtin responsible only
for command-line parsing, and expose a new sequencer_pick_revisions()
to do the actual work of sequencing commits.

This is intended to be almost a pure code movement patch with no
functional changes.  Check with:

  $ git blame -s -CCC HEAD^..HEAD -- sequencer.c | grep -C3 '^[^^]'

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Assuming that this iteration of the mini-series is alright, I quickly
 wanted to demonstrate the big "code movement" patch that I'll begin
 the next series with.  Does it look alright sans a few glitches like
 the replication of action_name()?

 builtin/revert.c |  821 +-----------------------------------------------------
 sequencer.c      |  802 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 sequencer.h      |   26 ++
 3 files changed, 828 insertions(+), 821 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index df9459b..c272920 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,19 +1,9 @@
 #include "cache.h"
 #include "builtin.h"
-#include "object.h"
-#include "commit.h"
-#include "tag.h"
-#include "run-command.h"
-#include "exec_cmd.h"
-#include "utf8.h"
 #include "parse-options.h"
-#include "cache-tree.h"
 #include "diff.h"
 #include "revision.h"
 #include "rerere.h"
-#include "merge-recursive.h"
-#include "refs.h"
-#include "dir.h"
 #include "sequencer.h"
 
 /*
@@ -39,40 +29,11 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
-
-struct replay_opts {
-	enum replay_action action;
-	enum replay_subcommand subcommand;
-
-	/* Boolean options */
-	int edit;
-	int record_origin;
-	int no_commit;
-	int signoff;
-	int allow_ff;
-	int allow_rerere_auto;
-
-	int mainline;
-
-	/* 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"
-
 static const char *action_name(const struct replay_opts *opts)
 {
 	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 == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
@@ -222,784 +183,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		usage_with_options(usage_str, options);
 }
 
-struct commit_message {
-	char *parent_label;
-	const char *label;
-	const char *subject;
-	char *reencoded_message;
-	const char *message;
-};
-
-static int get_message(struct commit *commit, struct commit_message *out)
-{
-	const char *encoding;
-	const char *abbrev, *subject;
-	int abbrev_len, subject_len;
-	char *q;
-
-	if (!commit->buffer)
-		return -1;
-	encoding = get_encoding(commit->buffer);
-	if (!encoding)
-		encoding = "UTF-8";
-	if (!git_commit_encoding)
-		git_commit_encoding = "UTF-8";
-
-	out->reencoded_message = NULL;
-	out->message = commit->buffer;
-	if (strcmp(encoding, git_commit_encoding))
-		out->reencoded_message = reencode_string(commit->buffer,
-					git_commit_encoding, encoding);
-	if (out->reencoded_message)
-		out->message = out->reencoded_message;
-
-	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
-	abbrev_len = strlen(abbrev);
-
-	subject_len = find_commit_subject(out->message, &subject);
-
-	out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
-			      strlen("... ") + subject_len + 1);
-	q = out->parent_label;
-	q = mempcpy(q, "parent of ", strlen("parent of "));
-	out->label = q;
-	q = mempcpy(q, abbrev, abbrev_len);
-	q = mempcpy(q, "... ", strlen("... "));
-	out->subject = q;
-	q = mempcpy(q, subject, subject_len);
-	*q = '\0';
-	return 0;
-}
-
-static void free_message(struct commit_message *msg)
-{
-	free(msg->parent_label);
-	free(msg->reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
-	const char *p = message, *eol;
-
-	while (*p && *p != '\n') {
-		for (eol = p + 1; *eol && *eol != '\n'; eol++)
-			; /* do nothing */
-		if (!prefixcmp(p, "encoding ")) {
-			char *result = xmalloc(eol - 8 - p);
-			strlcpy(result, p + 9, eol - 8 - p);
-			return result;
-		}
-		p = eol;
-		if (*p == '\n')
-			p++;
-	}
-	return NULL;
-}
-
-static void write_cherry_pick_head(struct commit *commit)
-{
-	int fd;
-	struct strbuf buf = STRBUF_INIT;
-
-	strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
-
-	fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
-	if (fd < 0)
-		die_errno(_("Could not open '%s' for writing"),
-			  git_path("CHERRY_PICK_HEAD"));
-	if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
-		die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
-	strbuf_release(&buf);
-}
-
-static void print_advice(int show_hint)
-{
-	char *msg = getenv("GIT_CHERRY_PICK_HELP");
-
-	if (msg) {
-		fprintf(stderr, "%s\n", msg);
-		/*
-		 * A conflict has occured but the porcelain
-		 * (typically rebase --interactive) wants to take care
-		 * of the commit itself so remove CHERRY_PICK_HEAD
-		 */
-		unlink(git_path("CHERRY_PICK_HEAD"));
-		return;
-	}
-
-	if (show_hint) {
-		advise("after resolving the conflicts, mark the corrected paths");
-		advise("with 'git add <paths>' or 'git rm <paths>'");
-		advise("and commit the result with 'git commit'");
-	}
-}
-
-static void write_message(struct strbuf *msgbuf, const char *filename)
-{
-	static struct lock_file msg_file;
-
-	int msg_fd = hold_lock_file_for_update(&msg_file, filename,
-					       LOCK_DIE_ON_ERROR);
-	if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
-		die_errno(_("Could not write to %s."), filename);
-	strbuf_release(msgbuf);
-	if (commit_lock_file(&msg_file) < 0)
-		die(_("Error wrapping up %s"), filename);
-}
-
-static struct tree *empty_tree(void)
-{
-	return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
-}
-
-static int error_dirty_index(struct replay_opts *opts)
-{
-	if (read_cache_unmerged())
-		return error_resolve_conflict(action_name(opts));
-
-	/* Different translation strings for cherry-pick and revert */
-	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."));
-
-	if (advice_commit_before_merge)
-		advise(_("Commit your changes or stash them to proceed."));
-	return -1;
-}
-
-static int fast_forward_to(const unsigned char *to, const unsigned char *from)
-{
-	struct ref_lock *ref_lock;
-
-	read_cache();
-	if (checkout_fast_forward(from, to))
-		exit(1); /* the callee should have complained already */
-	ref_lock = lock_any_ref_for_update("HEAD", from, 0);
-	return write_ref_sha1(ref_lock, to, "cherry-pick");
-}
-
-static int do_recursive_merge(struct commit *base, struct commit *next,
-			      const char *base_label, const char *next_label,
-			      unsigned char *head, struct strbuf *msgbuf,
-			      struct replay_opts *opts)
-{
-	struct merge_options o;
-	struct tree *result, *next_tree, *base_tree, *head_tree;
-	int clean, index_fd;
-	const char **xopt;
-	static struct lock_file index_lock;
-
-	index_fd = hold_locked_index(&index_lock, 1);
-
-	read_cache();
-
-	init_merge_options(&o);
-	o.ancestor = base ? base_label : "(empty tree)";
-	o.branch1 = "HEAD";
-	o.branch2 = next ? next_label : "(empty tree)";
-
-	head_tree = parse_tree_indirect(head);
-	next_tree = next ? next->tree : empty_tree();
-	base_tree = base ? base->tree : empty_tree();
-
-	for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
-		parse_merge_opt(&o, *xopt);
-
-	clean = merge_trees(&o,
-			    head_tree,
-			    next_tree, base_tree, &result);
-
-	if (active_cache_changed &&
-	    (write_cache(index_fd, active_cache, active_nr) ||
-	     commit_locked_index(&index_lock)))
-		/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
-		die(_("%s: Unable to write new index file"), action_name(opts));
-	rollback_lock_file(&index_lock);
-
-	if (!clean) {
-		int i;
-		strbuf_addstr(msgbuf, "\nConflicts:\n\n");
-		for (i = 0; i < active_nr;) {
-			struct cache_entry *ce = active_cache[i++];
-			if (ce_stage(ce)) {
-				strbuf_addch(msgbuf, '\t');
-				strbuf_addstr(msgbuf, ce->name);
-				strbuf_addch(msgbuf, '\n');
-				while (i < active_nr && !strcmp(ce->name,
-						active_cache[i]->name))
-					i++;
-			}
-		}
-	}
-
-	return !clean;
-}
-
-/*
- * If we are cherry-pick, and if the merge did not result in
- * hand-editing, we will hit this commit and inherit the original
- * author date and name.
- * If we are revert, or if our cherry-pick results in a hand merge,
- * we had better say that the current user is responsible for that.
- */
-static int run_git_commit(const char *defmsg, struct replay_opts *opts)
-{
-	/* 6 is max possible length of our args array including NULL */
-	const char *args[6];
-	int i = 0;
-
-	args[i++] = "commit";
-	args[i++] = "-n";
-	if (opts->signoff)
-		args[i++] = "-s";
-	if (!opts->edit) {
-		args[i++] = "-F";
-		args[i++] = defmsg;
-	}
-	args[i] = NULL;
-
-	return run_command_v_opt(args, RUN_GIT_CMD);
-}
-
-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;
-	const char *base_label, *next_label;
-	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
-	char *defmsg = NULL;
-	struct strbuf msgbuf = STRBUF_INIT;
-	int res;
-
-	if (opts->no_commit) {
-		/*
-		 * We do not intend to commit immediately.  We just want to
-		 * merge the differences in, so let's compute the tree
-		 * that represents the "current" state for merge-recursive
-		 * to work on.
-		 */
-		if (write_cache_as_tree(head, 0, NULL))
-			die (_("Your index file is unmerged."));
-	} else {
-		if (get_sha1("HEAD", head))
-			return error(_("You do not have a valid HEAD"));
-		if (index_differs_from("HEAD", 0))
-			return error_dirty_index(opts);
-	}
-	discard_cache();
-
-	if (!commit->parents) {
-		parent = NULL;
-	}
-	else if (commit->parents->next) {
-		/* Reverting or cherry-picking a merge commit */
-		int cnt;
-		struct commit_list *p;
-
-		if (!opts->mainline)
-			return error(_("Commit %s is a merge but no -m option was given."),
-				sha1_to_hex(commit->object.sha1));
-
-		for (cnt = 1, p = commit->parents;
-		     cnt != opts->mainline && p;
-		     cnt++)
-			p = p->next;
-		if (cnt != opts->mainline || !p)
-			return error(_("Commit %s does not have parent %d"),
-				sha1_to_hex(commit->object.sha1), opts->mainline);
-		parent = p->item;
-	} else if (0 < opts->mainline)
-		return error(_("Mainline was specified but commit %s is not a merge."),
-			sha1_to_hex(commit->object.sha1));
-	else
-		parent = commit->parents->item;
-
-	if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
-		return fast_forward_to(commit->object.sha1, head);
-
-	if (parent && parse_commit(parent) < 0)
-		/* TRANSLATORS: The first %s will be "revert" or
-		   "cherry-pick", the second %s a SHA1 */
-		return error(_("%s: cannot parse parent commit %s"),
-			action_name(opts), sha1_to_hex(parent->object.sha1));
-
-	if (get_message(commit, &msg) != 0)
-		return error(_("Cannot get commit message for %s"),
-			sha1_to_hex(commit->object.sha1));
-
-	/*
-	 * "commit" is an existing commit.  We would want to apply
-	 * the difference it introduces since its first parent "prev"
-	 * on top of the current HEAD if we are cherry-pick.  Or the
-	 * reverse of it if we are revert.
-	 */
-
-	defmsg = git_pathdup("MERGE_MSG");
-
-	if (action == REPLAY_REVERT) {
-		base = commit;
-		base_label = msg.label;
-		next = parent;
-		next_label = msg.parent_label;
-		strbuf_addstr(&msgbuf, "Revert \"");
-		strbuf_addstr(&msgbuf, msg.subject);
-		strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
-		strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-
-		if (commit->parents && commit->parents->next) {
-			strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
-			strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
-		}
-		strbuf_addstr(&msgbuf, ".\n");
-	} else {
-		const char *p;
-
-		base = parent;
-		base_label = msg.parent_label;
-		next = commit;
-		next_label = msg.label;
-
-		/*
-		 * Append the commit log message to msgbuf; it starts
-		 * after the tree, parent, author, committer
-		 * information followed by "\n\n".
-		 */
-		p = strstr(msg.message, "\n\n");
-		if (p) {
-			p += 2;
-			strbuf_addstr(&msgbuf, p);
-		}
-
-		if (opts->record_origin) {
-			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
-			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-			strbuf_addstr(&msgbuf, ")\n");
-		}
-	}
-
-	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);
-	} else {
-		struct commit_list *common = NULL;
-		struct commit_list *remotes = NULL;
-
-		write_message(&msgbuf, defmsg);
-
-		commit_list_insert(base, &common);
-		commit_list_insert(next, &remotes);
-		res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
-					common, sha1_to_hex(head), remotes);
-		free_commit_list(common);
-		free_commit_list(remotes);
-	}
-
-	/*
-	 * If the merge was clean or if it failed due to conflict, we write
-	 * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
-	 * However, if the merge did not even start, then we don't want to
-	 * write it at all.
-	 */
-	if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
-		write_cherry_pick_head(commit);
-
-	if (res) {
-		error(action == REPLAY_REVERT
-		      ? _("could not revert %s... %s")
-		      : _("could not apply %s... %s"),
-		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
-		      msg.subject);
-		print_advice(res == 1);
-		rerere(opts->allow_rerere_auto);
-	} else {
-		if (!opts->no_commit)
-			res = run_git_commit(defmsg, opts);
-	}
-
-	free_message(&msg);
-	free(defmsg);
-
-	return res;
-}
-
-static void prepare_revs(struct replay_opts *opts)
-{
-	if (opts->action != REPLAY_REVERT)
-		opts->revs->reverse ^= 1;
-
-	if (prepare_revision_walk(opts->revs))
-		die(_("revision walk setup failed"));
-
-	if (!opts->revs->commits)
-		die(_("empty commit set passed"));
-}
-
-static void read_and_refresh_cache(struct replay_opts *opts)
-{
-	static struct lock_file index_lock;
-	int index_fd = hold_locked_index(&index_lock, 0);
-	if (read_index_preload(&the_index, NULL) < 0)
-		die(_("git %s: failed to read the index"), action_name(opts));
-	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
-	if (the_index.cache_changed) {
-		if (write_index(&the_index, index_fd) ||
-		    commit_locked_index(&index_lock))
-			die(_("git %s: failed to refresh the index"), action_name(opts));
-	}
-	rollback_lock_file(&index_lock);
-}
-
-/*
- * Append a commit to the end of the commit_list.
- *
- * next starts by pointing to the variable that holds the head of an
- * empty commit_list, and is updated to point to the "next" field of
- * the last item on the list as new commits are appended.
- *
- * Usage example:
- *
- *     struct commit_list *list;
- *     struct commit_list **next = &list;
- *
- *     next = commit_list_append(c1, next);
- *     next = commit_list_append(c2, next);
- *     assert(commit_list_count(list) == 2);
- *     return list;
- */
-static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
-						struct commit *operand,
-						struct replay_insn_list **next)
-{
-	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 replay_insn_list *todo_list)
-{
-	struct replay_insn_list *cur;
-
-	for (cur = todo_list; cur; cur = cur->next) {
-		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 int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
-{
-	unsigned char commit_sha1[20];
-	char *end_of_object_name;
-	int saved, status;
-
-	if (!prefixcmp(bol, "pick ")) {
-		item->action = REPLAY_PICK;
-		bol += strlen("pick ");
-	} else if (!prefixcmp(bol, "revert ")) {
-		item->action = REPLAY_REVERT;
-		bol += strlen("revert ");
-	} 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;
-	*end_of_object_name = '\0';
-	status = get_sha1(bol, commit_sha1);
-	*end_of_object_name = saved;
-
-	if (status < 0)
-		return error(_("Malformed object name: %s"), bol);
-
-	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 replay_insn_list **todo_list)
-{
-	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');
-		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)
-		return error(_("No commits parsed."));
-	return 0;
-}
-
-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;
-	int fd, res;
-
-	fd = open(todo_file, O_RDONLY);
-	if (fd < 0)
-		die_errno(_("Could not open %s."), todo_file);
-	if (strbuf_read(&buf, fd, 0) < 0) {
-		close(fd);
-		strbuf_release(&buf);
-		die(_("Could not read %s."), todo_file);
-	}
-	close(fd);
-
-	res = parse_insn_buffer(buf.buf, todo_list);
-	strbuf_release(&buf);
-	if (res)
-		die(_("Unusable instruction sheet: %s"), todo_file);
-}
-
-static int populate_opts_cb(const char *key, const char *value, void *data)
-{
-	struct replay_opts *opts = data;
-	int error_flag = 1;
-
-	if (!value)
-		error_flag = 0;
-	else if (!strcmp(key, "options.no-commit"))
-		opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
-	else if (!strcmp(key, "options.edit"))
-		opts->edit = git_config_bool_or_int(key, value, &error_flag);
-	else if (!strcmp(key, "options.signoff"))
-		opts->signoff = git_config_bool_or_int(key, value, &error_flag);
-	else if (!strcmp(key, "options.record-origin"))
-		opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
-	else if (!strcmp(key, "options.allow-ff"))
-		opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
-	else if (!strcmp(key, "options.mainline"))
-		opts->mainline = git_config_int(key, value);
-	else if (!strcmp(key, "options.strategy"))
-		git_config_string(&opts->strategy, key, value);
-	else if (!strcmp(key, "options.strategy-option")) {
-		ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
-		opts->xopts[opts->xopts_nr++] = xstrdup(value);
-	} else
-		return error(_("Invalid key: %s"), key);
-
-	if (!error_flag)
-		return error(_("Invalid value for %s: %s"), key, value);
-
-	return 0;
-}
-
-static void read_populate_opts(struct replay_opts **opts_ptr)
-{
-	const char *opts_file = git_path(SEQ_OPTS_FILE);
-
-	if (!file_exists(opts_file))
-		return;
-	if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
-		die(_("Malformed options sheet: %s"), opts_file);
-}
-
-static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
-				struct replay_opts *opts)
-{
-	struct commit *commit;
-	struct replay_insn_list **next;
-
-	prepare_revs(opts);
-
-	next = todo_list;
-	while ((commit = get_revision(opts->revs)))
-		next = replay_insn_list_append(opts->action, commit, next);
-}
-
-static int create_seq_dir(void)
-{
-	const char *seq_dir = git_path(SEQ_DIR);
-
-	if (file_exists(seq_dir))
-		return error(_("%s already exists."), seq_dir);
-	else if (mkdir(seq_dir, 0777) < 0)
-		die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
-	return 0;
-}
-
-static void save_head(const char *head)
-{
-	const char *head_file = git_path(SEQ_HEAD_FILE);
-	static struct lock_file head_lock;
-	struct strbuf buf = STRBUF_INIT;
-	int fd;
-
-	fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
-	strbuf_addf(&buf, "%s\n", head);
-	if (write_in_full(fd, buf.buf, buf.len) < 0)
-		die_errno(_("Could not write to %s."), head_file);
-	if (commit_lock_file(&head_lock) < 0)
-		die(_("Error wrapping up %s."), head_file);
-}
-
-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;
-	struct strbuf buf = STRBUF_INIT;
-	int fd;
-
-	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
-	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);
-		die_errno(_("Could not write to %s."), todo_file);
-	}
-	if (commit_lock_file(&todo_lock) < 0) {
-		strbuf_release(&buf);
-		die(_("Error wrapping up %s."), todo_file);
-	}
-	strbuf_release(&buf);
-}
-
-static void save_opts(struct replay_opts *opts)
-{
-	const char *opts_file = git_path(SEQ_OPTS_FILE);
-
-	if (opts->no_commit)
-		git_config_set_in_file(opts_file, "options.no-commit", "true");
-	if (opts->edit)
-		git_config_set_in_file(opts_file, "options.edit", "true");
-	if (opts->signoff)
-		git_config_set_in_file(opts_file, "options.signoff", "true");
-	if (opts->record_origin)
-		git_config_set_in_file(opts_file, "options.record-origin", "true");
-	if (opts->allow_ff)
-		git_config_set_in_file(opts_file, "options.allow-ff", "true");
-	if (opts->mainline) {
-		struct strbuf buf = STRBUF_INIT;
-		strbuf_addf(&buf, "%d", opts->mainline);
-		git_config_set_in_file(opts_file, "options.mainline", buf.buf);
-		strbuf_release(&buf);
-	}
-	if (opts->strategy)
-		git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
-	if (opts->xopts) {
-		int i;
-		for (i = 0; i < opts->xopts_nr; i++)
-			git_config_set_multivar_in_file(opts_file,
-							"options.strategy-option",
-							opts->xopts[i], "^$", 0);
-	}
-}
-
-static int pick_commits(struct replay_insn_list *todo_list,
-			struct replay_opts *opts)
-{
-	struct replay_insn_list *cur;
-	int res;
-
-	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
-	if (opts->allow_ff)
-		assert(!(opts->signoff || opts->no_commit ||
-				opts->record_origin || opts->edit));
-	read_and_refresh_cache(opts);
-
-	for (cur = todo_list; cur; cur = cur->next) {
-		save_todo(cur);
-		res = do_pick_commit(cur->operand, cur->action, opts);
-		if (res) {
-			if (!cur->next)
-				/*
-				 * An error was encountered while
-				 * picking the last commit; the
-				 * sequencer state is useless now --
-				 * the user simply needs to resolve
-				 * the conflict and commit
-				 */
-				remove_sequencer_state(0);
-			return res;
-		}
-	}
-
-	/*
-	 * Sequence of picks finished successfully; cleanup by
-	 * removing the .git/sequencer directory
-	 */
-	remove_sequencer_state(1);
-	return 0;
-}
-
-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);
-
-	/*
-	 * Decide what to do depending on the arguments; a fresh
-	 * cherry-pick should be handled differently from an existing
-	 * one that is being continued
-	 */
-	if (opts->subcommand == REPLAY_RESET) {
-		remove_sequencer_state(1);
-		return 0;
-	} else if (opts->subcommand == REPLAY_CONTINUE) {
-		if (!file_exists(git_path(SEQ_TODO_FILE)))
-			goto error;
-		read_populate_opts(&opts);
-		read_populate_todo(&todo_list);
-
-		/* Verify that the conflict has been resolved */
-		if (!index_differs_from("HEAD", 0))
-			todo_list = todo_list->next;
-	} else {
-		/*
-		 * Start a new cherry-pick/ revert sequence; but
-		 * first, make sure that an existing one isn't in
-		 * progress
-		 */
-
-		walk_revs_populate_todo(&todo_list, opts);
-		if (create_seq_dir() < 0) {
-			error(_("A cherry-pick or revert is in progress."));
-			advise(_("Use --continue to continue the operation"));
-			advise(_("or --reset to forget about it"));
-			return -1;
-		}
-		if (get_sha1("HEAD", sha1)) {
-			if (opts->action == REPLAY_REVERT)
-				return error(_("Can't revert as initial commit"));
-			return error(_("Can't cherry-pick into empty head"));
-		}
-		save_head(sha1_to_hex(sha1));
-		save_opts(opts);
-	}
-	return pick_commits(todo_list, opts);
-error:
-	return error(_("No %s in progress"), action_name(opts));
-}
-
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	struct replay_opts opts;
@@ -1011,7 +194,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	opts.action = REPLAY_REVERT;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
-	res = pick_revisions(&opts);
+	res = sequencer_pick_revisions(&opts);
 	if (res < 0)
 		die(_("revert failed"));
 	return res;
@@ -1026,7 +209,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	opts.action = REPLAY_PICK;
 	git_config(git_default_config, NULL);
 	parse_args(argc, argv, &opts);
-	res = pick_revisions(&opts);
+	res = sequencer_pick_revisions(&opts);
 	if (res < 0)
 		die(_("cherry-pick failed"));
 	return res;
diff --git a/sequencer.c b/sequencer.c
index bc2c046..87f146b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,7 +1,27 @@
 #include "cache.h"
-#include "sequencer.h"
-#include "strbuf.h"
+#include "object.h"
+#include "commit.h"
+#include "tag.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "utf8.h"
+#include "cache-tree.h"
+#include "diff.h"
+#include "revision.h"
+#include "rerere.h"
+#include "merge-recursive.h"
+#include "refs.h"
 #include "dir.h"
+#include "sequencer.h"
+
+#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+
+static const char *action_name(const struct replay_opts *opts)
+{
+	return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+}
+
+static char *get_encoding(const char *message);
 
 void remove_sequencer_state(int aggressive)
 {
@@ -17,3 +37,781 @@ void remove_sequencer_state(int aggressive)
 	strbuf_release(&seq_dir);
 	strbuf_release(&seq_old_dir);
 }
+
+struct commit_message {
+	char *parent_label;
+	const char *label;
+	const char *subject;
+	char *reencoded_message;
+	const char *message;
+};
+
+static int get_message(struct commit *commit, struct commit_message *out)
+{
+	const char *encoding;
+	const char *abbrev, *subject;
+	int abbrev_len, subject_len;
+	char *q;
+
+	if (!commit->buffer)
+		return -1;
+	encoding = get_encoding(commit->buffer);
+	if (!encoding)
+		encoding = "UTF-8";
+	if (!git_commit_encoding)
+		git_commit_encoding = "UTF-8";
+
+	out->reencoded_message = NULL;
+	out->message = commit->buffer;
+	if (strcmp(encoding, git_commit_encoding))
+		out->reencoded_message = reencode_string(commit->buffer,
+					git_commit_encoding, encoding);
+	if (out->reencoded_message)
+		out->message = out->reencoded_message;
+
+	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+	abbrev_len = strlen(abbrev);
+
+	subject_len = find_commit_subject(out->message, &subject);
+
+	out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
+			      strlen("... ") + subject_len + 1);
+	q = out->parent_label;
+	q = mempcpy(q, "parent of ", strlen("parent of "));
+	out->label = q;
+	q = mempcpy(q, abbrev, abbrev_len);
+	q = mempcpy(q, "... ", strlen("... "));
+	out->subject = q;
+	q = mempcpy(q, subject, subject_len);
+	*q = '\0';
+	return 0;
+}
+
+static void free_message(struct commit_message *msg)
+{
+	free(msg->parent_label);
+	free(msg->reencoded_message);
+}
+
+static char *get_encoding(const char *message)
+{
+	const char *p = message, *eol;
+
+	while (*p && *p != '\n') {
+		for (eol = p + 1; *eol && *eol != '\n'; eol++)
+			; /* do nothing */
+		if (!prefixcmp(p, "encoding ")) {
+			char *result = xmalloc(eol - 8 - p);
+			strlcpy(result, p + 9, eol - 8 - p);
+			return result;
+		}
+		p = eol;
+		if (*p == '\n')
+			p++;
+	}
+	return NULL;
+}
+
+static void write_cherry_pick_head(struct commit *commit)
+{
+	int fd;
+	struct strbuf buf = STRBUF_INIT;
+
+	strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
+
+	fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
+	if (fd < 0)
+		die_errno(_("Could not open '%s' for writing"),
+			  git_path("CHERRY_PICK_HEAD"));
+	if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
+		die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
+	strbuf_release(&buf);
+}
+
+static void print_advice(int show_hint)
+{
+	char *msg = getenv("GIT_CHERRY_PICK_HELP");
+
+	if (msg) {
+		fprintf(stderr, "%s\n", msg);
+		/*
+		 * A conflict has occured but the porcelain
+		 * (typically rebase --interactive) wants to take care
+		 * of the commit itself so remove CHERRY_PICK_HEAD
+		 */
+		unlink(git_path("CHERRY_PICK_HEAD"));
+		return;
+	}
+
+	if (show_hint) {
+		advise("after resolving the conflicts, mark the corrected paths");
+		advise("with 'git add <paths>' or 'git rm <paths>'");
+		advise("and commit the result with 'git commit'");
+	}
+}
+
+static void write_message(struct strbuf *msgbuf, const char *filename)
+{
+	static struct lock_file msg_file;
+
+	int msg_fd = hold_lock_file_for_update(&msg_file, filename,
+					       LOCK_DIE_ON_ERROR);
+	if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
+		die_errno(_("Could not write to %s."), filename);
+	strbuf_release(msgbuf);
+	if (commit_lock_file(&msg_file) < 0)
+		die(_("Error wrapping up %s"), filename);
+}
+
+static struct tree *empty_tree(void)
+{
+	return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
+}
+
+static int error_dirty_index(struct replay_opts *opts)
+{
+	if (read_cache_unmerged())
+		return error_resolve_conflict(action_name(opts));
+
+	/* Different translation strings for cherry-pick and revert */
+	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."));
+
+	if (advice_commit_before_merge)
+		advise(_("Commit your changes or stash them to proceed."));
+	return -1;
+}
+
+static int fast_forward_to(const unsigned char *to, const unsigned char *from)
+{
+	struct ref_lock *ref_lock;
+
+	read_cache();
+	if (checkout_fast_forward(from, to))
+		exit(1); /* the callee should have complained already */
+	ref_lock = lock_any_ref_for_update("HEAD", from, 0);
+	return write_ref_sha1(ref_lock, to, "cherry-pick");
+}
+
+static int do_recursive_merge(struct commit *base, struct commit *next,
+			      const char *base_label, const char *next_label,
+			      unsigned char *head, struct strbuf *msgbuf,
+			      struct replay_opts *opts)
+{
+	struct merge_options o;
+	struct tree *result, *next_tree, *base_tree, *head_tree;
+	int clean, index_fd;
+	const char **xopt;
+	static struct lock_file index_lock;
+
+	index_fd = hold_locked_index(&index_lock, 1);
+
+	read_cache();
+
+	init_merge_options(&o);
+	o.ancestor = base ? base_label : "(empty tree)";
+	o.branch1 = "HEAD";
+	o.branch2 = next ? next_label : "(empty tree)";
+
+	head_tree = parse_tree_indirect(head);
+	next_tree = next ? next->tree : empty_tree();
+	base_tree = base ? base->tree : empty_tree();
+
+	for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
+		parse_merge_opt(&o, *xopt);
+
+	clean = merge_trees(&o,
+			    head_tree,
+			    next_tree, base_tree, &result);
+
+	if (active_cache_changed &&
+	    (write_cache(index_fd, active_cache, active_nr) ||
+	     commit_locked_index(&index_lock)))
+		/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
+		die(_("%s: Unable to write new index file"), action_name(opts));
+	rollback_lock_file(&index_lock);
+
+	if (!clean) {
+		int i;
+		strbuf_addstr(msgbuf, "\nConflicts:\n\n");
+		for (i = 0; i < active_nr;) {
+			struct cache_entry *ce = active_cache[i++];
+			if (ce_stage(ce)) {
+				strbuf_addch(msgbuf, '\t');
+				strbuf_addstr(msgbuf, ce->name);
+				strbuf_addch(msgbuf, '\n');
+				while (i < active_nr && !strcmp(ce->name,
+						active_cache[i]->name))
+					i++;
+			}
+		}
+	}
+
+	return !clean;
+}
+
+/*
+ * If we are cherry-pick, and if the merge did not result in
+ * hand-editing, we will hit this commit and inherit the original
+ * author date and name.
+ * If we are revert, or if our cherry-pick results in a hand merge,
+ * we had better say that the current user is responsible for that.
+ */
+static int run_git_commit(const char *defmsg, struct replay_opts *opts)
+{
+	/* 6 is max possible length of our args array including NULL */
+	const char *args[6];
+	int i = 0;
+
+	args[i++] = "commit";
+	args[i++] = "-n";
+	if (opts->signoff)
+		args[i++] = "-s";
+	if (!opts->edit) {
+		args[i++] = "-F";
+		args[i++] = defmsg;
+	}
+	args[i] = NULL;
+
+	return run_command_v_opt(args, RUN_GIT_CMD);
+}
+
+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;
+	const char *base_label, *next_label;
+	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+	char *defmsg = NULL;
+	struct strbuf msgbuf = STRBUF_INIT;
+	int res;
+
+	if (opts->no_commit) {
+		/*
+		 * We do not intend to commit immediately.  We just want to
+		 * merge the differences in, so let's compute the tree
+		 * that represents the "current" state for merge-recursive
+		 * to work on.
+		 */
+		if (write_cache_as_tree(head, 0, NULL))
+			die (_("Your index file is unmerged."));
+	} else {
+		if (get_sha1("HEAD", head))
+			return error(_("You do not have a valid HEAD"));
+		if (index_differs_from("HEAD", 0))
+			return error_dirty_index(opts);
+	}
+	discard_cache();
+
+	if (!commit->parents) {
+		parent = NULL;
+	}
+	else if (commit->parents->next) {
+		/* Reverting or cherry-picking a merge commit */
+		int cnt;
+		struct commit_list *p;
+
+		if (!opts->mainline)
+			return error(_("Commit %s is a merge but no -m option was given."),
+				sha1_to_hex(commit->object.sha1));
+
+		for (cnt = 1, p = commit->parents;
+		     cnt != opts->mainline && p;
+		     cnt++)
+			p = p->next;
+		if (cnt != opts->mainline || !p)
+			return error(_("Commit %s does not have parent %d"),
+				sha1_to_hex(commit->object.sha1), opts->mainline);
+		parent = p->item;
+	} else if (0 < opts->mainline)
+		return error(_("Mainline was specified but commit %s is not a merge."),
+			sha1_to_hex(commit->object.sha1));
+	else
+		parent = commit->parents->item;
+
+	if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
+		return fast_forward_to(commit->object.sha1, head);
+
+	if (parent && parse_commit(parent) < 0)
+		/* TRANSLATORS: The first %s will be "revert" or
+		   "cherry-pick", the second %s a SHA1 */
+		return error(_("%s: cannot parse parent commit %s"),
+			action_name(opts), sha1_to_hex(parent->object.sha1));
+
+	if (get_message(commit, &msg) != 0)
+		return error(_("Cannot get commit message for %s"),
+			sha1_to_hex(commit->object.sha1));
+
+	/*
+	 * "commit" is an existing commit.  We would want to apply
+	 * the difference it introduces since its first parent "prev"
+	 * on top of the current HEAD if we are cherry-pick.  Or the
+	 * reverse of it if we are revert.
+	 */
+
+	defmsg = git_pathdup("MERGE_MSG");
+
+	if (action == REPLAY_REVERT) {
+		base = commit;
+		base_label = msg.label;
+		next = parent;
+		next_label = msg.parent_label;
+		strbuf_addstr(&msgbuf, "Revert \"");
+		strbuf_addstr(&msgbuf, msg.subject);
+		strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
+		strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+
+		if (commit->parents && commit->parents->next) {
+			strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
+			strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
+		}
+		strbuf_addstr(&msgbuf, ".\n");
+	} else {
+		const char *p;
+
+		base = parent;
+		base_label = msg.parent_label;
+		next = commit;
+		next_label = msg.label;
+
+		/*
+		 * Append the commit log message to msgbuf; it starts
+		 * after the tree, parent, author, committer
+		 * information followed by "\n\n".
+		 */
+		p = strstr(msg.message, "\n\n");
+		if (p) {
+			p += 2;
+			strbuf_addstr(&msgbuf, p);
+		}
+
+		if (opts->record_origin) {
+			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+			strbuf_addstr(&msgbuf, ")\n");
+		}
+	}
+
+	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);
+	} else {
+		struct commit_list *common = NULL;
+		struct commit_list *remotes = NULL;
+
+		write_message(&msgbuf, defmsg);
+
+		commit_list_insert(base, &common);
+		commit_list_insert(next, &remotes);
+		res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+					common, sha1_to_hex(head), remotes);
+		free_commit_list(common);
+		free_commit_list(remotes);
+	}
+
+	/*
+	 * If the merge was clean or if it failed due to conflict, we write
+	 * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
+	 * However, if the merge did not even start, then we don't want to
+	 * write it at all.
+	 */
+	if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
+		write_cherry_pick_head(commit);
+
+	if (res) {
+		error(action == REPLAY_REVERT
+		      ? _("could not revert %s... %s")
+		      : _("could not apply %s... %s"),
+		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+		      msg.subject);
+		print_advice(res == 1);
+		rerere(opts->allow_rerere_auto);
+	} else {
+		if (!opts->no_commit)
+			res = run_git_commit(defmsg, opts);
+	}
+
+	free_message(&msg);
+	free(defmsg);
+
+	return res;
+}
+
+static void prepare_revs(struct replay_opts *opts)
+{
+	if (opts->action != REPLAY_REVERT)
+		opts->revs->reverse ^= 1;
+
+	if (prepare_revision_walk(opts->revs))
+		die(_("revision walk setup failed"));
+
+	if (!opts->revs->commits)
+		die(_("empty commit set passed"));
+}
+
+static void read_and_refresh_cache(struct replay_opts *opts)
+{
+	static struct lock_file index_lock;
+	int index_fd = hold_locked_index(&index_lock, 0);
+	if (read_index_preload(&the_index, NULL) < 0)
+		die(_("git %s: failed to read the index"), action_name(opts));
+	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
+	if (the_index.cache_changed) {
+		if (write_index(&the_index, index_fd) ||
+		    commit_locked_index(&index_lock))
+			die(_("git %s: failed to refresh the index"), action_name(opts));
+	}
+	rollback_lock_file(&index_lock);
+}
+
+/*
+ * Append a commit to the end of the commit_list.
+ *
+ * next starts by pointing to the variable that holds the head of an
+ * empty commit_list, and is updated to point to the "next" field of
+ * the last item on the list as new commits are appended.
+ *
+ * Usage example:
+ *
+ *     struct commit_list *list;
+ *     struct commit_list **next = &list;
+ *
+ *     next = commit_list_append(c1, next);
+ *     next = commit_list_append(c2, next);
+ *     assert(commit_list_count(list) == 2);
+ *     return list;
+ */
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+						struct commit *operand,
+						struct replay_insn_list **next)
+{
+	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 replay_insn_list *todo_list)
+{
+	struct replay_insn_list *cur;
+
+	for (cur = todo_list; cur; cur = cur->next) {
+		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 int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+{
+	unsigned char commit_sha1[20];
+	char *end_of_object_name;
+	int saved, status;
+
+	if (!prefixcmp(bol, "pick ")) {
+		item->action = REPLAY_PICK;
+		bol += strlen("pick ");
+	} else if (!prefixcmp(bol, "revert ")) {
+		item->action = REPLAY_REVERT;
+		bol += strlen("revert ");
+	} 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;
+	*end_of_object_name = '\0';
+	status = get_sha1(bol, commit_sha1);
+	*end_of_object_name = saved;
+
+	if (status < 0)
+		return error(_("Malformed object name: %s"), bol);
+
+	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 replay_insn_list **todo_list)
+{
+	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');
+		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)
+		return error(_("No commits parsed."));
+	return 0;
+}
+
+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;
+	int fd, res;
+
+	fd = open(todo_file, O_RDONLY);
+	if (fd < 0)
+		die_errno(_("Could not open %s."), todo_file);
+	if (strbuf_read(&buf, fd, 0) < 0) {
+		close(fd);
+		strbuf_release(&buf);
+		die(_("Could not read %s."), todo_file);
+	}
+	close(fd);
+
+	res = parse_insn_buffer(buf.buf, todo_list);
+	strbuf_release(&buf);
+	if (res)
+		die(_("Unusable instruction sheet: %s"), todo_file);
+}
+
+static int populate_opts_cb(const char *key, const char *value, void *data)
+{
+	struct replay_opts *opts = data;
+	int error_flag = 1;
+
+	if (!value)
+		error_flag = 0;
+	else if (!strcmp(key, "options.no-commit"))
+		opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
+	else if (!strcmp(key, "options.edit"))
+		opts->edit = git_config_bool_or_int(key, value, &error_flag);
+	else if (!strcmp(key, "options.signoff"))
+		opts->signoff = git_config_bool_or_int(key, value, &error_flag);
+	else if (!strcmp(key, "options.record-origin"))
+		opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
+	else if (!strcmp(key, "options.allow-ff"))
+		opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
+	else if (!strcmp(key, "options.mainline"))
+		opts->mainline = git_config_int(key, value);
+	else if (!strcmp(key, "options.strategy"))
+		git_config_string(&opts->strategy, key, value);
+	else if (!strcmp(key, "options.strategy-option")) {
+		ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
+		opts->xopts[opts->xopts_nr++] = xstrdup(value);
+	} else
+		return error(_("Invalid key: %s"), key);
+
+	if (!error_flag)
+		return error(_("Invalid value for %s: %s"), key, value);
+
+	return 0;
+}
+
+static void read_populate_opts(struct replay_opts **opts_ptr)
+{
+	const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+	if (!file_exists(opts_file))
+		return;
+	if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
+		die(_("Malformed options sheet: %s"), opts_file);
+}
+
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
+				struct replay_opts *opts)
+{
+	struct commit *commit;
+	struct replay_insn_list **next;
+
+	prepare_revs(opts);
+
+	next = todo_list;
+	while ((commit = get_revision(opts->revs)))
+		next = replay_insn_list_append(opts->action, commit, next);
+}
+
+static int create_seq_dir(void)
+{
+	const char *seq_dir = git_path(SEQ_DIR);
+
+	if (file_exists(seq_dir))
+		return error(_("%s already exists."), seq_dir);
+	else if (mkdir(seq_dir, 0777) < 0)
+		die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+	return 0;
+}
+
+static void save_head(const char *head)
+{
+	const char *head_file = git_path(SEQ_HEAD_FILE);
+	static struct lock_file head_lock;
+	struct strbuf buf = STRBUF_INIT;
+	int fd;
+
+	fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
+	strbuf_addf(&buf, "%s\n", head);
+	if (write_in_full(fd, buf.buf, buf.len) < 0)
+		die_errno(_("Could not write to %s."), head_file);
+	if (commit_lock_file(&head_lock) < 0)
+		die(_("Error wrapping up %s."), head_file);
+}
+
+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;
+	struct strbuf buf = STRBUF_INIT;
+	int fd;
+
+	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
+	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);
+		die_errno(_("Could not write to %s."), todo_file);
+	}
+	if (commit_lock_file(&todo_lock) < 0) {
+		strbuf_release(&buf);
+		die(_("Error wrapping up %s."), todo_file);
+	}
+	strbuf_release(&buf);
+}
+
+static void save_opts(struct replay_opts *opts)
+{
+	const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+	if (opts->no_commit)
+		git_config_set_in_file(opts_file, "options.no-commit", "true");
+	if (opts->edit)
+		git_config_set_in_file(opts_file, "options.edit", "true");
+	if (opts->signoff)
+		git_config_set_in_file(opts_file, "options.signoff", "true");
+	if (opts->record_origin)
+		git_config_set_in_file(opts_file, "options.record-origin", "true");
+	if (opts->allow_ff)
+		git_config_set_in_file(opts_file, "options.allow-ff", "true");
+	if (opts->mainline) {
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_addf(&buf, "%d", opts->mainline);
+		git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+		strbuf_release(&buf);
+	}
+	if (opts->strategy)
+		git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
+	if (opts->xopts) {
+		int i;
+		for (i = 0; i < opts->xopts_nr; i++)
+			git_config_set_multivar_in_file(opts_file,
+							"options.strategy-option",
+							opts->xopts[i], "^$", 0);
+	}
+}
+
+static int pick_commits(struct replay_insn_list *todo_list,
+			struct replay_opts *opts)
+{
+	struct replay_insn_list *cur;
+	int res;
+
+	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+	if (opts->allow_ff)
+		assert(!(opts->signoff || opts->no_commit ||
+				opts->record_origin || opts->edit));
+	read_and_refresh_cache(opts);
+
+	for (cur = todo_list; cur; cur = cur->next) {
+		save_todo(cur);
+		res = do_pick_commit(cur->operand, cur->action, opts);
+		if (res) {
+			if (!cur->next)
+				/*
+				 * An error was encountered while
+				 * picking the last commit; the
+				 * sequencer state is useless now --
+				 * the user simply needs to resolve
+				 * the conflict and commit
+				 */
+				remove_sequencer_state(0);
+			return res;
+		}
+	}
+
+	/*
+	 * Sequence of picks finished successfully; cleanup by
+	 * removing the .git/sequencer directory
+	 */
+	remove_sequencer_state(1);
+	return 0;
+}
+
+int sequencer_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);
+
+	/*
+	 * Decide what to do depending on the arguments; a fresh
+	 * cherry-pick should be handled differently from an existing
+	 * one that is being continued
+	 */
+	if (opts->subcommand == REPLAY_RESET) {
+		remove_sequencer_state(1);
+		return 0;
+	} else if (opts->subcommand == REPLAY_CONTINUE) {
+		if (!file_exists(git_path(SEQ_TODO_FILE)))
+			goto error;
+		read_populate_opts(&opts);
+		read_populate_todo(&todo_list);
+
+		/* Verify that the conflict has been resolved */
+		if (!index_differs_from("HEAD", 0))
+			todo_list = todo_list->next;
+	} else {
+		/*
+		 * Start a new cherry-pick/ revert sequence; but
+		 * first, make sure that an existing one isn't in
+		 * progress
+		 */
+
+		walk_revs_populate_todo(&todo_list, opts);
+		if (create_seq_dir() < 0) {
+			error(_("A cherry-pick or revert is in progress."));
+			advise(_("Use --continue to continue the operation"));
+			advise(_("or --reset to forget about it"));
+			return -1;
+		}
+		if (get_sha1("HEAD", sha1)) {
+			if (opts->action == REPLAY_REVERT)
+				return error(_("Can't revert as initial commit"));
+			return error(_("Can't cherry-pick into empty head"));
+		}
+		save_head(sha1_to_hex(sha1));
+		save_opts(opts);
+	}
+	return pick_commits(todo_list, opts);
+error:
+	return error(_("No %s in progress"), action_name(opts));
+}
diff --git a/sequencer.h b/sequencer.h
index f4db257..92b2d63 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -8,6 +8,30 @@
 #define SEQ_OPTS_FILE	"sequencer/opts"
 
 enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
+
+struct replay_opts {
+	enum replay_action action;
+	enum replay_subcommand subcommand;
+
+	/* Boolean options */
+	int edit;
+	int record_origin;
+	int no_commit;
+	int signoff;
+	int allow_ff;
+	int allow_rerere_auto;
+
+	int mainline;
+
+	/* Merge strategy */
+	const char *strategy;
+	const char **xopts;
+	size_t xopts_nr, xopts_alloc;
+
+	/* Only used by REPLAY_NONE */
+	struct rev_info *revs;
+};
 
 struct replay_insn_list {
 	enum replay_action action;
@@ -25,4 +49,6 @@ struct replay_insn_list {
  */
 void remove_sequencer_state(int aggressive);
 
+int sequencer_pick_revisions(struct replay_opts *opts);
+
 #endif
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [PATCH 1/2] pretty.c: free get_header() return value
From: Nguyễn Thái Ngọc Duy @ 2011-10-23 11:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 pretty.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/pretty.c b/pretty.c
index f45eb54..375ff7b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1094,7 +1094,6 @@ void format_commit_message(const struct commit *commit,
 {
 	struct format_commit_context context;
 	static const char utf8[] = "UTF-8";
-	const char *enc;
 	const char *output_enc = pretty_ctx->output_encoding;
 
 	memset(&context, 0, sizeof(context));
@@ -1103,10 +1102,10 @@ void format_commit_message(const struct commit *commit,
 	context.wrap_start = sb->len;
 	context.message = commit->buffer;
 	if (output_enc) {
-		enc = get_header(commit, "encoding");
-		enc = enc ? enc : utf8;
-		if (strcmp(enc, output_enc))
+		char *enc = get_header(commit, "encoding");
+		if (strcmp(enc ? enc : utf8, output_enc))
 			context.message = logmsg_reencode(commit, output_enc);
+		free(enc);
 	}
 
 	strbuf_expand(sb, format, format_commit_item, &context);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 2/2] pretty.c: use original commit message if reencoding fails
From: Nguyễn Thái Ngọc Duy @ 2011-10-23 11:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1319370695-12638-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 pretty.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/pretty.c b/pretty.c
index 375ff7b..230fe1c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1103,8 +1103,11 @@ void format_commit_message(const struct commit *commit,
 	context.message = commit->buffer;
 	if (output_enc) {
 		char *enc = get_header(commit, "encoding");
-		if (strcmp(enc ? enc : utf8, output_enc))
+		if (strcmp(enc ? enc : utf8, output_enc)) {
 			context.message = logmsg_reencode(commit, output_enc);
+			if (!context.message)
+				context.message = commit->buffer;
+		}
 		free(enc);
 	}
 
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: a bug when execute "git status" in git version 1.7.7.431.g89633
From: René Scharfe @ 2011-10-23 13:25 UTC (permalink / raw)
  To: John Hsing; +Cc: Matthieu Moy, git, Jeff King
In-Reply-To: <4EA3D1BB.2010802@gmail.com>

Am 23.10.2011 10:35, schrieb John Hsing:
> ok,when i finish compiling git 1.7.7.431.g89633,I use it to check
> Cyanogenod(an Android mod source) by “git status”,it happend this
> error!but when i reuse git v1.7.7,it is OK!so i think it is a bug in
> git 1.7.7.431.g89633!My OS is Ubuntu Linux 10.10,sorry for my bad
> english! If you want to reproduce this error,please excute "git status"
> in https://github.com/CyanogenMod/android_packages_apps_DSPManager.git
> repo!

I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:

	$ a=android_packages_apps_DSPManager
	$ git-v1.7.7 clone https://github.com/CyanogenMod/$a.git
	Cloning into android_packages_apps_DSPManager...
	remote: Counting objects: 902, done.
	remote: Compressing objects: 100% (412/412), done.
	remote: Total 902 (delta 367), reused 838 (delta 324)
	Receiving objects: 100% (902/902), 136.78 KiB | 264 KiB/s, done.
	Resolving deltas: 100% (367/367), done.
	$ cd $a

	$ git-v1.7.7 status
	# On branch gingerbread
	nothing to commit (working directory clean)

	$ git-master status
	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.
	Aborted

Bisect points to 2548183ba, "fix phantom untracked files when
core.ignorecase is set" from Jeff (cc:d).  If I revert that patch from
master (8963314c), git status works fine.

The following experimental patch fixes it for me as well, but I can't
claim to know exactly why.  In any case, estimate_cache_size() seems
to guess too low.

René


---
 read-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 01a0e25..b143bd3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1257,7 +1257,7 @@ static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entrie
 	 * Alignment can cause differences. This should be "alignof", but
 	 * since that's a gcc'ism, just use the size of a pointer.
 	 */
-	per_entry += sizeof(void *);
+	per_entry += 2 * sizeof(void *);
 	return ondisk_size + entries*per_entry;
 }
 

^ permalink raw reply related

* Re: a bug when execute "git status" in git version 1.7.7.431.g89633
From: René Scharfe @ 2011-10-23 14:28 UTC (permalink / raw)
  Cc: John Hsing, Matthieu Moy, git, Jeff King
In-Reply-To: <4EA415BD.1040109@lsrfire.ath.cx>

Am 23.10.2011 15:25, schrieb René Scharfe:
> Am 23.10.2011 10:35, schrieb John Hsing:
>> ok,when i finish compiling git 1.7.7.431.g89633,I use it to check
>> Cyanogenod(an Android mod source) by “git status”,it happend this
>> error!but when i reuse git v1.7.7,it is OK!so i think it is a bug in
>> git 1.7.7.431.g89633!My OS is Ubuntu Linux 10.10,sorry for my bad
>> english! If you want to reproduce this error,please excute "git status"
>> in https://github.com/CyanogenMod/android_packages_apps_DSPManager.git
>> repo!
> 
> I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:
> 
> 	$ a=android_packages_apps_DSPManager
> 	$ git-v1.7.7 clone https://github.com/CyanogenMod/$a.git
> 	Cloning into android_packages_apps_DSPManager...
> 	remote: Counting objects: 902, done.
> 	remote: Compressing objects: 100% (412/412), done.
> 	remote: Total 902 (delta 367), reused 838 (delta 324)
> 	Receiving objects: 100% (902/902), 136.78 KiB | 264 KiB/s, done.
> 	Resolving deltas: 100% (367/367), done.
> 	$ cd $a
> 
> 	$ git-v1.7.7 status
> 	# On branch gingerbread
> 	nothing to commit (working directory clean)
> 
> 	$ git-master status
> 	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.
> 	Aborted

And valgrind reports the following errors (git was compiled with -O0 and
-g) for master, but not for 1.7.7 nor master plus my ugly patch:

Invalid write of size 1
   at 0x4029DE5: memcpy (mc_replace_strmem.c:635)
   by 0x81159A5: convert_from_disk (read-cache.c:1247)
   by 0x8115BE4: read_index_from (read-cache.c:1326)
   by 0x81157D7: read_index (read-cache.c:1202)
   by 0x813A802: gitmodules_config (submodule.c:105)
   by 0x806857E: cmd_status (commit.c:1209)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)
 Address 0x41f2d5e is 2 bytes after a block of size 6,356 alloc'd
   at 0x4028876: malloc (vg_replace_malloc.c:236)
   by 0x814C070: xmalloc (wrapper.c:35)
   by 0x8115B8E: read_index_from (read-cache.c:1315)
   by 0x81157D7: read_index (read-cache.c:1202)
   by 0x813A802: gitmodules_config (submodule.c:105)
   by 0x806857E: cmd_status (commit.c:1209)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)

Syscall param lstat64(file_name) points to unaddressable byte(s)
   at 0x4131D32: __lxstat64@@GLIBC_2.2 (lxstat64.c:48)
   by 0x81154DE: refresh_index (read-cache.c:1133)
   by 0x8068687: cmd_status (commit.c:1226)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)
 Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd
   at 0x4028876: malloc (vg_replace_malloc.c:236)
   by 0x814C070: xmalloc (wrapper.c:35)
   by 0x8115B8E: read_index_from (read-cache.c:1315)
   by 0x81157D7: read_index (read-cache.c:1202)
   by 0x813A802: gitmodules_config (submodule.c:105)
   by 0x806857E: cmd_status (commit.c:1209)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)

Invalid read of size 1
   at 0x402A682: bcmp (mc_replace_strmem.c:679)
   by 0x8113D12: df_name_compare (read-cache.c:387)
   by 0x81478F9: do_compare_entry (unpack-trees.c:499)
   by 0x814791B: compare_entry (unpack-trees.c:504)
   by 0x8148086: unpack_callback (unpack-trees.c:747)
   by 0x8145EA5: traverse_trees (tree-walk.c:407)
   by 0x81477EB: traverse_trees_recursive (unpack-trees.c:460)
   by 0x814823D: unpack_callback (unpack-trees.c:809)
   by 0x8145EA5: traverse_trees (tree-walk.c:407)
   by 0x81477EB: traverse_trees_recursive (unpack-trees.c:460)
   by 0x814823D: unpack_callback (unpack-trees.c:809)
   by 0x8145EA5: traverse_trees (tree-walk.c:407)
 Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd
   at 0x4028876: malloc (vg_replace_malloc.c:236)
   by 0x814C070: xmalloc (wrapper.c:35)
   by 0x8115B8E: read_index_from (read-cache.c:1315)
   by 0x81157D7: read_index (read-cache.c:1202)
   by 0x813A802: gitmodules_config (submodule.c:105)
   by 0x806857E: cmd_status (commit.c:1209)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)

Invalid read of size 1
   at 0x8100E0E: hash_name (name-hash.c:28)
   by 0x8100F3D: hash_index_entry (name-hash.c:78)
   by 0x8100FD3: lazy_init_name_hash (name-hash.c:96)
   by 0x8101197: index_name_exists (name-hash.c:159)
   by 0x80EADA9: dir_add_name (dir.c:596)
   by 0x80EB8CD: read_directory_recursive (dir.c:994)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EBCED: read_directory (dir.c:1101)
 Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd
   at 0x4028876: malloc (vg_replace_malloc.c:236)
   by 0x814C070: xmalloc (wrapper.c:35)
   by 0x8115B8E: read_index_from (read-cache.c:1315)
   by 0x81157D7: read_index (read-cache.c:1202)
   by 0x813A802: gitmodules_config (submodule.c:105)
   by 0x806857E: cmd_status (commit.c:1209)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)

Invalid read of size 1
   at 0x402A687: bcmp (mc_replace_strmem.c:679)
   by 0x8113DF4: cache_name_compare (read-cache.c:413)
   by 0x81010F8: same_name (name-hash.c:134)
   by 0x81011E0: index_name_exists (name-hash.c:164)
   by 0x80EADA9: dir_add_name (dir.c:596)
   by 0x80EB8CD: read_directory_recursive (dir.c:994)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
   by 0x80EB895: read_directory_recursive (dir.c:983)
 Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd
   at 0x4028876: malloc (vg_replace_malloc.c:236)
   by 0x814C070: xmalloc (wrapper.c:35)
   by 0x8115B8E: read_index_from (read-cache.c:1315)
   by 0x81157D7: read_index (read-cache.c:1202)
   by 0x813A802: gitmodules_config (submodule.c:105)
   by 0x806857E: cmd_status (commit.c:1209)
   by 0x804B7F8: run_builtin (git.c:308)
   by 0x804B956: handle_internal_command (git.c:466)
   by 0x804BA4E: run_argv (git.c:512)
   by 0x804BBC0: main (git.c:585)

^ permalink raw reply

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Jeff King @ 2011-10-23 16:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git, Ævar Arnfjörð
In-Reply-To: <7vehy459bg.fsf@alter.siamese.dyndns.org>

On Sun, Oct 23, 2011 at 02:46:59AM -0700, Junio C Hamano wrote:

> > But when it comes to "Git" Porcelains (e.g. the log family of commands),
> > we do assume people do not store random binary byte sequences in commits,
> > and we do take advantage of that assumption by splitting each "line" at
> > LF, indenting them with 4 spaces, etc. In other words, a commit log in the
> > Git context _is_ pretty much text and not arbitrary byte sequence.
> 
> Think what would cutting at a byte whose value is 012 and adding four
> bytes whose values are 040 to each of "lines" that formed with such
> cutting do to UTF-16 goo, even if it does not contain any NUL byte. As far
> as Git Porcelains are concerned, it is no different from random binary
> byte sequences.

But as Duy mentions, we have an encoding header. Shouldn't we treat it
like binary goo until we do reencode_log_message, and _then_ we can
break it into lines?

-Peff

^ permalink raw reply

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

On Sun, Oct 23, 2011 at 12:44:30PM +0200, Robin Rosenberg wrote:

> Jeff King skrev 2011-10-22 21.09:
> >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?
> Yes please. I don't think allowing NUL makes sense, but it makes sense
> to state how NUL should be handled when anyone attempt it, so there
> might be things to fix even if NUL is banned.
> 
> Are there any such commits in the wild?

Adding an arbitrary NUL, no, I don't think I've ever seen it outside of
people (myself included) trying to break git in interesting ways.

But utf16 may contains NUL bytes, so I expect configuring your editor to
output utf16 and running "git commit" would give you the most likely
example.

-Peff

^ permalink raw reply

* git gui: adding bare remotes on local filesystem reports error
From: Peter Oberndorfer @ 2011-10-23 16:14 UTC (permalink / raw)
  To: git; +Cc: Giuseppe Bilotta, Shawn O. Pearce

Hi,

i just tried using git gui to add a remote on the local file system
and push to it.
(set Location to /tmp/blah.git, select "Initialize Remote Repository and Push")

but git gui reported:
fatal: GIT_WORK_TREE (or --work-tree=<directory>) not allowed without specifying GIT_DIR (or --git-dir=<directory>)

I bisected the error to
a9fa11fe5bd5978bb175b3b5663f6477a345d428 git-gui: set GIT_DIR and GIT_WORK_TREE after setup

I guess it is necessary to unset the GIT_DIR and GIT_WORKTREE env vars before calling
git --git-dir=$location init --bare

On a side note i am wondering why it is necessary to call mkdir -p?
And why it is not using git init --bare <path>
During some tests it created all leading directories.

Thanks,
Greetings Peter

^ permalink raw reply

* Re: a bug when execute "git status" in git version 1.7.7.431.g89633
From: Jeff King @ 2011-10-23 16:29 UTC (permalink / raw)
  To: René Scharfe; +Cc: John Hsing, Matthieu Moy, git
In-Reply-To: <4EA415BD.1040109@lsrfire.ath.cx>

On Sun, Oct 23, 2011 at 03:25:17PM +0200, René Scharfe wrote:

> I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:
> [...]
> Bisect points to 2548183ba, "fix phantom untracked files when
> core.ignorecase is set" from Jeff (cc:d).  If I revert that patch from
> master (8963314c), git status works fine.

Hmm. Interesting. I can't reproduce here. And I've been running with
this patch for over a year, and never seen that. Given your fix, I guess
it's related to pointer size. Are you on a 32-bit machine, by any
chance?

-Peff

^ permalink raw reply

* Re: a bug when execute "git status" in git version 1.7.7.431.g89633
From: René Scharfe @ 2011-10-23 17:50 UTC (permalink / raw)
  To: Jeff King; +Cc: John Hsing, Matthieu Moy, git
In-Reply-To: <20111023162944.GB28156@sigill.intra.peff.net>

Am 23.10.2011 18:29, schrieb Jeff King:
> On Sun, Oct 23, 2011 at 03:25:17PM +0200, René Scharfe wrote:
> 
>> I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:
>> [...]
>> Bisect points to 2548183ba, "fix phantom untracked files when
>> core.ignorecase is set" from Jeff (cc:d).  If I revert that patch from
>> master (8963314c), git status works fine.
> 
> Hmm. Interesting. I can't reproduce here. And I've been running with
> this patch for over a year, and never seen that. Given your fix, I guess
> it's related to pointer size. Are you on a 32-bit machine, by any
> chance?

Yes, it's a 32-bit VM.  I think it's a case of unlucky filename lengths,
combined with the rounding up to the next multiple of 8.  The following
table lists the actual size needed for entries based on the length of
their name entry.  Length calculation uses these offsets:

	offsetof(struct cache_entry, name) == 72
	offsetof(struct ondisk_cache_entry, name) == 62

	len  ce_size                 ondisk_ce_size          delta
	  1  (72 + 1 + 8) & ~7 = 80  (62 + 1 + 8) & ~7 = 64     16
	  2  (72 + 2 + 8) & ~7 = 80  (62 + 2 + 8) & ~7 = 72      8
	  3  (72 + 3 + 8) & ~7 = 80  (62 + 3 + 8) & ~7 = 72      8
	  4  (72 + 4 + 8) & ~7 = 80  (62 + 4 + 8) & ~7 = 72      8
	  5  (72 + 5 + 8) & ~7 = 80  (62 + 5 + 8) & ~7 = 72      8
	  6  (72 + 6 + 8) & ~7 = 80  (62 + 6 + 8) & ~7 = 72      8
	  7  (72 + 7 + 8) & ~7 = 80  (62 + 7 + 8) & ~7 = 72      8
	  8  (72 + 8 + 8) & ~7 = 88  (62 + 8 + 8) & ~7 = 72     16

So in 25% of the cases an entry needs 16 bytes more in memory than on
disk and the rest needs 8 bytes more.

estimate_cache_size() calculates the amount of memory needed for the
index, based on its on-disk representation.  It simply adds the
difference of the sizes of the two structs and the size of a pointer for
each entry to its total size and returns that number.  I have:

	sizeof(void *) == 4
	sizeof(struct cache_entry) == 72
	sizeof(struct ondisk_cache_entry) == 64

So each entry gets 72 - 64 + 4 = 12 bytes extra.  If you happen to have
a lot of filenames with a delta of 16 then the resulting size won't be
enough to hold the in-memory index.

Is there a nice way to derive that we need 16 bytes per entry in the
worst case, preferably without trying all eight possibilities as I did
in the table above?  My modular math is rusty..

René

^ permalink raw reply

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Junio C Hamano @ 2011-10-23 20:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, git, Ævar Arnfjörð
In-Reply-To: <20111023160744.GA22444@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> But as Duy mentions, we have an encoding header. Shouldn't we treat it
> like binary goo until we do reencode_log_message, and _then_ we can
> break it into lines?

That's sensible. If we go that route, I think the "one allocation of
separate struct commit_buffer pointed from a pointer field in struct
commit to replace the current member 'buffer'" is a reasonable thing
to do.

^ permalink raw reply

* Re: [PATCHv2 3/3] completion: match ctags symbol names in grep patterns
From: SZEDER Gábor @ 2011-10-23 21:29 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111021173021.GC24417@sigill.intra.peff.net>

Hi,

On Fri, Oct 21, 2011 at 01:30:21PM -0400, Jeff King wrote:
> This incorporates the suggestions from Gábor's review, with one
> exception: it still looks only in the current directory for the "tags"
> files. I think that might have some performance implications, so I'd
> rather add it separately, if at all.

I agree that scanning through a whole working tree for tags files
would cost too much.  But I think that a tags file at the top of the
working tree is common enough to be supported, and checking its
existence is fairly cheap.

> +	case "$cword,$prev" in
> +	2,*|*,-*)
> +		if test -r tags; then
> +			__gitcomp "$(__git_match_ctag "$cur" tags)"
> +			return
> +		fi
> +		;;

So how about something like this for the case arm? (I didn't actually
tested it.)

		local tagsfile
		if test -r tags; then
			tagsfile=tags
		else
			local dir="$(__gitdir)"
			if test -r "$dir"/tags; then
				tagsfile="$dir"/tags
			fi
		fi
		if [ -n "tagsfile" ]; then
			__gitcomp "$(__git_match_ctag "$cur" "$tagsfile")"
			return
		fi


Btw, there is a bug in the case statement: 'git --no-pager grep <TAB>'
offers refs instead of symbols, because $cword is not 2 and $prev
doesn't start with a dash.  But it's not worse than the current
behavior, so I don't think this bug is a show-stopper for the patch.


Best,
Gábor

^ permalink raw reply

* [PATCH] read-cache.c: fix index memory allocation
From: René Scharfe @ 2011-10-24  1:01 UTC (permalink / raw)
  Cc: Jeff King, John Hsing, Matthieu Moy, git, Junio C Hamano
In-Reply-To: <4EA453D3.7080002@lsrfire.ath.cx>

estimate_cache_size() tries to guess how much memory is needed for the
in-memory representation of an index file.  It does that by using the
file size, the number of entries and the difference of the sizes of the
on-disk and in-memory structs -- without having to check the length of
the name of each entry, which varies for each entry, but their sums are
the same no matter the representation.

Except there can be a difference.  First of all, the size is really
calculated by ce_size and ondisk_ce_size based on offsetof(..., name),
not sizeof, which can be different.  And entries are padded with 1 to 8
NULs at the end (after the variable name) to make their total length a
multiple of eight.

So in order to allocate enough memory to hold the index, change the
delta calculation to be based on offsetof(..., name) and round up to
the next multiple of eight.

On a 32-bit Linux, this delta was used before:

	sizeof(struct cache_entry)        == 72
	sizeof(struct ondisk_cache_entry) == 64
	                                    ---
	                                      8

The actual difference for an entry with a filename length of one was,
however (find the definitions are in cache.h):

	offsetof(struct cache_entry, name)        == 72
	offsetof(struct ondisk_cache_entry, name) == 62

	ce_size        == (72 + 1 + 8) & ~7 == 80
	ondisk_ce_size == (62 + 1 + 8) & ~7 == 64
	                                      ---
	                                       16

So eight bytes less had been allocated for such entries.  The new
formula yields the correct delta:

	(72 - 62 + 7) & ~7 == 16

Reported-by: John Hsing <tsyj2007@gmail.com>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 read-cache.c            |    6 ++--
 t/t7510-status-index.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100755 t/t7510-status-index.sh

diff --git a/read-cache.c b/read-cache.c
index 01a0e25..5790a91 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1249,9 +1249,9 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en
 
 static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
 {
-	long per_entry;
-
-	per_entry = sizeof(struct cache_entry) - sizeof(struct ondisk_cache_entry);
+	size_t fix_size_mem = offsetof(struct cache_entry, name);
+	size_t fix_size_dsk = offsetof(struct ondisk_cache_entry, name);
+	long per_entry = (fix_size_mem - fix_size_dsk + 7) & ~7;
 
 	/*
 	 * Alignment can cause differences. This should be "alignof", but
diff --git a/t/t7510-status-index.sh b/t/t7510-status-index.sh
new file mode 100755
index 0000000..bca359d
--- /dev/null
+++ b/t/t7510-status-index.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='git status with certain file name lengths'
+
+. ./test-lib.sh
+
+files="0 1 2 3 4 5 6 7 8 9 a b c d e f g h i j k l m n o p q r s t u v w x y z"
+
+check() {
+	len=$1
+	prefix=$2
+
+	for i in $files
+	do
+		: >$prefix$i
+	done
+
+	test_expect_success "status, filename length $len" "
+		git add $prefix* &&
+		git status
+	"
+	rm $prefix* .git/index
+}
+
+check  1
+check  2 p
+check  3 pr
+check  4 pre
+check  5 pref
+check  6 prefi
+check  7 prefix
+check  8 prefix-
+check  9 prefix-p
+check 10 prefix-pr
+check 11 prefix-pre
+check 12 prefix-pref
+check 13 prefix-prefi
+check 14 prefix-prefix
+check 15 prefix-prefix-
+check 16 prefix-prefix-p
+check 17 prefix-prefix-pr
+check 18 prefix-prefix-pre
+check 19 prefix-prefix-pref
+check 20 prefix-prefix-prefi
+check 21 prefix-prefix-prefix
+check 22 prefix-prefix-prefix-
+check 23 prefix-prefix-prefix-p
+check 24 prefix-prefix-prefix-pr
+
+test_done
-- 
1.7.7

^ permalink raw reply related

* [PATCH] Reindent closing bracket using tab instead of spaces
From: Nguyễn Thái Ngọc Duy @ 2011-10-24  4:24 UTC (permalink / raw)
  To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I'm not going to convert all leading spaces to tabs. But this one looks just ugly
 because it mis-aligns with the rest of the function.

 wt-status.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 8836a52..70fdb76 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -396,7 +396,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	if (s->ignore_submodule_arg) {
 		DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
 		handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
-    }
+	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
 	init_pathspec(&rev.prune_data, s->pathspec);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Junio C Hamano @ 2011-10-24  4:40 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyen Thai Ngoc Duy, Ævar Arnfjörð
In-Reply-To: <7v39ej5uqb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> But as Duy mentions, we have an encoding header. Shouldn't we treat it
>> like binary goo until we do reencode_log_message, and _then_ we can
>> break it into lines?
>
> That's sensible. If we go that route, I think the "one allocation of
> separate struct commit_buffer pointed from a pointer field in struct
> commit to replace the current member 'buffer'" is a reasonable thing
> to do.

Having given that "sensible" comment, I am not convinced if this is worth
it. We are talking about what is left in the ephemeral COMMIT_EDITMSG by
the chosen editor, but are there really editors that can _only_ write in
UTF-16 and not in UTF-8, and is it worth bending backwards to add support
such an editor?

^ permalink raw reply

* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Nguyen Thai Ngoc Duy @ 2011-10-24  5:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Ævar Arnfjörð
In-Reply-To: <7vy5wb3sto.fsf@alter.siamese.dyndns.org>

On Mon, Oct 24, 2011 at 3:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> But as Duy mentions, we have an encoding header. Shouldn't we treat it
>>> like binary goo until we do reencode_log_message, and _then_ we can
>>> break it into lines?
>>
>> That's sensible. If we go that route, I think the "one allocation of
>> separate struct commit_buffer pointed from a pointer field in struct
>> commit to replace the current member 'buffer'" is a reasonable thing
>> to do.
>
> Having given that "sensible" comment, I am not convinced if this is worth
> it. We are talking about what is left in the ephemeral COMMIT_EDITMSG by
> the chosen editor, but are there really editors that can _only_ write in
> UTF-16 and not in UTF-8, and is it worth bending backwards to add support
> such an editor?

This is argument for the sake of argument because I don't use utf-16
and do not care much. UTF-16 can have more code points and some may
prefer utf-16 to utf-8.

I'd be happy with git's refusing to create broken commits because
people accidentally set editor encoding to utf-16. If people do use
utf-16, they'll get caught and should yell up if they want utf-16
supported.
-- 
Duy

^ permalink raw reply

* [ANNOUNCE] Git 1.7.7.1
From: Junio C Hamano @ 2011-10-24  6:18 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

The latest maintenance release Git 1.7.7.1 is available.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

9200e0b8ee543d297952b78aac8f61f8b3693f8e  git-1.7.7.1.tar.gz
b25dacb07ebbfc37e7a90c3d47f76b4c0f0487d9  git-htmldocs-1.7.7.1.tar.gz
419c750617ae0c952e2e43f0357c16de6ebc0a44  git-manpages-1.7.7.1.tar.gz

Also the following public repositories all have a copy of the v1.7.7.1
tag and the maint branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

----------------------------------------------------------------

Changes since v1.7.7 are as follows:

Brad King (1):
      rev-list: Demonstrate breakage with --ancestry-path --all

Brandon Casey (1):
      strbuf.c: remove unnecessary strbuf_grow() from strbuf_getwholeline()

Ilari Liusvaara (1):
      Support ERR in remote archive like in fetch/push

Jay Soffian (1):
      merge-one-file: fix "expr: non-numeric argument"

Jeff King (2):
      fetch: avoid quadratic loop checking for updated submodules
      filter-branch: use require_clean_work_tree

Jim Meyering (1):
      fix "git apply --index ..." not to deref NULL

Jonathan Nieder (2):
      Makefile: do not set setgid bit on directories on GNU/kFreeBSD
      RelNotes/1.7.7.1: setgid bit patch is about fixing "git init" via Makefile setting

Junio C Hamano (14):
      revision: keep track of the end-user input from the command line
      revision: do not include sibling history in --ancestry-path output
      rebase -i: notice and warn if "exec $cmd" modifies the index or the working tree
      traverse_trees(): allow pruning with pathspec
      unpack-trees: allow pruning with pathspec
      diff-index: pass pathspec down to unpack-trees machinery
      fsck: do not abort upon finding an empty blob
      Teach progress eye-candy to fetch_refs_from_bundle()
      apply --whitespace=error: correctly report new blank lines at end
      checkout $tree $path: do not clobber local changes in $path not in $tree
      diff: resurrect XDF_NEED_MINIMAL with --minimal
      Prepare for 1.7.7.1
      Almost ready for 1.7.7.1
      Git 1.7.7.1

Matthieu Moy (2):
      rebase -i: clean error message for --continue after failed exec
      config: display key_delim for config --bool --get-regexp

Michael Schubert (1):
      patch-id.c: use strbuf instead of a fixed buffer

Nguyễn Thái Ngọc Duy (4):
      merge: keep stash[] a local variable
      merge: use return value of resolve_ref() to determine if HEAD is invalid
      merge: remove global variable head[]
      Accept tags in HEAD or MERGE_HEAD

Nicolas Morey-Chaisemartin (1):
      grep: Fix race condition in delta_base_cache

René Scharfe (2):
      Revert removal of multi-match discard heuristic in 27af01
      t1304: fall back to $USER if $LOGNAME is not defined

Thomas Rast (2):
      Symlink mergetools scriptlets into valgrind wrappers
      t6019: avoid refname collision on case-insensitive systems

^ permalink raw reply

* [PATCH/WIP 00/11] read_directory() rewrite to support struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2011-10-24  6:36 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This is the first time "make test" fully passes (*) for me, so it's
probably good enough for human eyes. Just heads up where this might
go.

A few points:

 - "git add --ignore-missing" is killed because I could not find an
   easy way to incorporate it to the new read_directory(). It looks
   like a hack to me, to expose .gitignore matching. Luckily no one
   except submodule seems to use it.

 - I chose to use tree_entry_interesting() instead of
   match_pathspec(). The former has more optimizations but requires a
   tree-based structure. So I have to read the whole directory in,
   re-construct a temporary tree object to make t_e_i() happy. I
   _think_ it does not impact performance with reasonable dir size.

 - there'll be more work to get rid of match_pathspec() calls after
   read_directory()/fill_directory(). I haven't got finished this part
   yet.

 - I really like to kill match_pathspec() so we only have one pathspec
   implementation instead of two now, but that may be real hard
   because of staged entries in index.

(*) t7012.7 fails but I think that's the test's fault.

Nguyễn Thái Ngọc Duy (11):
  Introduce "check-attr --excluded" as a replacement for "add --ignore-missing"
  notes-merge: use opendir/readdir instead of using read_directory()
  t5403: avoid doing "git add foo/bar" where foo/.git exists
  tree-walk.c: do not leak internal structure in tree_entry_len()
  symbolize return values of tree_entry_interesting()
  read_directory_recursive: reduce one indentation level
  tree_entry_interesting: make use of local pointer "item"
  tree-walk: mark useful pathspecs
  tree_entry_interesting: differentiate partial vs full match
  read-dir: stop using path_simplify code in favor of tree_entry_interesting()
  dir.c: remove dead code after read_directory() rewrite

 Documentation/git-check-attr.txt |    4 +
 builtin/add.c                    |   36 ++--
 builtin/check-attr.c             |   26 +++
 builtin/grep.c                   |   11 +-
 builtin/pack-objects.c           |    2 +-
 cache.h                          |    1 +
 dir.c                            |  428 +++++++++++++++++++-------------------
 dir.h                            |    8 +-
 git-submodule.sh                 |    2 +-
 list-objects.c                   |    9 +-
 notes-merge.c                    |   45 +++--
 t/t3700-add.sh                   |   19 --
 t/t5403-post-checkout-hook.sh    |   17 +-
 tree-diff.c                      |   19 +-
 tree-walk.c                      |   85 ++++----
 tree-walk.h                      |   19 ++-
 tree.c                           |   11 +-
 unpack-trees.c                   |    6 +-
 18 files changed, 394 insertions(+), 354 deletions(-)

-- 
1.7.3.1.256.g2539c.dirty

^ 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