Git development
 help / color / mirror / Atom feed
* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Larry D'Anna @ 2010-02-05 23:00 UTC (permalink / raw)
  To: Wesley J. Landaker; +Cc: Junio C Hamano, git
In-Reply-To: <201002051553.27315.wjl@icecavern.net>

* Wesley J. Landaker (wjl@icecavern.net) [100205 17:53]:
> On Friday 05 February 2010 15:33:23 Junio C Hamano wrote:
> > The placeholder in particular and the cover letter itself in general are
> > meant to be edited.  I do not see much point in forcing people to edit
> >  yet another file and have them specify with an cover-blurb option.
> > 
> > Not very interested.
> 
> The original use-case is also pretty close to just doing the following:
> 
> $ git format-patch master..HEAD --cover-letter 
> $ vi 0000-cover-letter.patch
> $ git send-email --to my@mydomain.org *.patch
> $ git send-email --to git@vger.kernel.org *.patch
> 
> Isn't that just as easy as the proposed --cover-* options?

Except when you decide you need to modify it after sending it to yourself.  

       --larry

^ permalink raw reply

* Re: Implement --password option for git svn perl script
From: Steve Diver @ 2010-02-05 23:01 UTC (permalink / raw)
  To: Eric Wong; +Cc: Laszlo Papp, Frank Li, Tay Ray Chuan, git
In-Reply-To: <20100202090538.GA28437@dcvr.yhbt.net>

On 02/02/2010 09:05, Eric Wong wrote:
>
> Laszlo Papp<djszapi@archlinux.us>  wrote:
>>> The basic requirement is git svn can provide a way to prompt a dialog
>>> box to input passwd.
>>
>> Yeah, it's a very basic use case, but I don't know whether there is a
>> workaround for it somehow, maybe perl gurus or git svn users could
>> help more...
>>
>> My company, where there are windows users (95% of the company), would
>> like to use TortoiseGIT as a graphical client for SVN server
>> repositories as an SVN client, no other way by us, so it'd be high
>> priority :) So I've got a beer for any volunteer, thanks :P
>
> Hi, (fixed subject so I don't mistake it for junk again)
>
> I don't know (and have no way to test) how to deal with input dialogs
> for reading a password on Windows (nor any sort of IPC).  Somebody else
> will have to implement it.
>
> Having a --password option in the command line could work, but it's
> painfully insecure if there's any way for another regular user to view
> the process table.  Not something I'd like to encourage...
>
> Since SVN already caches passwords in a mostly secure location on disk
> (at least on *nix), shouldn't git svn be able to use the password cache
> SVN uses?
>

Tortoise, along with at least one other Windows GUI utilizes the Putty 
suite as a helper app for key chain and password handling, and needs to 
be loaded prior to any SSH operations. Tortoise have gone so far as to 
rewrite plink (Tortoiseplink.exe)so that that a prompt for the password 
is given, but this is undocumented AFAIK.

Otherwise, it is an unfortunate fact that a DOS console session is 
invoked whenever openSSH is used. For the average Windows user this is a 
scary occurrence, and is one of the obstacles that prevents wider casual 
take up of Git on Windows IMHO.

I understand that it comes down to a problem with communication between 
a dialogue and an SSH client via STDOUT. I believe Dscho might have some 
insight into this as it is a recurrent topic on the MsysGit tracker.

Steve

^ permalink raw reply

* git-grep: option parsing conflicts with prefix-dash searches
From: Jan Engelhardt @ 2010-02-05 23:09 UTC (permalink / raw)
  To: git

Greetings.


Just about now I wanted to grep for accesses of a particular struct 
member. Needless to say that it was not a very amusing experience.
I would expect that (1) probably fails:

(1)	$ git grep '->cnt' net/ipv4/netfilter/
	error: unknown switch `>'

So far so good, seems reasonable and matches what I would expect from 
most other userspace tools. So let's add -- to terminate the option 
list:

(2)	$ git grep -- '->cnt' net/ipv4/netfilter/
	fatal: bad flag '->cnt' used after filename

*bzzt*. This does not match typical behavior. Let alone that "--"
is not a filename.

What works is (3).

(3)	$ git grep -- -- '->cnt' net/ipv4/netfilter/

But it almost looks like Morse code. Or Perl. Imagine I were to
approxmiately search for all options in iptables's in-code help texts:

	git grep -- -- -- .

I think that overloading "--" was a bad choice. The option parser has
many more awkward behavior, such as not allowing to bundle most
options (`git log -z -p` vs. `git log -zp`) yet forcing it on other
options (`git log -Spattern` vs `git log -S pattern`). The use of
historic counts (cf. `git log -30` and `tail -30`) compared to a more
modern `tail -n30`) also prohibits using many standard parsers
(most notably getopt(3)), as they would recognize that as -3 -0.

As I said, it's a mess. And I know not whether any code can convince
the "but we need to watch compatibility"-sayers, because this would
definitely be a flag change.

^ permalink raw reply

* [PATCH 0/9] add --ff option to cherry-pick
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd

Changes since the previous series are the following:

- added some tests in a new patch
- added some documentation in a new patch
- added a patch to use the new "reset" function in builtin-merge.c
- made --ff incompatible with --edit -x and --signoff too, thanks to Paolo
- improved the first patch with suggestions from Stephen Boyd, thanks
- made a "struct lock_file" static to initialize it in the first patch
- improved commit messages

Christian Couder (8):
  reset: refactor updating heads into a static function
  reset: refactor reseting in its own function
  reset: make reset function non static and declare it in "reset.h"
  revert: add --ff option to allow fast forward when cherry-picking
  cherry-pick: add tests for new --ff option
  Documentation: describe new cherry-pick --ff option
  rebase -i: use new --ff cherry-pick option
  merge: use new "reset" function instead of running "git read-tree"

Stephan Beyer (1):
  revert: libify cherry-pick and revert functionnality

 Documentation/git-cherry-pick.txt |    6 +-
 Makefile                          |    2 +
 builtin-merge.c                   |   31 ++---
 builtin-reset.c                   |  175 ++++++++++++----------
 builtin-revert.c                  |  303 +++++++++----------------------------
 git-rebase--interactive.sh        |   15 +--
 pick.c                            |  218 ++++++++++++++++++++++++++
 pick.h                            |   13 ++
 reset.h                           |   10 ++
 t/t3506-cherry-pick-ff.sh         |   38 +++++
 10 files changed, 471 insertions(+), 340 deletions(-)
 create mode 100644 pick.c
 create mode 100644 pick.h
 create mode 100644 reset.h
 create mode 100755 t/t3506-cherry-pick-ff.sh

^ permalink raw reply

* [PATCH 4/9] reset: make reset function non static and declare it in "reset.h"
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>


Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c |    8 ++++----
 reset.h         |   10 ++++++++++
 2 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 reset.h

diff --git a/builtin-reset.c b/builtin-reset.c
index bf97931..b004f9a 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -20,6 +20,7 @@
 #include "parse-options.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
+#include "reset.h"
 
 static const char * const git_reset_usage[] = {
 	"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
@@ -27,7 +28,6 @@ static const char * const git_reset_usage[] = {
 	NULL
 };
 
-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
 static const char *reset_type_names[] = { "mixed", "soft", "hard", "merge", NULL };
 
 static char *args_to_str(const char **argv)
@@ -235,9 +235,9 @@ static int update_heads(unsigned char *sha1)
 	return update_ref(msg, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 }
 
-static int reset(const char *rev, const char *prefix,
-		 int reset_type, int quiet, int patch_mode,
-		 int argc, const char **argv)
+int reset(const char *rev, const char *prefix,
+	  int reset_type, int quiet, int patch_mode,
+	  int argc, const char **argv)
 {
 	struct commit *commit;
 	unsigned char sha1[20];
diff --git a/reset.h b/reset.h
new file mode 100644
index 0000000..c8497e4
--- /dev/null
+++ b/reset.h
@@ -0,0 +1,10 @@
+#ifndef RESET_H
+#define RESET_H
+
+enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
+
+int reset(const char *rev, const char *prefix,
+	  int reset_type, int quiet, int patch_mode,
+	  int argc, const char **argv);
+
+#endif
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 1/9] revert: libify cherry-pick and revert functionnality
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>

From: Stephan Beyer <s-beyer@gmx.net>

The goal of this commit is to abstract out "git cherry-pick" and
"git revert" functionnality into a new pick_commit() function made
of code from "builtin-revert.c".

The new pick_commit() function is in a new "pick.c" file with an
associated "pick.h".

This function starts from the current index (not HEAD), and allow
the effect of one commit replayed (either forward or backward) to
that state, leaving the result in the index. So it makes it
possible to replay many commits to the index in sequence without
commiting in between.

This commit is made of code from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

And it contains some changes and comments suggested by Junio.

The original commit in the sequencer project that introduced
this change is: 94a568a78d243d7a6c13778bc6b7ac1eb46e48cc

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile         |    2 +
 builtin-revert.c |  272 +++++++++---------------------------------------------
 pick.c           |  218 +++++++++++++++++++++++++++++++++++++++++++
 pick.h           |   13 +++
 4 files changed, 277 insertions(+), 228 deletions(-)
 create mode 100644 pick.c
 create mode 100644 pick.h

diff --git a/Makefile b/Makefile
index d624530..71901c8 100644
--- a/Makefile
+++ b/Makefile
@@ -458,6 +458,7 @@ LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
+LIB_H += pick.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
 LIB_H += quote.h
@@ -550,6 +551,7 @@ LIB_OBJS += parse-options.o
 LIB_OBJS += patch-delta.o
 LIB_OBJS += patch-ids.o
 LIB_OBJS += path.o
+LIB_OBJS += pick.o
 LIB_OBJS += pkt-line.o
 LIB_OBJS += preload-index.o
 LIB_OBJS += pretty.o
diff --git a/builtin-revert.c b/builtin-revert.c
index 8ac86f0..7488d4c 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -1,18 +1,14 @@
 #include "cache.h"
 #include "builtin.h"
-#include "object.h"
 #include "commit.h"
 #include "tag.h"
-#include "wt-status.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 "pick.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -35,26 +31,24 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline, signoff;
-static enum { REVERT, CHERRY_PICK } action;
+static int edit, no_commit, mainline, signoff;
+static int flags;
 static struct commit *commit;
 static int allow_rerere_auto;
 
-static const char *me;
-
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
 static void parse_args(int argc, const char **argv)
 {
 	const char * const * usage_str =
-		action == REVERT ?  revert_usage : cherry_pick_usage;
+		flags & PICK_REVERSE ? revert_usage : cherry_pick_usage;
 	unsigned char sha1[20];
 	const char *arg;
 	int noop;
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
-		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
+		OPT_BIT('x', NULL, &flags, "append commit name when cherry-picking", PICK_ADD_NOTE),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
@@ -79,42 +73,12 @@ static void parse_args(int argc, const char **argv)
 		die ("'%s' does not point to a commit", arg);
 }
 
-static char *get_oneline(const char *message)
-{
-	char *result;
-	const char *p = message, *abbrev, *eol;
-	int abbrev_len, oneline_len;
-
-	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (*p) {
-		p += 2;
-		for (eol = p + 1; *eol && *eol != '\n'; eol++)
-			; /* do nothing */
-	} else
-		eol = p;
-	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
-	abbrev_len = strlen(abbrev);
-	oneline_len = eol - p;
-	result = xmalloc(abbrev_len + 5 + oneline_len);
-	memcpy(result, abbrev, abbrev_len);
-	memcpy(result + abbrev_len, "... ", 4);
-	memcpy(result + abbrev_len + 4, p, oneline_len);
-	result[abbrev_len + 4 + oneline_len] = '\0';
-	return result;
-}
-
 static char *get_encoding(const char *message)
 {
 	const char *p = message, *eol;
 
 	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
+		return NULL;
 	while (*p && *p != '\n') {
 		for (eol = p + 1; *eol && *eol != '\n'; eol++)
 			; /* do nothing */
@@ -130,30 +94,6 @@ static char *get_encoding(const char *message)
 	return NULL;
 }
 
-static struct lock_file msg_file;
-static int msg_fd;
-
-static void add_to_msg(const char *string)
-{
-	int len = strlen(string);
-	if (write_in_full(msg_fd, string, len) < 0)
-		die_errno ("Could not write to MERGE_MSG");
-}
-
-static void add_message_to_msg(const char *message)
-{
-	const char *p = message;
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (!*p)
-		add_to_msg(sha1_to_hex(commit->object.sha1));
-
-	p += 2;
-	add_to_msg(p);
-	return;
-}
-
 static void set_author_ident_env(const char *message)
 {
 	const char *p = message;
@@ -216,7 +156,7 @@ static char *help_msg(const unsigned char *sha1)
 	       "mark the corrected paths with 'git add <paths>' "
 	       "or 'git rm <paths>' and commit the result.");
 
-	if (action == CHERRY_PICK) {
+	if (!(flags & PICK_REVERSE)) {
 		sprintf(helpbuf + strlen(helpbuf),
 			"\nWhen commiting, use the option "
 			"'-c %s' to retain authorship and message.",
@@ -225,14 +165,17 @@ static char *help_msg(const unsigned char *sha1)
 	return helpbuf;
 }
 
-static struct tree *empty_tree(void)
+static void write_message(struct strbuf *msgbuf, const char *filename)
 {
-	struct tree *tree = xcalloc(1, sizeof(struct tree));
-
-	tree->object.parsed = 1;
-	tree->object.type = OBJ_TREE;
-	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
-	return tree;
+	static struct lock_file msg_file;
+	int msg_fd;
+	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 NORETURN void die_dirty_index(const char *me)
@@ -250,175 +193,53 @@ static NORETURN void die_dirty_index(const char *me)
 
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
-	unsigned char head[20];
-	struct commit *base, *next, *parent;
-	int i, index_fd, clean;
-	char *oneline, *reencoded_message = NULL;
-	const char *message, *encoding;
-	char *defmsg = git_pathdup("MERGE_MSG");
-	struct merge_options o;
-	struct tree *result, *next_tree, *base_tree, *head_tree;
-	static struct lock_file index_lock;
+	const char *me;
+	struct strbuf msgbuf;
+	char *reencoded_message = NULL;
+	const char *encoding;
+	int failed;
 
 	git_config(git_default_config, NULL);
-	me = action == REVERT ? "revert" : "cherry-pick";
+	me = flags & PICK_REVERSE ? "revert" : "cherry-pick";
 	setenv(GIT_REFLOG_ACTION, me, 0);
 	parse_args(argc, argv);
 
-	/* this is copied from the shell script, but it's never triggered... */
-	if (action == REVERT && !no_replay)
-		die("revert is incompatible with replay");
-
 	if (read_cache() < 0)
 		die("git %s: failed to read the index", me);
-	if (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))
-			die ("You do not have a valid HEAD");
-		if (index_differs_from("HEAD", 0))
-			die_dirty_index(me);
-	}
-	discard_cache();
-
-	index_fd = hold_locked_index(&index_lock, 1);
+	if (!no_commit && index_differs_from("HEAD", 0))
+		die_dirty_index(me);
 
-	if (!commit->parents) {
-		if (action == REVERT)
-			die ("Cannot revert a root commit");
-		parent = NULL;
-	}
-	else if (commit->parents->next) {
-		/* Reverting or cherry-picking a merge commit */
-		int cnt;
-		struct commit_list *p;
-
-		if (!mainline)
-			die("Commit %s is a merge but no -m option was given.",
-			    sha1_to_hex(commit->object.sha1));
-
-		for (cnt = 1, p = commit->parents;
-		     cnt != mainline && p;
-		     cnt++)
-			p = p->next;
-		if (cnt != mainline || !p)
-			die("Commit %s does not have parent %d",
-			    sha1_to_hex(commit->object.sha1), mainline);
-		parent = p->item;
-	} else if (0 < mainline)
-		die("Mainline was specified but commit %s is not a merge.",
-		    sha1_to_hex(commit->object.sha1));
-	else
-		parent = commit->parents->item;
-
-	if (!(message = commit->buffer))
-		die ("Cannot get commit message for %s",
+	if (!commit->buffer)
+		return error("Cannot get commit message for %s",
 				sha1_to_hex(commit->object.sha1));
-
-	if (parent && parse_commit(parent) < 0)
-		die("%s: cannot parse parent commit %s",
-		    me, sha1_to_hex(parent->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.
-	 */
-
-	msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
-					   LOCK_DIE_ON_ERROR);
-
-	encoding = get_encoding(message);
+	encoding = get_encoding(commit->buffer);
 	if (!encoding)
 		encoding = "UTF-8";
 	if (!git_commit_encoding)
 		git_commit_encoding = "UTF-8";
-	if ((reencoded_message = reencode_string(message,
+	if ((reencoded_message = reencode_string(commit->buffer,
 					git_commit_encoding, encoding)))
-		message = reencoded_message;
-
-	oneline = get_oneline(message);
-
-	if (action == REVERT) {
-		char *oneline_body = strchr(oneline, ' ');
-
-		base = commit;
-		next = parent;
-		add_to_msg("Revert \"");
-		add_to_msg(oneline_body + 1);
-		add_to_msg("\"\n\nThis reverts commit ");
-		add_to_msg(sha1_to_hex(commit->object.sha1));
-
-		if (commit->parents->next) {
-			add_to_msg(", reversing\nchanges made to ");
-			add_to_msg(sha1_to_hex(parent->object.sha1));
-		}
-		add_to_msg(".\n");
-	} else {
-		base = parent;
-		next = commit;
-		set_author_ident_env(message);
-		add_message_to_msg(message);
-		if (no_replay) {
-			add_to_msg("(cherry picked from commit ");
-			add_to_msg(sha1_to_hex(commit->object.sha1));
-			add_to_msg(")\n");
-		}
-	}
-
-	read_cache();
-	init_merge_options(&o);
-	o.branch1 = "HEAD";
-	o.branch2 = oneline;
+		commit->buffer = reencoded_message;
 
-	head_tree = parse_tree_indirect(head);
-	next_tree = next ? next->tree : empty_tree();
-	base_tree = base ? base->tree : empty_tree();
-
-	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)))
-		die("%s: Unable to write new index file", me);
-	rollback_lock_file(&index_lock);
-
-	if (!clean) {
-		add_to_msg("\nConflicts:\n\n");
-		for (i = 0; i < active_nr;) {
-			struct cache_entry *ce = active_cache[i++];
-			if (ce_stage(ce)) {
-				add_to_msg("\t");
-				add_to_msg(ce->name);
-				add_to_msg("\n");
-				while (i < active_nr && !strcmp(ce->name,
-						active_cache[i]->name))
-					i++;
-			}
-		}
-		if (commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up %s", defmsg);
+	failed = pick_commit(commit, mainline, flags, &msgbuf);
+	if (failed < 0) {
+		exit(1);
+	} else if (failed > 0) {
 		fprintf(stderr, "Automatic %s failed.%s\n",
 			me, help_msg(commit->object.sha1));
+		write_message(&msgbuf, git_path("MERGE_MSG"));
 		rerere(allow_rerere_auto);
 		exit(1);
 	}
-	if (commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up %s", defmsg);
+	if (!(flags & PICK_REVERSE))
+		set_author_ident_env(commit->buffer);
+	free(reencoded_message);
+
 	fprintf(stderr, "Finished one %s.\n", me);
 
+	write_message(&msgbuf, git_path("MERGE_MSG"));
+
 	/*
-	 *
 	 * 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.
@@ -436,14 +257,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			args[i++] = "-s";
 		if (!edit) {
 			args[i++] = "-F";
-			args[i++] = defmsg;
+			args[i++] = git_path("MERGE_MSG");
 		}
 		args[i] = NULL;
 		return execv_git_cmd(args);
 	}
-	free(reencoded_message);
-	free(defmsg);
-
 	return 0;
 }
 
@@ -451,14 +269,12 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	if (isatty(0))
 		edit = 1;
-	no_replay = 1;
-	action = REVERT;
+	flags = PICK_REVERSE | PICK_ADD_NOTE;
 	return revert_or_cherry_pick(argc, argv);
 }
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-	no_replay = 0;
-	action = CHERRY_PICK;
+	flags = 0;
 	return revert_or_cherry_pick(argc, argv);
 }
diff --git a/pick.c b/pick.c
new file mode 100644
index 0000000..1e1628a
--- /dev/null
+++ b/pick.c
@@ -0,0 +1,218 @@
+#include "cache.h"
+#include "commit.h"
+#include "run-command.h"
+#include "cache-tree.h"
+#include "pick.h"
+#include "merge-recursive.h"
+
+static struct commit *commit;
+
+static char *get_oneline(const char *message)
+{
+	char *result;
+	const char *p = message, *abbrev, *eol;
+	int abbrev_len, oneline_len;
+
+	if (!p)
+		return NULL;
+	while (*p && (*p != '\n' || p[1] != '\n'))
+		p++;
+
+	if (*p) {
+		p += 2;
+		for (eol = p + 1; *eol && *eol != '\n'; eol++)
+			; /* do nothing */
+	} else
+		eol = p;
+	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+	abbrev_len = strlen(abbrev);
+	oneline_len = eol - p;
+	result = xmalloc(abbrev_len + 5 + oneline_len);
+	memcpy(result, abbrev, abbrev_len);
+	memcpy(result + abbrev_len, "... ", 4);
+	memcpy(result + abbrev_len + 4, p, oneline_len);
+	result[abbrev_len + 4 + oneline_len] = '\0';
+	return result;
+}
+
+static void add_message_to_msg(struct strbuf *msg, const char *message)
+{
+	const char *p = message;
+	while (*p && (*p != '\n' || p[1] != '\n'))
+		p++;
+
+	if (!*p)
+		strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+
+	p += 2;
+	strbuf_addstr(msg, p);
+	return;
+}
+
+static struct tree *empty_tree(void)
+{
+	struct tree *tree = xcalloc(1, sizeof(struct tree));
+
+	tree->object.parsed = 1;
+	tree->object.type = OBJ_TREE;
+	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
+	return tree;
+}
+
+/*
+ * Pick changes introduced by "commit" argument into current working
+ * tree and index.
+ *
+ * It starts from the current index (not HEAD), and allow the effect
+ * of one commit replayed (either forward or backward) to that state,
+ * leaving the result in the index.
+ *
+ * You do not have to start from a commit, so you can replay many commits
+ * to the index in sequence without commiting in between to squash multiple
+ * steps if you wanted to.
+ *
+ * Return 0 on success.
+ * Return negative value on error before picking,
+ * and a positive value after picking,
+ * and return 1 if and only if a conflict occurs but no other error.
+ */
+int pick_commit(struct commit *pick_commit, int mainline, int flags,
+		struct strbuf *msg)
+{
+	unsigned char head[20];
+	struct commit *base, *next, *parent;
+	int i, index_fd, clean;
+	int ret = 0;
+	char *oneline;
+	const char *message;
+	struct merge_options o;
+	struct tree *result, *next_tree, *base_tree, *head_tree;
+	static struct lock_file index_lock;
+
+	strbuf_init(msg, 0);
+	commit = pick_commit;
+
+	/*
+	 * Let's compute the tree that represents the "current" state
+	 * for merge-recursive to work on.
+	 */
+	if (write_cache_as_tree(head, 0, NULL))
+		return error("Your index file is unmerged.");
+	discard_cache();
+
+	index_fd = hold_locked_index(&index_lock, 0);
+	if (index_fd < 0)
+		return error("Unable to create locked index: %s",
+			     strerror(errno));
+
+	if (!commit->parents) {
+		if (flags & PICK_REVERSE)
+			return error("Cannot revert a root commit");
+		parent = NULL;
+	}
+	else if (commit->parents->next) {
+		/* Reverting or cherry-picking a merge commit */
+		int cnt;
+		struct commit_list *p;
+
+		if (!mainline)
+			return error("Commit %s is a merge but no mainline was given.",
+				     sha1_to_hex(commit->object.sha1));
+
+		for (cnt = 1, p = commit->parents;
+		     cnt != mainline && p;
+		     cnt++)
+			p = p->next;
+		if (cnt != mainline || !p)
+			return error("Commit %s does not have parent %d",
+				     sha1_to_hex(commit->object.sha1),
+				     mainline);
+		parent = p->item;
+	} else if (0 < 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 (!(message = commit->buffer))
+		return error("Cannot get commit message for %s",
+			     sha1_to_hex(commit->object.sha1));
+
+	if (parent && parse_commit(parent) < 0)
+		return error("Cannot parse parent commit %s",
+			     sha1_to_hex(parent->object.sha1));
+
+	oneline = get_oneline(message);
+
+	if (flags & PICK_REVERSE) {
+		char *oneline_body = strchr(oneline, ' ') + 1;
+
+		base = commit;
+		next = parent;
+		strbuf_addstr(msg, "Revert \"");
+		strbuf_addstr(msg, oneline_body);
+		strbuf_addstr(msg, "\"\n\nThis reverts commit ");
+		strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+
+		if (commit->parents->next) {
+			strbuf_addstr(msg, ", reversing\nchanges made to ");
+			strbuf_addstr(msg, sha1_to_hex(parent->object.sha1));
+		}
+		strbuf_addstr(msg, ".\n");
+	} else {
+		base = parent;
+		next = commit;
+		add_message_to_msg(msg, message);
+		if (flags & PICK_ADD_NOTE) {
+			strbuf_addstr(msg, "(cherry picked from commit ");
+			strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+			strbuf_addstr(msg, ")\n");
+		}
+	}
+
+	read_cache();
+	init_merge_options(&o);
+	o.branch1 = "HEAD";
+	o.branch2 = oneline;
+
+	head_tree = parse_tree_indirect(head);
+	next_tree = next ? next->tree : empty_tree();
+	base_tree = base ? base->tree : empty_tree();
+
+	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))) {
+		error("Unable to write new index file");
+		return 2;
+	}
+	rollback_lock_file(&index_lock);
+
+	if (!clean) {
+		strbuf_addstr(msg, "\nConflicts:\n\n");
+		for (i = 0; i < active_nr;) {
+			struct cache_entry *ce = active_cache[i++];
+			if (ce_stage(ce)) {
+				strbuf_addch(msg, '\t');
+				strbuf_addstr(msg, ce->name);
+				strbuf_addch(msg, '\n');
+				while (i < active_nr && !strcmp(ce->name,
+						active_cache[i]->name))
+					i++;
+			}
+		}
+		ret = 1;
+	}
+	free(oneline);
+
+	discard_cache();
+	if (read_cache() < 0) {
+		error("Cannot read the index");
+		return 2;
+	}
+
+	return ret;
+}
diff --git a/pick.h b/pick.h
new file mode 100644
index 0000000..7a74ad8
--- /dev/null
+++ b/pick.h
@@ -0,0 +1,13 @@
+#ifndef PICK_H
+#define PICK_H
+
+#include "commit.h"
+
+/* Pick flags: */
+#define PICK_REVERSE   1 /* pick the reverse changes ("revert") */
+#define PICK_ADD_NOTE  2 /* add note about original commit (unless conflict) */
+/* We don't need a PICK_QUIET. This is done by
+ *	setenv("GIT_MERGE_VERBOSITY", "0", 1); */
+extern int pick_commit(struct commit *commit, int mainline, int flags, struct strbuf *msg);
+
+#endif
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>

As "git merge" fast forwards if possible, it seems sensible to
have such a feature for "git cherry-pick" too, especially as it
could be used in git-rebase--interactive.sh.

Maybe this option could be made the default in the long run, with
another --no-ff option to disable this default behavior, but that
could make some scripts backward incompatible and/or that would
require testing if some GIT_AUTHOR_* environment variables are
set. So we don't do that for now.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-revert.c |   35 +++++++++++++++++++++++++++++++----
 1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 7488d4c..b78926f 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -9,6 +9,7 @@
 #include "revision.h"
 #include "rerere.h"
 #include "pick.h"
+#include "reset.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -31,13 +32,19 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_commit, mainline, signoff;
+static int edit, no_commit, mainline, signoff, ff_ok;
 static int flags;
 static struct commit *commit;
 static int allow_rerere_auto;
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
+static void die_if_ff_incompatible(int opt_set, const char *opt_name)
+{
+	if (opt_set)
+		die ("options --ff and %s are incompatible", opt_name);
+}
+
 static void parse_args(int argc, const char **argv)
 {
 	const char * const * usage_str =
@@ -51,6 +58,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_BIT('x', NULL, &flags, "append commit name when cherry-picking", PICK_ADD_NOTE),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
+		OPT_BOOLEAN(0, "ff", &ff_ok, "allow fast forward"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 		OPT_END(),
@@ -71,6 +79,12 @@ static void parse_args(int argc, const char **argv)
 	}
 	if (commit->object.type != OBJ_COMMIT)
 		die ("'%s' does not point to a commit", arg);
+	if (ff_ok) {
+		die_if_ff_incompatible(edit, "--edit");
+		die_if_ff_incompatible(flags & PICK_ADD_NOTE, "-x");
+		die_if_ff_incompatible(no_commit, "--no-commit");
+		die_if_ff_incompatible(signoff, "--signoff");
+	}
 }
 
 static char *get_encoding(const char *message)
@@ -191,7 +205,7 @@ static NORETURN void die_dirty_index(const char *me)
 	}
 }
 
-static int revert_or_cherry_pick(int argc, const char **argv)
+static int revert_or_cherry_pick(int argc, const char **argv, const char *prefix)
 {
 	const char *me;
 	struct strbuf msgbuf;
@@ -209,6 +223,19 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	if (!no_commit && index_differs_from("HEAD", 0))
 		die_dirty_index(me);
 
+	if (!(flags & PICK_REVERSE) && ff_ok && commit->parents) {
+		unsigned char head_sha1[20];
+		if (get_sha1("HEAD", head_sha1))
+			die("You do not have a valid HEAD.");
+		if (!hashcmp(commit->parents->item->object.sha1, head_sha1)) {
+			char *hex = sha1_to_hex(commit->object.sha1);
+			int res = reset(hex, prefix, HARD, 0, 0, 0, NULL);
+			if (!res)
+				fprintf(stderr, "Fast-forward to %s\n", hex);
+			return res;
+		}
+	}
+
 	if (!commit->buffer)
 		return error("Cannot get commit message for %s",
 				sha1_to_hex(commit->object.sha1));
@@ -270,11 +297,11 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	if (isatty(0))
 		edit = 1;
 	flags = PICK_REVERSE | PICK_ADD_NOTE;
-	return revert_or_cherry_pick(argc, argv);
+	return revert_or_cherry_pick(argc, argv, prefix);
 }
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
 	flags = 0;
-	return revert_or_cherry_pick(argc, argv);
+	return revert_or_cherry_pick(argc, argv, prefix);
 }
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 6/9] cherry-pick: add tests for new --ff option
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>


Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t3506-cherry-pick-ff.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 t/t3506-cherry-pick-ff.sh

diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh
new file mode 100755
index 0000000..21c0729
--- /dev/null
+++ b/t/t3506-cherry-pick-ff.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='test cherry-picking with --ff option'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo first > file1 &&
+	git add file1 &&
+	test_tick &&
+	git commit -m "first" &&
+	git tag first &&
+
+	git checkout -b other &&
+	echo second >> file1 &&
+	git add file1 &&
+	test_tick &&
+	git commit -m "second" &&
+	git tag second
+'
+
+test_expect_success 'cherry-pick using --ff fast forwards' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick --ff second &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify second)"
+'
+
+test_expect_success 'cherry-pick not using --ff does not fast forwards' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick second &&
+	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify second)"
+'
+
+test_done
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 7/9] Documentation: describe new cherry-pick --ff option
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>


Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-cherry-pick.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 78f4714..d71607a 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit
 
 SYNOPSIS
 --------
-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] <commit>
+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>
 
 DESCRIPTION
 -----------
@@ -70,6 +70,10 @@ effect to your index in a row.
 --signoff::
 	Add Signed-off-by line at the end of the commit message.
 
+--ff::
+	If the current HEAD is the same as the parent of the
+	cherry-pick'ed commit, then a fast forward to this commit will
+	be performed.
 
 Author
 ------
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 3/9] reset: refactor reseting in its own function
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>

This splits the reseting logic away from the argument parsing.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c |  138 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 74 insertions(+), 64 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 3569695..bf97931 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -235,68 +235,13 @@ static int update_heads(unsigned char *sha1)
 	return update_ref(msg, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 }
 
-int cmd_reset(int argc, const char **argv, const char *prefix)
+static int reset(const char *rev, const char *prefix,
+		 int reset_type, int quiet, int patch_mode,
+		 int argc, const char **argv)
 {
-	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0;
-	const char *rev = "HEAD";
-	unsigned char sha1[20];
 	struct commit *commit;
-	char *reflog_action;
-	const struct option options[] = {
-		OPT__QUIET(&quiet),
-		OPT_SET_INT(0, "mixed", &reset_type,
-						"reset HEAD and index", MIXED),
-		OPT_SET_INT(0, "soft", &reset_type, "reset only HEAD", SOFT),
-		OPT_SET_INT(0, "hard", &reset_type,
-				"reset HEAD, index and working tree", HARD),
-		OPT_SET_INT(0, "merge", &reset_type,
-				"reset HEAD, index and working tree", MERGE),
-		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
-		OPT_END()
-	};
-
-	git_config(git_default_config, NULL);
-
-	argc = parse_options(argc, argv, prefix, options, git_reset_usage,
-						PARSE_OPT_KEEP_DASHDASH);
-	reflog_action = args_to_str(argv);
-	setenv("GIT_REFLOG_ACTION", reflog_action, 0);
-
-	/*
-	 * Possible arguments are:
-	 *
-	 * git reset [-opts] <rev> <paths>...
-	 * git reset [-opts] <rev> -- <paths>...
-	 * git reset [-opts] -- <paths>...
-	 * git reset [-opts] <paths>...
-	 *
-	 * At this point, argv[i] points immediately after [-opts].
-	 */
-
-	if (i < argc) {
-		if (!strcmp(argv[i], "--")) {
-			i++; /* reset to HEAD, possibly with paths */
-		} else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
-			rev = argv[i];
-			i += 2;
-		}
-		/*
-		 * Otherwise, argv[i] could be either <rev> or <paths> and
-		 * has to be unambiguous.
-		 */
-		else if (!get_sha1(argv[i], sha1)) {
-			/*
-			 * Ok, argv[i] looks like a rev; it should not
-			 * be a filename.
-			 */
-			verify_non_filename(prefix, argv[i]);
-			rev = argv[i++];
-		} else {
-			/* Otherwise we treat this as a filename */
-			verify_filename(prefix, argv[i]);
-		}
-	}
+	unsigned char sha1[20];
+	int update_ref_status;
 
 	if (get_sha1(rev, sha1))
 		die("Failed to resolve '%s' as a valid ref.", rev);
@@ -309,19 +254,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (patch_mode) {
 		if (reset_type != NONE)
 			die("--patch is incompatible with --{hard,mixed,soft}");
-		return interactive_reset(rev, argv + i, prefix);
+		return interactive_reset(rev, argv, prefix);
 	}
 
 	/* git reset tree [--] paths... can be used to
 	 * load chosen paths from the tree into the index without
 	 * affecting the working tree nor HEAD. */
-	if (i < argc) {
+	if (argc > 0) {
 		if (reset_type == MIXED)
 			warning("--mixed option is deprecated with paths.");
 		else if (reset_type != NONE)
 			die("Cannot do %s reset with paths.",
 					reset_type_names[reset_type]);
-		return read_from_tree(prefix, argv + i, sha1,
+		return read_from_tree(prefix, argv, sha1,
 				quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
 	}
 	if (reset_type == NONE)
@@ -361,7 +306,72 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 
 	remove_branch_state();
 
+	return update_ref_status;
+}
+
+int cmd_reset(int argc, const char **argv, const char *prefix)
+{
+	int i = 0, reset_type = NONE, quiet = 0;
+	int patch_mode = 0;
+	const char *rev = "HEAD";
+	unsigned char sha1[20];
+	char *reflog_action;
+	const struct option options[] = {
+		OPT__QUIET(&quiet),
+		OPT_SET_INT(0, "mixed", &reset_type,
+						"reset HEAD and index", MIXED),
+		OPT_SET_INT(0, "soft", &reset_type, "reset only HEAD", SOFT),
+		OPT_SET_INT(0, "hard", &reset_type,
+				"reset HEAD, index and working tree", HARD),
+		OPT_SET_INT(0, "merge", &reset_type,
+				"reset HEAD, index and working tree", MERGE),
+		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
+		OPT_END()
+	};
+
+	git_config(git_default_config, NULL);
+
+	argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+						PARSE_OPT_KEEP_DASHDASH);
+	reflog_action = args_to_str(argv);
+	setenv("GIT_REFLOG_ACTION", reflog_action, 0);
 	free(reflog_action);
 
-	return update_ref_status;
+	/*
+	 * Possible arguments are:
+	 *
+	 * git reset [-opts] <rev> <paths>...
+	 * git reset [-opts] <rev> -- <paths>...
+	 * git reset [-opts] -- <paths>...
+	 * git reset [-opts] <paths>...
+	 *
+	 * At this point, argv[i] points immediately after [-opts].
+	 */
+
+	if (i < argc) {
+		if (!strcmp(argv[i], "--")) {
+			i++; /* reset to HEAD, possibly with paths */
+		} else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
+			rev = argv[i];
+			i += 2;
+		}
+		/*
+		 * Otherwise, argv[i] could be either <rev> or <paths> and
+		 * has to be unambiguous.
+		 */
+		else if (!get_sha1(argv[i], sha1)) {
+			/*
+			 * Ok, argv[i] looks like a rev; it should not
+			 * be a filename.
+			 */
+			verify_non_filename(prefix, argv[i]);
+			rev = argv[i++];
+		} else {
+			/* Otherwise we treat this as a filename */
+			verify_filename(prefix, argv[i]);
+		}
+	}
+
+	return reset(rev, prefix, reset_type, quiet, patch_mode,
+		     argc - i, argv + i);
 }
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 2/9] reset: refactor updating heads into a static function
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>


Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c |   43 +++++++++++++++++++++++++++----------------
 1 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 0f5022e..3569695 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -211,15 +211,38 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
 		warning("Reflog action message too long: %.*s...", 50, buf);
 }
 
+/*
+ * Any resets update HEAD to the head being switched to,
+ * saving the previous head in ORIG_HEAD before.
+ */
+static int update_heads(unsigned char *sha1)
+{
+	unsigned char sha1_orig[20], *orig = NULL,
+		sha1_old_orig[20], *old_orig = NULL;
+	char msg[1024];
+
+	if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+		old_orig = sha1_old_orig;
+	if (!get_sha1("HEAD", sha1_orig)) {
+		orig = sha1_orig;
+		prepend_reflog_action("updating ORIG_HEAD", msg, sizeof(msg));
+		update_ref(msg, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+	}
+	else if (old_orig)
+		delete_ref("ORIG_HEAD", old_orig, 0);
+	prepend_reflog_action("updating HEAD", msg, sizeof(msg));
+
+	return update_ref(msg, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
 	int patch_mode = 0;
 	const char *rev = "HEAD";
-	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
-				*old_orig = NULL, sha1_old_orig[20];
+	unsigned char sha1[20];
 	struct commit *commit;
-	char *reflog_action, msg[1024];
+	char *reflog_action;
 	const struct option options[] = {
 		OPT__QUIET(&quiet),
 		OPT_SET_INT(0, "mixed", &reset_type,
@@ -321,19 +344,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	else if (reset_index_file(sha1, reset_type, quiet))
 		die("Could not reset index file to revision '%s'.", rev);
 
-	/* Any resets update HEAD to the head being switched to,
-	 * saving the previous head in ORIG_HEAD before. */
-	if (!get_sha1("ORIG_HEAD", sha1_old_orig))
-		old_orig = sha1_old_orig;
-	if (!get_sha1("HEAD", sha1_orig)) {
-		orig = sha1_orig;
-		prepend_reflog_action("updating ORIG_HEAD", msg, sizeof(msg));
-		update_ref(msg, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
-	}
-	else if (old_orig)
-		delete_ref("ORIG_HEAD", old_orig, 0);
-	prepend_reflog_action("updating HEAD", msg, sizeof(msg));
-	update_ref_status = update_ref(msg, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+	update_ref_status = update_heads(sha1);
 
 	switch (reset_type) {
 	case HARD:
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 9/9] merge: use new "reset" function instead of running "git read-tree"
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>

This simplifies "git merge" code and make it more efficient in some
cases.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-merge.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 3aaec7b..de2343a 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -25,6 +25,7 @@
 #include "help.h"
 #include "merge-recursive.h"
 #include "resolve-undo.h"
+#include "reset.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -231,24 +232,14 @@ static void save_state(void)
 		die("not a valid object: %s", buffer.buf);
 }
 
-static void reset_hard(unsigned const char *sha1, int verbose)
+static void reset_hard(const char *prefix, unsigned const char *sha1, int verbose)
 {
-	int i = 0;
-	const char *args[6];
-
-	args[i++] = "read-tree";
-	if (verbose)
-		args[i++] = "-v";
-	args[i++] = "--reset";
-	args[i++] = "-u";
-	args[i++] = sha1_to_hex(sha1);
-	args[i] = NULL;
-
-	if (run_command_v_opt(args, RUN_GIT_CMD))
-		die("read-tree failed");
+	int res = reset(sha1_to_hex(sha1), prefix, HARD, !verbose, 0, 0, NULL);
+	if (res)
+		die("hard reset failed");
 }
 
-static void restore_state(void)
+static void restore_state(const char *prefix)
 {
 	struct strbuf sb = STRBUF_INIT;
 	const char *args[] = { "stash", "apply", NULL, NULL };
@@ -256,7 +247,7 @@ static void restore_state(void)
 	if (is_null_sha1(stash))
 		return;
 
-	reset_hard(head, 1);
+	reset_hard(prefix, head, 1);
 
 	args[2] = sha1_to_hex(stash);
 
@@ -970,7 +961,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die("%s - not something we can merge", argv[0]);
 		update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
 				DIE_ON_ERR);
-		reset_hard(remote_head->sha1, 0);
+		reset_hard(prefix, remote_head->sha1, 0);
 		return 0;
 	} else {
 		struct strbuf msg = STRBUF_INIT;
@@ -1167,7 +1158,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		int ret;
 		if (i) {
 			printf("Rewinding the tree to pristine...\n");
-			restore_state();
+			restore_state(prefix);
 		}
 		if (use_strategies_nr != 1)
 			printf("Trying merge strategy %s...\n",
@@ -1228,7 +1219,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * it up.
 	 */
 	if (!best_strategy) {
-		restore_state();
+		restore_state(prefix);
 		if (use_strategies_nr > 1)
 			fprintf(stderr,
 				"No merge strategy handled the merge.\n");
@@ -1240,7 +1231,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		; /* We already have its result in the working tree. */
 	else {
 		printf("Rewinding the tree to pristine...\n");
-		restore_state();
+		restore_state(prefix);
 		printf("Using the %s to prepare resolving by hand.\n",
 			best_strategy);
 		try_merge_strategy(best_strategy, common, head_arg);
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* [PATCH 8/9] rebase -i: use new --ff cherry-pick option
From: Christian Couder @ 2010-02-05 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>

This simplifies rebase -i a little bit.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-rebase--interactive.sh |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c2f6089..6b224a6 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -222,8 +222,8 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
-	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
+	ff=--ff
+	case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
 		pick_one_preserving_merges "$@" && return
@@ -232,16 +232,7 @@ pick_one () {
 		output git cherry-pick "$@"
 		return
 	fi
-	parent_sha1=$(git rev-parse --verify $sha1^) ||
-		die "Could not get the parent of $sha1"
-	current_sha1=$(git rev-parse --verify HEAD)
-	if test -z "$no_ff" && test "$current_sha1" = "$parent_sha1"
-	then
-		output git reset --hard $sha1
-		output warn Fast-forward to $(git rev-parse --short $sha1)
-	else
-		output git cherry-pick "$@"
-	fi
+	output git cherry-pick $ff "$@"
 }
 
 pick_one_preserving_merges () {
-- 
1.6.6.1.557.g77031

^ permalink raw reply related

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Jan Engelhardt @ 2010-02-05 23:17 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LSU.2.01.1002052351060.30204@obet.zrqbmnf.qr>

On Saturday 2010-02-06 00:09, Jan Engelhardt wrote:

>What works is (3).
>
>(3)	$ git grep -- -- '->cnt' net/ipv4/netfilter/

No, I spoke too soon. This command will search for --, not ->cnt.
So git cannot search for patterns starting with a dash at all,
as I see it. This is getting fun..

>As I said, it's a mess. And I know not whether any code can convince
>the "but we need to watch compatibility"-sayers, because this would
>definitely be a flag change.

^ permalink raw reply

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Santi Béjar @ 2010-02-05 23:27 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.01.1002060009430.30204@obet.zrqbmnf.qr>

On Sat, Feb 6, 2010 at 12:17 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
> On Saturday 2010-02-06 00:09, Jan Engelhardt wrote:
>
>>What works is (3).
>>
>>(3)    $ git grep -- -- '->cnt' net/ipv4/netfilter/
>
> No, I spoke too soon. This command will search for --, not ->cnt.
> So git cannot search for patterns starting with a dash at all,
> as I see it. This is getting fun..

You should use -e:

     -e
           The next parameter is the pattern. This option has to be
used for patterns starting with - and should be used in scripts
passing user input to grep.

The working command is:

$ git grep -e '->cnt' net/ipv4/netfilter/

Although there is not such pattern in net/ipv4/netfilter, maybe you
wanted '->counters' :-)

HTH,
Santi

^ permalink raw reply

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Junio C Hamano @ 2010-02-05 23:31 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.01.1002052351060.30204@obet.zrqbmnf.qr>

Jan Engelhardt <jengelh@medozas.de> writes:

> Just about now I wanted to grep for accesses of a particular struct 
> member. Needless to say that it was not a very amusing experience.
> I would expect that (1) probably fails:
>
> (1)	$ git grep '->cnt' net/ipv4/netfilter/
> 	error: unknown switch `>'
>
> So far so good, seems reasonable and matches what I would expect from 
> most other userspace tools. So let's add -- to terminate the option 
> list:

Also you can say "grep -e '->cnt'".  Not just "git grep" but regular grep
understands this, too.

> (2)	$ git grep -- '->cnt' net/ipv4/netfilter/
> 	fatal: bad flag '->cnt' used after filename
>
> *bzzt*.

This indeed is bzzt, especially if you had a file called "./->cnt" in the
work tree.  That would mean that you cannot tell the command to look for a
pattern in the work tree.

But because you are not giving anything before "--", that "git grep" is
not looking for anything.  Indeed, (2) is a user error.  If you try this:

        $ git grep a -- '->cnt' net/ipv4/netfilter/

does do what the command line specifies:  Look for a pattern "a" in files
whose names match given pathspecs ('->cnt' or 'net/ipv4/netfilter/').

> What works is (3).
>
> (3)	$ git grep -- -- '->cnt' net/ipv4/netfilter/

Huh?  Now I am lost.  Weren't you looking for a pattern "->cnt"?

And if this command looks for and finds the string '->cnt' in files whose
path match net/ipv4/netfilter/ pathspec, I would say it _is_ a bug.

The command line looks for "--" (the first one) as a pattern, and
interprets the second "--" as your attempt to tell git that '->cnt' is not
an option but is a pathspec.  So it looks for a pattern "--" in files
whose names match given pathspecs( again '->cnt' or 'net/ipv4/netfilter/').

> But it almost looks like Morse code.

Indeed.  But did (3) really work?  I tried it myself in a copy of the
kernel repository, and it found lines that contain '--' in files whose
names match net/ipv4/netfilter/ pathspec, as my copy of the kernel source
does not have a file '->cnt' at all.

^ permalink raw reply

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Junio C Hamano @ 2010-02-05 23:34 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.01.1002060009430.30204@obet.zrqbmnf.qr>

Jan Engelhardt <jengelh@medozas.de> writes:

> On Saturday 2010-02-06 00:09, Jan Engelhardt wrote:
>
>>What works is (3).
>>
>>(3)	$ git grep -- -- '->cnt' net/ipv4/netfilter/
>
> No, I spoke too soon. This command will search for --, not ->cnt.
> So git cannot search for patterns starting with a dash at all,
> as I see it. This is getting fun..
>
>>As I said, it's a mess. And I know not whether any code can convince
>>the "but we need to watch compatibility"-sayers, because this would
>>definitely be a flag change.

I guess our mails crossed.  You don't have to worry about "flag change",
as I don't think there is any change necessary.

You just need to learn:

 (1) "-e" can come before a string to tell git: "this might look like
     an option to you but it isn't; it is what I am looking for"; and

 (2) Everything that follows "--" are pathspecs, not revs nor options.

And all is well.

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Jakub Narebski @ 2010-02-05 23:44 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <1265300338-25021-2-git-send-email-giuseppe.bilotta@gmail.com>

On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:

BTW. shouldn't this series be marked as RFC?

> Introduce support for notes by collecting them when creating commit
> lists. The list of noterefs to look into is configurable, and can be a(n
> array of) refspec(s), which will be looked for in the refs/notes
> namespace.
> 
> The feature is disabled by default because it's presently not very
> efficient (one extra git call per configured refspec, plus two extra git
> calls per commit per noteref).

Signoff?
> ---
>  gitweb/gitweb.perl |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 62 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index d0c3ff2..9ba5815 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -411,6 +411,22 @@ our %feature = (
>  		'override' => 0,
>  		'default' => [16]},
>  
> +	# Notes support. When this feature is enabled, the presence of notes
> +	# for any commit is signaled, and the note content is made available
> +	# in a way appropriate for the current view.
> +	# Set this to '*' to enable all notes namespace, or to a shell-glob
> +	# specification to enable specific namespaces only.

It is not obvious from this description that you can provide _list_ of
notes namespaces (or list of shell-globs).

> +
> +	# To enable system wide have in $GITWEB_CONFIG
> +	# $feature{'notes'}{'default'} = ['*'];
> +	# To have project specific config enable override in $GITWEB_CONFIG
> +	# $feature{'notes'}{'override'} = 1;
> +	# and in project config gitweb.notes = namespace;

How you can provide list of notes here?  Is overriding limited to single
name or shell-glob?

See feature_snapshot for example implementation.

> +	'notes' => {
> +		'sub' => \&feature_notes,
> +		'override' => 0,
> +		'default' => []},
> +
>  	# Avatar support. When this feature is enabled, views such as
>  	# shortlog or commit will display an avatar associated with
>  	# the email of the committer(s) and/or author(s).
> @@ -513,6 +529,16 @@ sub feature_patches {
>  	return ($_[0]);
>  }
>  
> +sub feature_notes {
> +	my @val = (git_get_project_config('notes'));
> +
> +	if (@val) {
> +		return @val;
> +	}
> +
> +	return @_;
> +}

First, this I think limits overriding in repository config to single value.

Second, perhaps it is time to refactor all those similar feature_xxx
subroutines (just a possible suggestion)?

> +
>  sub feature_avatar {
>  	my @val = (git_get_project_config('avatar'));
>  
> @@ -2786,10 +2812,30 @@ sub parse_commit {
>  	return %co;
>  }
>  
> +# return all refs matching refs/notes/<globspecs> where the globspecs
> +# are taken from the notes feature content.
> +sub get_note_refs {
> +	my @globs = gitweb_get_feature('notes');
> +	my @note_refs = ();
> +	foreach my $glob (@globs) {
> +		if (open my $fd, '-|', git_cmd(), 'for-each-ref',
> +		    '--format=%(refname)', "refs/notes/$glob") {

   		open my $fd, '-|', git_cmd(), 'for-each-ref',
   			'--format=%(refname)', "refs/notes/$glob"
   			or return;

would reduce indent level a bit.

> +			while (<$fd>) {
> +				chomp;
> +				push @note_refs, $_ if $_;
> +			}

Why not simply

   		chomp(@note_refs = <$fd>);

> +			close $fd;
> +		}
> +	}
> +	return @note_refs;
> +}
> +
>  sub parse_commits {
>  	my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
>  	my @cos;
>  
> +	my @note_refs = get_note_refs();
> +
>  	$maxcount ||= 1;
>  	$skip ||= 0;
>  
> @@ -2807,6 +2853,22 @@ sub parse_commits {
>  		or die_error(500, "Open git-rev-list failed");
>  	while (my $line = <$fd>) {
>  		my %co = parse_commit_text($line);
> +		my %notes = () ;
> +		foreach my $note_ref (@note_refs) {
> +			my $obj = "$note_ref:$co{'id'}";
> +			if (open my $fd, '-|', git_cmd(), 'rev-parse',
> +				'--verify', '-q', $obj) {
> +				my $exists = <$fd>;
> +				close $fd;
> +				if (defined $exists) {
> +					if (open $fd, '-|', git_cmd(), 'show', $obj) {
> +						$notes{$note_ref} = scalar <$fd>;
> +						close $fd;
> +					}
> +				}
> +			}
> +		}

First, there are '--batch' and '--batch-check' options to git-cat-file.
With these I think you can get all notes with just single git command,
although using it is a bit complicated (requires open2 from IPC::Open2
for bidi communication).

Second, if not using 'git cat-file --batch', perhaps it would be easier
to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and
parse its output to check for which commits/objects there are notes
available, and only then call 'git show' (or reuse connection to
'git cat-file --batch').

The second solution, with a bit more work, could work even in presence
of fan-out schemes for notes, I think.

> +		$co{'notes'} = \%notes;
>  		push @cos, \%co;
>  	}
>  	close $fd;
> -- 
> 1.7.0.rc1.193.ge8618
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 0/9] add --ff option to cherry-pick
From: Junio C Hamano @ 2010-02-05 23:45 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231028.3689.12228.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Changes since the previous series are the following:

Could you please briefly describe what good does "adding -ff option to
cherry-pick" do in the first place?

IOW, help reviewers to convince themselves that it is worth allocating a
block of time to read through a 9 patch series.  That is what the cover
letter is for.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected
From: Tay Ray Chuan @ 2010-02-05 23:50 UTC (permalink / raw)
  To: Larry D'Anna; +Cc: git
In-Reply-To: <cea7001d626116d76d885e24152d68cf41eca4dd.1265402797.git.larry@elder-gods.org>

Hi,

instead of:

> diff --git a/transport.c b/transport.c
> index 00d986c..b41e1dc 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -558,8 +558,10 @@ static int fetch_refs_via_pack(struct transport *transport,
>  	return (refs ? 0 : -1);
>  }
>  
> -static int push_had_errors(struct ref *ref)
> +static int push_had_errors(struct ref *ref, int flags)
>  {
> +	if (flags & TRANSPORT_PUSH_DRY_RUN && flags & TRANSPORT_PUSH_PORCELAIN)
> +		return 0;
>  	for (; ref; ref = ref->next) {
>  		switch (ref->status) {
>  		case REF_STATUS_NONE:

and:

> @@ -1052,7 +1055,7 @@ int transport_push(struct transport *transport,
>  			flags & TRANSPORT_PUSH_FORCE);
>  
>  		ret = transport->push_refs(transport, remote_refs, flags);
> -		err = push_had_errors(remote_refs);
> +		err = push_had_errors(remote_refs, flags);
>  
>  		ret |= err;
>  

why not:

-->8--
@@ -1049,7 +1052,7 @@ int transport_push(struct transport *transport,
			flags & TRANSPORT_PUSH_FORCE);

		ret = transport->push_refs(transport, remote_refs, flags);
-		err = push_had_errors(remote_refs);
+		err = (pretend && porcelain) ? 0 : push_had_errors(remote_refs);

		ret |= err;

-->8--

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* upload-pack timing issue on windows?
From: Erik Faye-Lund @ 2010-02-05 23:51 UTC (permalink / raw)
  To: Git Mailing List, msysGit

As some of you might know, I've been working on porting git-daemon to
Windows for quite some time now. As it stands now, there's really only
one known issue that is blocking on my end here:

Something weird happens *sometimes* when upload-pack is exiting,
leading to a client dying with a "fatal: read error: Invalid
argument\nfatal: early EOF"-error. If I place a sleep(1) at some place
after exiting the while(1)-loop in create_pack() in upload-pack.c, the
symptom goes away. create_pack() contains some async-code, but this
doesn't seem to be triggered in my minimal case at all. I've tried
flushing stdout and stderr explicitly, no luck.

How often the issue triggers seems to depend on two things, the size
of the repo and the connection speed. If I clone from localhost, I
can't get it to trigger at all. If the repo is of some size, it
triggers rarely. However if I have a repo with only one commit, it
seems to trigger every single time for me.

I've noticed that one of the last things that happens is a call to
poll with nfds=1. This triggers a special case in our poll-emulation
on Windows; but removing that special case hasn't given me any
positive results.

Does anyone have a hunch about what might trigger this issue?

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [PATCH 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Junio C Hamano @ 2010-02-05 23:57 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
	Stephan Beyer, Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231112.3689.67634.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> As "git merge" fast forwards if possible, it seems sensible to
> have such a feature for "git cherry-pick" too, especially as it
> could be used in git-rebase--interactive.sh.


> +	if (!(flags & PICK_REVERSE) && ff_ok && commit->parents) {
> +		unsigned char head_sha1[20];
> +		if (get_sha1("HEAD", head_sha1))
> +			die("You do not have a valid HEAD.");
> +		if (!hashcmp(commit->parents->item->object.sha1, head_sha1)) {
> +			char *hex = sha1_to_hex(commit->object.sha1);

With this check, you are saying:

	If we are cherry-picking commit $C, and if the current HEAD is
        the first parent of $C, then just reset to $C instead of running a
        cherry-pick.

I didn't check if you have access to commit->parents->item->object.sha1 at
this point in the codepath, though (e.g. have you parsed "commit" yet?).

If the goal is to make untouched 'pick' in rebase-i to fast forward
without actually running cherry-picking, perhaps it is much cleaner to do
this kind of comparison in the caller of cherry-pick (i.e. rebase-i and
anything that runs cherry-pick)?

The whole series is titled as if "cherry-pick --ff" is the primary goal,
but I am puzzled why earlier patches in the series were necessary for this
change.

^ permalink raw reply

* Re: [PATCH 9/9] merge: use new "reset" function instead of running "git read-tree"
From: Junio C Hamano @ 2010-02-06  0:03 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <20100205231112.3689.34673.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> This simplifies "git merge" code and make it more efficient in some
> cases.

I vaguely recall somebody (perhaps it was you) tried to do something like
this before to drive unpack_trees() inside the main process, broke the
program rather badly, and then we ended up keeping read-tree invocation
external to the process.  Am I misremembering things?

If not, could you describe how is this round different from the old one?

^ permalink raw reply

* Re: [PATCH 2/4] gitweb: show notes in shortlog view
From: Jakub Narebski @ 2010-02-06  0:18 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <1265300338-25021-3-git-send-email-giuseppe.bilotta@gmail.com>

On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:

> Subject: [PATCH 2/4] gitweb: show notes in shortlog view

Is it RFC?

Why it is only for 'shortlog' view, and not also for 'history' which is
also shortlog-like view?  Or is there reason why it is not present for
'history' view?

> The presence of the note is shown by a small icon, hovering on which
> reveals the actual note content.

Signoff?

> ---
>  gitweb/gitweb.css  |   29 +++++++++++++++++++++++++++++
>  gitweb/gitweb.perl |   30 +++++++++++++++++++++++++++++-
>  2 files changed, 58 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 50067f2..7d1836b 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -572,3 +572,32 @@ span.match {
>  div.binary {
>  	font-style: italic;
>  }
> +
> +span.notes {
> +	float:right;
> +	position:relative;
> +}
> +
> +span.notes span.note-container:before {
> +	content: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAgMAAAC5YVYYAAAAAXNSR0IArs4c6QAAAAlQTFRFAABnybuD//+t5rXizQAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAAGElEQVQI12MIDWXIWglDQHYIQ1YAQ6gDAFWPBrAKFe0fAAAAAElFTkSuQmCC');
> +}

Not all web browsers support ':before' pseudo-element, and 'content'
(pseudo-)property.

Not all web browsers support 'data:' URI schema in CSS; also such image
cannot be cached (on the other hand it doesn't require extra TCP 
connection on first access, and CSS file is cached anyway).

On the other hand adding extra images to gitweb would probably require
additional (yet another) build time parameter to tell where static
images are (besides logo and favicon).

So perhaps it is good solution, at least for a first attempt.

[...]
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9ba5815..1701ed1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1628,6 +1628,33 @@ sub format_subject_html {
>  	}
>  }
>  
> +# display notes next to a commit
> +sub format_notes_html {
> +	my %notes = %{$_[0]};

Why not use 'my $notes = shift;', and later '%$notes'?

> +	my $ret = "";

Perhaps $return or $result would be a better name, to better distinguish
it from visually similar $ref (see $ref vs $res);

> +	while (my ($ref, $text) = each %notes) {
> +		# remove 'refs/notes/' and an optional final s
> +		$ref =~ s/^refs\/notes\///;

You can use different delimiter than / to avoid 'leaning toothpick'
syndrome, e.g.: $ref =~ s!^refs/notes/!!;

> +		$ref =~ s/s$//;
> +
> +		# double markup is needed to allow pure CSS cross-browser 'popup'
> +		# of the note
> +		$ret .= "<span title='$ref' class='note-container $ref'>";
> +		$ret .= "<span title='$ref' class='note $ref'>";
> +		foreach my $line (split /\n/, $text) {
> +			$ret .= esc_html($line) . "<br/>";

Probably would want

   			$ret .= esc_html($line) . "<br/>\n";

here.  Or do we want single string here?


Also, do you want/need final <br>?  If not, perhaps

   		join("<br/>", map { esc_html($_) } split(/\n/, $text);

would be a better solution (you can always add final "<br/>" later)?

> +		}
> +		$ret .= "</span></span>";
> +	}
> +	if ($ret) {
> +		return "<span class='notes'>$ret</span>";
> +	} else {
> +		return $ret;
> +	}
> +
> +
> +}
> +
>  # Rather than recomputing the url for an email multiple times, we cache it
>  # after the first hit. This gives a visible benefit in views where the avatar
>  # for the same email is used repeatedly (e.g. shortlog).
> @@ -4595,6 +4622,7 @@ sub git_shortlog_body {
>  		my %co = %{$commitlist->[$i]};
>  		my $commit = $co{'id'};
>  		my $ref = format_ref_marker($refs, $commit);
> +		my $notes = format_notes_html($co{'notes'});
>  		if ($alternate) {
>  			print "<tr class=\"dark\">\n";
>  		} else {
> @@ -4605,7 +4633,7 @@ sub git_shortlog_body {
>  		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
>  		      format_author_html('td', \%co, 10) . "<td>";
>  		print format_subject_html($co{'title'}, $co{'title_short'},
> -		                          href(action=>"commit", hash=>$commit), $ref);
> +		                          href(action=>"commit", hash=>$commit), $ref . $notes);
>  		print "</td>\n" .
>  		      "<td class=\"link\">" .
>  		      $cgi->a({-href => href(action=>"commit", hash=>$commit)}, "commit") . " | " .

Nice.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Junio C Hamano @ 2010-02-06  0:21 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <7vpr4jnsm9.fsf@alter.siamese.dyndns.org>

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

> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> As "git merge" fast forwards if possible, it seems sensible to
>> have such a feature for "git cherry-pick" too, especially as it
>> could be used in git-rebase--interactive.sh.

One more thing, in the same part of the code:

>> +	if (!(flags & PICK_REVERSE) && ff_ok && commit->parents) {
>> +		unsigned char head_sha1[20];
>> +		if (get_sha1("HEAD", head_sha1))
>> +			die("You do not have a valid HEAD.");
>> +		if (!hashcmp(commit->parents->item->object.sha1, head_sha1)) {
>> +			char *hex = sha1_to_hex(commit->object.sha1);

Is there a need to check commit->parents->next?

Should this code work the same way if the commit being cherry-picked is a
merge?  Should "-m <parent-num>" option affect this codepath in any way?

^ 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