Git development
 help / color / mirror / Atom feed
* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Junio C Hamano @ 2009-01-28  8:22 UTC (permalink / raw)
  To: Jeff King
  Cc: Shawn O. Pearce, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <7vfxj3vos2.fsf@gitster.siamese.dyndns.org>

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

> Jeff King <peff@peff.net> writes:
>
>> But in other cases, it silently gives you the wrong answer.  For
>> example, consider a history like:
>>
>>        C--D
>>       /
>>   A--B
>>       \
>>        E--F
>>
>> now let's suppose I have everything except 'E'. If I ask for
>>
>>   git rev-list F..D
>>
>> then it will not realize that A and B are uninteresting, and I will get
>> A-B-C-D. I think it is much better for git to complain loudly that it
>> could not compute the correct answer.
>
> Fair enough.  I think we can resurrect the conditional and the traversal
> option revs->ignore_missing_negative only for this hunk in my [2/2] patch
> to support that use case.
> ...

Nah, I take that back.

Even the original code does not consider this case an error.

If you really want that, the revision machinery needs major surgery, as I
already noted that the design of mark_parents_uninteresting() wants to
treat a missing uninteresting commit as a non-error event.

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Jeff King @ 2009-01-28  8:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Shawn O. Pearce, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <7vfxj3vos2.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 28, 2009 at 12:05:33AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > But in other cases, it silently gives you the wrong answer.  For
> > example, consider a history like:
> >
> >        C--D
> >       /
> >   A--B
> >       \
> >        E--F
> >
> > now let's suppose I have everything except 'E'. If I ask for
> >
> >   git rev-list F..D
> >
> > then it will not realize that A and B are uninteresting, and I will get
> > A-B-C-D. I think it is much better for git to complain loudly that it
> > could not compute the correct answer.
> 
> Fair enough.  I think we can resurrect the conditional and the traversal
> option revs->ignore_missing_negative only for this hunk in my [2/2] patch
> to support that use case.
> [ hunk handling parent lookup]

Don't the other changes have similar parallel use cases? [2/2] also deals
with tag lookup. Wouldn't you also expect, if you had a tag "T" pointing
to "E" in the above scenario that "git rev-list T..D" would barf? I
really think you don't want to ignore missing negations _ever_ unless
the caller knows that such a miss is really only about optimization and
not correctness.

Side note:

As you described, we expect to reach this situation from a partial
transfer. Which means that you don't actually have a _ref_ for "T" (or
"F"). So it is unlikely to come up in normal use (you would have to
manually specify the sha1 of a broken portion of the graph).

But what is more important is that your repository _is_ corrupted, I
think we are losing an important method by which the user finds out. Git
is usually very good at informing you of a problem in the repo early,
and I think unconditionally ignoring missing objects would lose that.

-Peff

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Junio C Hamano @ 2009-01-28  8:05 UTC (permalink / raw)
  To: Jeff King
  Cc: Shawn O. Pearce, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <20090128075515.GA1133@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> But in other cases, it silently gives you the wrong answer.  For
> example, consider a history like:
>
>        C--D
>       /
>   A--B
>       \
>        E--F
>
> now let's suppose I have everything except 'E'. If I ask for
>
>   git rev-list F..D
>
> then it will not realize that A and B are uninteresting, and I will get
> A-B-C-D. I think it is much better for git to complain loudly that it
> could not compute the correct answer.

Fair enough.  I think we can resurrect the conditional and the traversal
option revs->ignore_missing_negative only for this hunk in my [2/2] patch
to support that use case.

@@ -480,7 +483,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
 			struct commit *p = parent->item;
 			parent = parent->next;
 			if (parse_commit(p) < 0)
-				return -1;
+				continue;
 			p->object.flags |= UNINTERESTING;
 			if (p->parents)
 				mark_parents_uninteresting(p);

^ permalink raw reply

* Re: [PATCH] mergetool merge/skip/abort at prompt
From: David Aguilar @ 2009-01-28  8:04 UTC (permalink / raw)
  To: Caleb Cushing; +Cc: Junio C Hamano, Charles Bailey, git, Nanako Shiraishi
In-Reply-To: <81bfc67a0901272256t726bf206k351bb6c8b2778bd5@mail.gmail.com>

Hi Caleb

On Tue, Jan 27, 2009 at 10:56 PM, Caleb Cushing <xenoterracide@gmail.com> wrote:
> previously git mergetool when run with prompt only allowed the user to continue
> merging. This changes git mergetool to allow the option of skipping a file or
> aborting, and includes an addtional key to explicitly select merge.
>
> Signed-off-by: Caleb Cushing <xenoterracide@gmail.com>
> ---
>  git-mergetool.sh |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 00e1337..575fbb2 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -177,8 +177,24 @@ merge_file () {
>     describe_file "$local_mode" "local" "$LOCAL"
>     describe_file "$remote_mode" "remote" "$REMOTE"
>     if "$prompt" = true; then
> -       printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
> -       read ans
> +       while true; do
> +           printf "Use (m)erge file or (s)kip file, or (a)bort? (%s): " \


I really like the feature you added here.  I'm sorry to bikeshed on
this conversation, but after trying it I have one tiny suggestion.

Right now the prompt looks like this with your patch:

"""
Merging the files: foo bar baz

Normal merge conflict for 'foo':
  {local}: created
  {remote}: created
Use (m)erge file or (s)kip file, or (a)bort? (xxdiff): m
"""

do you think
"Use merge file or skip file, or abort?"
might be better expressed as:

"""
Merging: foo bar baz

Normal merge conflict for 'foo':
  {local}: created
  {remote}: created
(m)erge, (s)kip, or (q)uit? (xxdiff): m
"""?

I realize that your patch only touches the last line of the prompt
(and not the introductory "Merging the files:" line) so if you agree
then maybe I can throw a patch together for the introductory line.

Also, my example has quit instead of abort for two reasons (the first
one is silly)
1. skip rhymes with quit, so it reads very nicely out loud
2. consistency with git add --interactive
3. less typos (q and s are diagonal on qwerty, s and a are adjacent)
(okay, that last one is silly too)

Some might also mis-associate 'abort' with meaning "abort the merge."

slightly off-topic:
If we're looking at cleaning up mergetool a bit would you all mind a
separate patch to convert it to using hard tabs throughout, just like
git-rebase.sh?


> +           "$merge_tool"
> +           read ans
> +           case "$ans" in
> +               [mM]*|"")
> +                   break
> +                   ;;
> +               [sS]*)
> +                   cleanup_temp_files
> +                   return 0
> +                   ;;
> +               [aA]*)
> +                   cleanup_temp_files
> +                   exit 0
> +                   ;;
> +           esac
> +       done
>     fi
>
>     case "$merge_tool" in
> --
> 1.6.1.1
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
    David

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Jeff King @ 2009-01-28  7:55 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Shawn O. Pearce, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <7vd4e7x5ov.fsf@gitster.siamese.dyndns.org>

On Tue, Jan 27, 2009 at 11:14:56PM -0800, Junio C Hamano wrote:

> I've been toying with an idea for an alternative solution, and need
> somebody competent to bounce it around with.

Well, unfortunately for you, you are stuck with me. ;P

> Here is my work in progress.  It introduces "ignore-missing-negative"
> option to the revision traversal machinery, and squelches the places we
> currently complain loudly and die when we expect an object to be
> available, when the color we are going to paint the object with is
> UNINTERESTING.
> 
> I have a mild suspicion that it may even be the right thing to ignore them
> unconditionally, and it might even match the intention of Linus's original
> code.  That would make many hunks in this patch much simpler.

I'm not sure it is a good idea to do so unconditionally. In the case of
negatives for transferring files, a missed negative is simply a missed
opportunity for optimizing the resulting pack.

But in other cases, it silently gives you the wrong answer.  For
example, consider a history like:

       C--D
      /
  A--B
      \
       E--F

now let's suppose I have everything except 'E'. If I ask for

  git rev-list F..D

then it will not realize that A and B are uninteresting, and I will get
A-B-C-D. I think it is much better for git to complain loudly that it
could not compute the correct answer.

Am I understanding the issue correctly?

-Peff

^ permalink raw reply

* Re: [RFC/PATCH 0/3] fix "Funny: git -p submodule summary"
From: Junio C Hamano @ 2009-01-28  7:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128073059.GD19165@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Jan 27, 2009 at 05:31:02PM +0100, Johannes Schindelin wrote:
>
>> I like the patch series, well designed and concise (especially with the 
>> fixes Hannes proposed).

The series looks quite sane and clean.  Will queue once I finish wrapping
up the push with ".have" thing.

^ permalink raw reply

* [PATCH 1/2] send-pack: do not send unknown object name from ".have" to pack-objects
From: Junio C Hamano @ 2009-01-28  7:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Linus Torvalds, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <7vvdrzvpwd.fsf@gitster.siamese.dyndns.org>

v1.6.1 introduced ".have" extension to the protocol to allow the receiving
side to advertise objects that are reachable from refs in the repositories
it borrows from.  This was meant to be used by the sending side to avoid
sending such objects; they are already available through the alternates
mechanism.

The client side implementation in v1.6.1, which was introduced with
40c155f (push: prepare sender to receive extended ref information from the
receiver, 2008-09-09) aka v1.6.1-rc1~203^2~1, were faulty in that it did
not consider the possiblity that the repository receiver borrows from
might have objects it does not know about.

This fixes it by refraining from passing missing commits to underlying
pack-objects.  Revision machinery may need to be tightened further to
treat missing uninteresting objects as non-error events, but this is an
obvious and safe fix for a maintenance release that is almost good enough.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This lost the "NEEDSWORK" comment, as it turns out that revision
   traversal machinery mostly ignores missing uninteresting objects as
   non-error events except for a few corner cases, which we can tighten
   separately.

 builtin-send-pack.c        |   43 +++++++++---------
 t/t5519-push-alternates.sh |  106 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+), 22 deletions(-)
 create mode 100755 t/t5519-push-alternates.sh

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index a9fdbf9..d65d019 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -15,6 +15,20 @@ static struct send_pack_args args = {
 	/* .receivepack = */ "git-receive-pack",
 };
 
+static int feed_object(const unsigned char *sha1, int fd, int negative)
+{
+	char buf[42];
+
+	if (negative && !has_sha1_file(sha1))
+		return 1;
+
+	memcpy(buf + negative, sha1_to_hex(sha1), 40);
+	if (negative)
+		buf[0] = '^';
+	buf[40 + negative] = '\n';
+	return write_or_whine(fd, buf, 41 + negative, "send-pack: send refs");
+}
+
 /*
  * Make a pack stream and spit it out into file descriptor fd
  */
@@ -35,7 +49,6 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	};
 	struct child_process po;
 	int i;
-	char buf[42];
 
 	if (args.use_thin_pack)
 		argv[4] = "--thin";
@@ -51,31 +64,17 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	 * We feed the pack-objects we just spawned with revision
 	 * parameters by writing to the pipe.
 	 */
-	for (i = 0; i < extra->nr; i++) {
-		memcpy(buf + 1, sha1_to_hex(&extra->array[i][0]), 40);
-		buf[0] = '^';
-		buf[41] = '\n';
-		if (!write_or_whine(po.in, buf, 42, "send-pack: send refs"))
+	for (i = 0; i < extra->nr; i++)
+		if (!feed_object(extra->array[i], po.in, 1))
 			break;
-	}
 
 	while (refs) {
 		if (!is_null_sha1(refs->old_sha1) &&
-		    has_sha1_file(refs->old_sha1)) {
-			memcpy(buf + 1, sha1_to_hex(refs->old_sha1), 40);
-			buf[0] = '^';
-			buf[41] = '\n';
-			if (!write_or_whine(po.in, buf, 42,
-						"send-pack: send refs"))
-				break;
-		}
-		if (!is_null_sha1(refs->new_sha1)) {
-			memcpy(buf, sha1_to_hex(refs->new_sha1), 40);
-			buf[40] = '\n';
-			if (!write_or_whine(po.in, buf, 41,
-						"send-pack: send refs"))
-				break;
-		}
+		    !feed_object(refs->old_sha1, po.in, 1))
+			break;
+		if (!is_null_sha1(refs->new_sha1) &&
+		    !feed_object(refs->new_sha1, po.in, 0))
+			break;
 		refs = refs->next;
 	}
 
diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh
new file mode 100755
index 0000000..6dfc55a
--- /dev/null
+++ b/t/t5519-push-alternates.sh
@@ -0,0 +1,106 @@
+#!/bin/sh
+
+test_description='push to a repository that borrows from elsewhere'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	mkdir alice-pub &&
+	(
+		cd alice-pub &&
+		GIT_DIR=. git init
+	) &&
+	mkdir alice-work &&
+	(
+		cd alice-work &&
+		git init &&
+		>file &&
+		git add . &&
+		git commit -m initial &&
+		git push ../alice-pub master
+	) &&
+
+	# Project Bob is a fork of project Alice
+	mkdir bob-pub &&
+	(
+		cd bob-pub &&
+		GIT_DIR=. git init &&
+		mkdir -p objects/info &&
+		echo ../../alice-pub/objects >objects/info/alternates
+	) &&
+	git clone alice-pub bob-work &&
+	(
+		cd bob-work &&
+		git push ../bob-pub master
+	)
+'
+
+test_expect_success 'alice works and pushes' '
+	(
+		cd alice-work &&
+		echo more >file &&
+		git commit -a -m second &&
+		git push ../alice-pub
+	)
+'
+
+test_expect_success 'bob fetches from alice, works and pushes' '
+	(
+		# Bob acquires what Alice did in his work tree first.
+		# Even though these objects are not directly in
+		# the public repository of Bob, this push does not
+		# need to send the commit Bob received from Alice
+		# to his public repository, as all the object Alice
+		# has at her public repository are available to it
+		# via its alternates.
+		cd bob-work &&
+		git pull ../alice-pub master &&
+		echo more bob >file &&
+		git commit -a -m third &&
+		git push ../bob-pub
+	) &&
+
+	# Check that the second commit by Alice is not sent
+	# to ../bob-pub
+	(
+		cd bob-pub &&
+		second=$(git rev-parse HEAD^) &&
+		rm -f objects/info/alternates &&
+		test_must_fail git cat-file -t $second &&
+		echo ../../alice-pub/objects >objects/info/alternates
+	)
+'
+
+test_expect_success 'clean-up in case the previous failed' '
+	(
+		cd bob-pub &&
+		echo ../../alice-pub/objects >objects/info/alternates
+	)
+'
+
+test_expect_success 'alice works and pushes again' '
+	(
+		# Alice does not care what Bob does.  She does not
+		# even have to be aware of his existence.  She just
+		# keeps working and pushing
+		cd alice-work &&
+		echo more alice >file &&
+		git commit -a -m fourth &&
+		git push ../alice-pub
+	)
+'
+
+test_expect_success 'bob works and pushes' '
+	(
+		# This time Bob does not pull from Alice, and
+		# the master branch at her public repository points
+		# at a commit Bob does not know about.  This should
+		# not prevent the push by Bob from succeeding.
+		cd bob-work &&
+		echo yet more bob >file &&
+		git commit -a -m fifth &&
+		git push ../bob-pub
+	)
+'
+
+test_done
-- 
1.6.1.1.273.g0e555

^ permalink raw reply related

* Re: [PATCHv2 3/4] run-command: help callers distinguish errors
From: Jeff King @ 2009-01-28  7:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128074334.GB31951@coredump.intra.peff.net>

On Wed, Jan 28, 2009 at 02:43:34AM -0500, Jeff King wrote:

> On Wed, Jan 28, 2009 at 02:36:39AM -0500, Jeff King wrote:
> 
> > +#define IS_RUN_COMMAND_ERR(x) (x >= ERR_RUN_COMMAND_FORK)
> 
> <sigh> This should be "<= -ERR_RUN_COMMAND_FORK", since we expect the
> negated status.
> 
> Maybe it is time I wrote a test for a failed external.

Hmm. Actually, it is hard to write a test case for this; it only
triggers if we actually have an error running a program, like fork()
failing.

-Peff

^ permalink raw reply

* Re: [PATCHv2 3/4] run-command: help callers distinguish errors
From: Jeff King @ 2009-01-28  7:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128073639.GC31884@coredump.intra.peff.net>

On Wed, Jan 28, 2009 at 02:36:39AM -0500, Jeff King wrote:

> +#define IS_RUN_COMMAND_ERR(x) (x >= ERR_RUN_COMMAND_FORK)

<sigh> This should be "<= -ERR_RUN_COMMAND_FORK", since we expect the
negated status.

Maybe it is time I wrote a test for a failed external.

-Peff

^ permalink raw reply

* Re: git 1.6.1 on AIX 5.3
From: Jeff King @ 2009-01-28  7:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Perry Smith, Mike Ralphson, git
In-Reply-To: <7v4ozjx4ni.fsf@gitster.siamese.dyndns.org>

On Tue, Jan 27, 2009 at 11:37:21PM -0800, Junio C Hamano wrote:

> >> Just to be sure we are on the same page.  My directory structure has a
> >> top/src/git-1.6.1 and top/build/git.1.6.1.  The src/git-1.6.1 is the
> >> tar ball.  The  build/git-1.6.1 starts out empty.  I cd into it and
> >> then do: ../../src/git-1.6.1/configure <options>  After this
> >> completes, you can do "make".
> >
> > I don't see how this would work without automake support, which git does
> > not have.
> 
> ... nor want to have ;-).

Heh. Yes, in case there was any confusion: I don't want my statement in
any way to be construed as a suggestion to support automake.

I would not be opposed to it if it somehow enhanced some users'
experience without bothering people who didn't want to touch it (like
the way that autoconf support is implemented). But I don't see how that
would be possible.

-Peff

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Junio C Hamano @ 2009-01-28  7:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Linus Torvalds, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <7vd4e7x5ov.fsf@gitster.siamese.dyndns.org>

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

> Here is my work in progress.  It introduces "ignore-missing-negative"
> option to the revision traversal machinery, and squelches the places we
> currently complain loudly and die when we expect an object to be
> available, when the color we are going to paint the object with is
> UNINTERESTING.
>
> I have a mild suspicion that it may even be the right thing to ignore them
> unconditionally, and it might even match the intention of Linus's original
> code.  That would make many hunks in this patch much simpler.
>
> The evidences behind this suspicion are found in a handful of places in
> revision.c.  mark_blob_uninteresting() does not complain if the caller
> fails to find the blob.  mark_tree_uninteresting() does not, either.
> mark_parents_uninteresting() does not, either, and it even has a comment
> that strongly suggests the original intention was not to care about
> missing UNINTERESTING objects.

Here is what I ended up with doing.  It lost "ignore-missing-negative"
so missing UNINTERESTING objects are non-error events more uniformly,
but on the other hand get_reference() which is about the command line
arguments always wants the named objects to exist, even if they are
marked as UNINTERESTING.

I'll send [PATCH 1/2] which is an update to my previous fix as a follow-up
to this message.

-- >8 --
Subject: [PATCH 2/2] revision traversal: allow UNINTERESTING objects to be missing

Most of the existing codepaths were meant to treat missing uninteresting
objects to be a silently ignored non-error, but there were a few places
in handle_commit() and add_parents_to_list(), which are two key functions
in the revision traversal machinery, that cared:

 - When a tag refers to an object that we do not have, we barfed.  We
   ignore such a tag if it is painted as UNINTERESTING with this change.

 - When digging deeper into the ancestry chain of a commit that is already
   painted as UNINTERESTING, in order to paint its parents UNINTERESTING,
   we barfed if parse_parent() for a parent commit object failed.  We can
   ignore such a parent commit object.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c                 |    7 +++++--
 t/t5519-push-alternates.sh |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index db60f06..ea8ba0f 100644
--- a/revision.c
+++ b/revision.c
@@ -183,8 +183,11 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
 		if (!tag->tagged)
 			die("bad tag");
 		object = parse_object(tag->tagged->sha1);
-		if (!object)
+		if (!object) {
+			if (flags & UNINTERESTING)
+				return NULL;
 			die("bad object %s", sha1_to_hex(tag->tagged->sha1));
+		}
 	}
 
 	/*
@@ -480,7 +483,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
 			struct commit *p = parent->item;
 			parent = parent->next;
 			if (parse_commit(p) < 0)
-				return -1;
+				continue;
 			p->object.flags |= UNINTERESTING;
 			if (p->parents)
 				mark_parents_uninteresting(p);
diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh
index 6dfc55a..96be523 100755
--- a/t/t5519-push-alternates.sh
+++ b/t/t5519-push-alternates.sh
@@ -103,4 +103,41 @@ test_expect_success 'bob works and pushes' '
 	)
 '
 
+test_expect_success 'alice works and pushes yet again' '
+	(
+		# Alice does not care what Bob does.  She does not
+		# even have to be aware of his existence.  She just
+		# keeps working and pushing
+		cd alice-work &&
+		echo more and more alice >file &&
+		git commit -a -m sixth.1 &&
+		echo more and more alice >>file &&
+		git commit -a -m sixth.2 &&
+		echo more and more alice >>file &&
+		git commit -a -m sixth.3 &&
+		git push ../alice-pub
+	)
+'
+
+test_expect_success 'bob works and pushes again' '
+	(
+		cd alice-pub &&
+		git cat-file commit master >../bob-work/commit
+	)
+	(
+		# This time Bob does not pull from Alice, and
+		# the master branch at her public repository points
+		# at a commit Bob does not fully know about, but
+		# he happens to have the commit object (but not the
+		# necessary tree) in his repository from Alice.
+		# This should not prevent the push by Bob from
+		# succeeding.
+		cd bob-work &&
+		git hash-object -t commit -w commit &&
+		echo even more bob >file &&
+		git commit -a -m seventh &&
+		git push ../bob-pub
+	)
+'
+
 test_done
-- 
1.6.1.1.273.g0e555

^ permalink raw reply related

* [PATCHv2 4/4] git: use run_command to execute dashed externals
From: Jeff King @ 2009-01-28  7:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128073059.GD19165@coredump.intra.peff.net>

We used to simply try calling execvp(); if it succeeded,
then we were done and the new program was running. If it
didn't, then we knew that it wasn't a valid command.

Unfortunately, this interacted badly with the new pager
handling. Now that git remains the parent process and the
pager is spawned, git has to hang around until the pager is
finished. We install an atexit handler to do this, but that
handler never gets called if we successfully run execvp.

You could see this behavior by running any dashed external
using a pager (e.g., "git -p stash list"). The command
finishes running, but the pager is still going. In the case
of less, it then gets an error reading from the terminal and
exits, potentially leaving the terminal in a broken state
(and not showing the output).

This patch just uses run_command to try running the
dashed external. The parent git process then waits for the
external process to complete and then handles the pager
cleanup as it would for an internal command.

Signed-off-by: Jeff King <peff@peff.net>
---
Incorporates negated status fix from JSixt.

This version also differentiates in the exit code and stderr output
whether we simply failed to exec the command versus passing along its
status code.

 git.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index 45e493d..b02b05b 100644
--- a/git.c
+++ b/git.c
@@ -2,6 +2,7 @@
 #include "exec_cmd.h"
 #include "cache.h"
 #include "quote.h"
+#include "run-command.h"
 
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
@@ -392,6 +393,7 @@ static void execv_dashed_external(const char **argv)
 {
 	struct strbuf cmd = STRBUF_INIT;
 	const char *tmp;
+	int status;
 
 	strbuf_addf(&cmd, "git-%s", argv[0]);
 
@@ -406,10 +408,17 @@ static void execv_dashed_external(const char **argv)
 
 	trace_argv_printf(argv, "trace: exec:");
 
-	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
-
-	trace_printf("trace: exec failed: %s\n", strerror(errno));
+	/*
+	 * if we fail because the command is not found, it is
+	 * OK to return. Otherwise, we just pass along the status code.
+	 */
+	status = run_command_v_opt(argv, 0);
+	if (status != -ERR_RUN_COMMAND_EXEC) {
+		if (IS_RUN_COMMAND_ERR(status))
+			die("unable to run '%s'", argv[0]);
+		exit(-status);
+	}
+	errno = ENOENT; /* as if we called execvp */
 
 	argv[0] = tmp;
 
-- 
1.6.1.1.367.g30b36

^ permalink raw reply related

* Re: git 1.6.1 on AIX 5.3
From: Junio C Hamano @ 2009-01-28  7:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Perry Smith, Mike Ralphson, git
In-Reply-To: <20090128070114.GB19165@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Jan 27, 2009 at 07:35:00PM -0600, Perry Smith wrote:
>
>> Just to be sure we are on the same page.  My directory structure has a
>> top/src/git-1.6.1 and top/build/git.1.6.1.  The src/git-1.6.1 is the
>> tar ball.  The  build/git-1.6.1 starts out empty.  I cd into it and
>> then do: ../../src/git-1.6.1/configure <options>  After this
>> completes, you can do "make".
>
> I don't see how this would work without automake support, which git does
> not have.

... nor want to have ;-).

^ permalink raw reply

* [PATCHv2 3/4] run-command: help callers distinguish errors
From: Jeff King @ 2009-01-28  7:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128073059.GD19165@coredump.intra.peff.net>

run_command returns a single integer specifying either an
error code or the exit status of the spawned program. The
only way to tell the difference is that the error codes are
outside of the allowed range of exit status values.

Rather than make each caller implement the test against a
magic limit, let's provide a macro.

Signed-off-by: Jeff King <peff@peff.net>
---
New since v1 of the series.

 run-command.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/run-command.h b/run-command.h
index 0211e1d..a88b3cd 100644
--- a/run-command.h
+++ b/run-command.h
@@ -10,6 +10,7 @@ enum {
 	ERR_RUN_COMMAND_WAITPID_SIGNAL,
 	ERR_RUN_COMMAND_WAITPID_NOEXIT,
 };
+#define IS_RUN_COMMAND_ERR(x) (x >= ERR_RUN_COMMAND_FORK)
 
 struct child_process {
 	const char **argv;
-- 
1.6.1.1.367.g30b36

^ permalink raw reply related

* [PATCHv2 2/4] run_command: handle missing command errors more gracefully
From: Jeff King @ 2009-01-28  7:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128073059.GD19165@coredump.intra.peff.net>

When run_command was asked to run a non-existant command,
its behavior varied depending on the platform:

  - on POSIX systems, we would fork, and then after the
    execvp call failed, we could call die(), which prints a
    message to stderr and exits with code 128.

  - on Windows, we do a PATH lookup, realize the program
    isn't there, and then return ERR_RUN_COMMAND_FORK

The goal of this patch is to make it clear to callers that
the specific error was a missing command. To do this, we
will return the error code ERR_RUN_COMMAND_EXEC, which is
already defined in run-command.h, checked for in several
places, but never actually gets set.

The new behavior is:

  - on POSIX systems, we exit the forked process with code
    127 (the same as the shell uses to report missing
    commands). The parent process recognizes this code and
    returns an EXEC error. The stderr message is silenced,
    since the caller may be speculatively trying to run a
    command. Instead, we use trace_printf so that somebody
    interested in debugging can see the error that occured.

  - on Windows, we check errno, which is already set
    correctly by mingw_spawnvpe, and report an EXEC error
    instead of a FORK error

Thus it is safe to speculatively run a command:

  int r = run_command_v_opt(argv, 0);
  if (r == -ERR_RUN_COMMAND_EXEC)
	  /* oops, it wasn't found; try something else */
  else
	  /* we failed for some other reason, error is in r */

Signed-off-by: Jeff King <peff@peff.net>
---
Incorporates JSixt's fix to retain errno across system calls.

 run-command.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/run-command.c b/run-command.c
index db9ce59..b05c734 100644
--- a/run-command.c
+++ b/run-command.c
@@ -118,7 +118,9 @@ int start_command(struct child_process *cmd)
 		} else {
 			execvp(cmd->argv[0], (char *const*) cmd->argv);
 		}
-		die("exec %s failed.", cmd->argv[0]);
+		trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
+				strerror(errno));
+		exit(127);
 	}
 #else
 	int s0 = -1, s1 = -1, s2 = -1;	/* backups of stdin, stdout, stderr */
@@ -187,6 +189,7 @@ int start_command(struct child_process *cmd)
 #endif
 
 	if (cmd->pid < 0) {
+		int err = errno;
 		if (need_in)
 			close_pair(fdin);
 		else if (cmd->in)
@@ -197,7 +200,9 @@ int start_command(struct child_process *cmd)
 			close(cmd->out);
 		if (need_err)
 			close_pair(fderr);
-		return -ERR_RUN_COMMAND_FORK;
+		return err == ENOENT ?
+			-ERR_RUN_COMMAND_EXEC :
+			-ERR_RUN_COMMAND_FORK;
 	}
 
 	if (need_in)
@@ -236,9 +241,14 @@ static int wait_or_whine(pid_t pid)
 		if (!WIFEXITED(status))
 			return -ERR_RUN_COMMAND_WAITPID_NOEXIT;
 		code = WEXITSTATUS(status);
-		if (code)
+		switch (code) {
+		case 127:
+			return -ERR_RUN_COMMAND_EXEC;
+		case 0:
+			return 0;
+		default:
 			return -code;
-		return 0;
+		}
 	}
 }
 
-- 
1.6.1.1.367.g30b36

^ permalink raw reply related

* [PATCHv2 1/4] git: s/run_command/run_builtin/
From: Jeff King @ 2009-01-28  7:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <20090128073059.GD19165@coredump.intra.peff.net>

There is a static function called run_command which
conflicts with the library function in run-command.c; this
isn't a problem currently, but prevents including
run-command.h in git.c.

This patch just renames the static function to something
more specific and non-conflicting.

Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.

 git.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index ecc8fad..45e493d 100644
--- a/git.c
+++ b/git.c
@@ -219,7 +219,7 @@ struct cmd_struct {
 	int option;
 };
 
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
+static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 {
 	int status;
 	struct stat st;
@@ -384,7 +384,7 @@ static void handle_internal_command(int argc, const char **argv)
 		struct cmd_struct *p = commands+i;
 		if (strcmp(p->cmd, cmd))
 			continue;
-		exit(run_command(p, argc, argv));
+		exit(run_builtin(p, argc, argv));
 	}
 }
 
-- 
1.6.1.1.367.g30b36

^ permalink raw reply related

* Re: [RFC/PATCH 0/3] fix "Funny: git -p submodule summary"
From: Jeff King @ 2009-01-28  7:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901271728130.3586@pacific.mpi-cbg.de>

On Tue, Jan 27, 2009 at 05:31:02PM +0100, Johannes Schindelin wrote:

> I like the patch series, well designed and concise (especially with the 
> fixes Hannes proposed).

Good. Response seems positive, so I will drop the RFC, then, and post a
fixed-up series meant for inclusion.

> > There are two potential downsides to the fix:
> > 
> >  1. There is an extra fork and a parent process sitting in memory for
> >     dashed externals. This is pretty necessary to any fix, since
> >     something has to wait to do pager cleanup, and we can't rely on the
> >     child to do so.
> 
> Actually, I think this is a good thing; that way, we can catch 
> segmentation fault properly and display an error message in the pager.  
> That was not possible previously.

True. On the other hand, most of our externals are shell scripts. It's
the builtins that segfault, and we don't have a watcher process for
that. :)

-Peff

^ permalink raw reply

* Re: [PATCH] Windows: Fix intermittent failures of t7701
From: Johannes Sixt @ 2009-01-28  7:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0901271740320.3586@pacific.mpi-cbg.de>

Johannes Schindelin schrieb:
> I use this trick in my valgrind series:
> 
> 	($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&
> 	test 0 = "$(cat exit.code)"

Ah, using a file as temporary storage? Why not simply

	$PROGRAM > data &&
	$OTHER_PROGRAM < data

Hm? ;)

-- Hannes

^ permalink raw reply

* feature needed imap-send pass as cli switch
From: Caleb Cushing @ 2009-01-28  7:18 UTC (permalink / raw)
  To: git

I tend to have my .gitconfig shared on github with a bunch of other
config files and this has been ok, until submitting a patch here I
needed to use imap-send, and the only way I see in the documentation
is to put my email password in .gitconfig, obviously I can't push this
up to my remote as I usually do. I'd like to see an option to pass the
password on the cli, either as an input prompt or as just an argument
to an option, or both. I think storing it in the config file is a
security risk, storing passwords in plaintext is just bad practice.

if it's already possible could someone share how with me? and perhaps
patch the documentation to include how.
-- 
Caleb Cushing

http://xenoterracide.blogspot.com

^ permalink raw reply

* Re: [RFC/PATCH 0/3] fix "Funny: git -p submodule summary"
From: Jeff King @ 2009-01-28  7:17 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Johannes Schindelin, git
In-Reply-To: <497F021B.2050306@viscovery.net>

On Tue, Jan 27, 2009 at 01:46:19PM +0100, Johannes Sixt wrote:

> Exit code and start_command/finish_command's return code handling is a
> complete mess IMHO and deserves a clean-up series of its own. If the few

Yes, the negation is a bit confusing, just for being allowed to say

  if (run_command(foo) < 0)

since you end up having to store and re-negate anyway to get the actual
code. Plus the value of errno is untrustworthy, since we may have been
doing cleanup calls.

> codes at 10000 and above are truncated to 8 bits, then we get exit codes
> 16 and higher; I think that's good enough for this series.

I think it is nice to differentiate between an exit code from the
sub-program and our own error, though. See my updated series for what I
think is a reasonable one-liner fix.

-Peff

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Junio C Hamano @ 2009-01-28  7:14 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Linus Torvalds, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <20090128044150.GI1321@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> >
>> > Oh, right, its not.  I was pointing out that the last time the
>> > protocol changed in a way the server can infer something about the
>> > client, which IIRC was 41fa7d2, we still don't have a way to tell
>> > what the client is.
>> 
>> But you are still talking as if there is one protocol you can call "the
>> protocol", but it is not.  The send-pack receive-pack protocol is on topic
>> in this thread; the quoted commit was about a separate and independent
>> fetch-pack upload-pack protocol.  It does not matter when that unrelated
>> protocol was enhanced.
>
> Blargh.  Of course you are right.  Its been a long 2 months for me
> at work.  I'm too #@*#@!@! tired to keep the basics straight anymore.
>
> I'm going to shut up now.

Please don't.

I've been toying with an idea for an alternative solution, and need
somebody competent to bounce it around with.

pack-objects ends up doing eventually

    rev-list --objects $send1 $send2 $send3 ... --not $have1 $have2 ...

which lists commits and associated objects reachable from $sendN,
excluding the ones that are reachable from $haveN.

The tentative solution Björn Steinbrink and I came up with excludes
missing commit from $haveN to avoid rev-list machinery to barf, but it
violates the ref-object contract as I explained to Björn in my other
message.

	Side note.  We often cite "interrupted commit walkers" as the
	reason why has_sha1_file() is not a good enough check, but you can
	discard a deep commit chain by deleting a branch, and have gc
	expire older commit in the commit chain while retaining newer ones
	near the tip of that branch.  If (1) you earlier gave that branch
	to somebody else, (2) that somebody else has the tip of the branch
	you discarded in his repository, and (3) the repository you are
	pushing into borrows from that somebody else's repository, then
	you have the same situation that your has_sha1_file() succeeds but
	it will fail when you start digging deeper.

Checking if each commit is reachable from any of the refs is quite
expensive, and it would especially be so if it is done once per ".have"
and real ref we receive from the other end.

An alternative is to realize that rev-list traversal already does
something quite similar to what is needed to prove if these ".have"s are
reachable from refs when listing the reachable objects.  This computation
is what it needs to do anyway, so if we teach rev-list to ignore missing
or broken chain while traversing negative refs, we do not have to incur
any overhead over existing code.

Here is my work in progress.  It introduces "ignore-missing-negative"
option to the revision traversal machinery, and squelches the places we
currently complain loudly and die when we expect an object to be
available, when the color we are going to paint the object with is
UNINTERESTING.

I have a mild suspicion that it may even be the right thing to ignore them
unconditionally, and it might even match the intention of Linus's original
code.  That would make many hunks in this patch much simpler.

The evidences behind this suspicion are found in a handful of places in
revision.c.  mark_blob_uninteresting() does not complain if the caller
fails to find the blob.  mark_tree_uninteresting() does not, either.
mark_parents_uninteresting() does not, either, and it even has a comment
that strongly suggests the original intention was not to care about
missing UNINTERESTING objects.

 builtin-pack-objects.c |    1 +
 revision.c             |   24 ++++++++++++++++++++----
 revision.h             |    1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git i/builtin-pack-objects.c w/builtin-pack-objects.c
index cedef52..c615a2f 100644
--- i/builtin-pack-objects.c
+++ w/builtin-pack-objects.c
@@ -2026,6 +2026,7 @@ static void get_object_list(int ac, const char **av)
 	int flags = 0;
 
 	init_revisions(&revs, NULL);
+	revs.ignore_missing_negative = 1;
 	save_commit_buffer = 0;
 	setup_revisions(ac, av, &revs, NULL);
 
diff --git i/revision.c w/revision.c
index db60f06..314341b 100644
--- i/revision.c
+++ w/revision.c
@@ -132,6 +132,8 @@ void mark_parents_uninteresting(struct commit *commit)
 
 static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)
 {
+	if (!obj)
+		return;
 	if (revs->no_walk && (obj->flags & UNINTERESTING))
 		die("object ranges do not make sense when not walking revisions");
 	if (revs->reflog_info && obj->type == OBJ_COMMIT &&
@@ -163,8 +165,11 @@ static struct object *get_reference(struct rev_info *revs, const char *name, con
 	struct object *object;
 
 	object = parse_object(sha1);
-	if (!object)
+	if (!object) {
+		if (revs->ignore_missing_negative && (flags & UNINTERESTING))
+			return NULL;
 		die("bad object %s", name);
+	}
 	object->flags |= flags;
 	return object;
 }
@@ -183,8 +188,11 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
 		if (!tag->tagged)
 			die("bad tag");
 		object = parse_object(tag->tagged->sha1);
-		if (!object)
+		if (!object) {
+			if (revs->ignore_missing_negative && (flags & UNINTERESTING))
+				return NULL;
 			die("bad object %s", sha1_to_hex(tag->tagged->sha1));
+		}
 	}
 
 	/*
@@ -193,8 +201,11 @@ static struct commit *handle_commit(struct rev_info *revs, struct object *object
 	 */
 	if (object->type == OBJ_COMMIT) {
 		struct commit *commit = (struct commit *)object;
-		if (parse_commit(commit) < 0)
+		if (parse_commit(commit) < 0) {
+			if (revs->ignore_missing_negative && (flags & UNINTERESTING))
+				return NULL;
 			die("unable to parse commit %s", name);
+		}
 		if (flags & UNINTERESTING) {
 			commit->object.flags |= UNINTERESTING;
 			mark_parents_uninteresting(commit);
@@ -479,8 +490,11 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
 		while (parent) {
 			struct commit *p = parent->item;
 			parent = parent->next;
-			if (parse_commit(p) < 0)
+			if (parse_commit(p) < 0) {
+				if (revs->ignore_missing_negative)
+					return 0;
 				return -1;
+			}
 			p->object.flags |= UNINTERESTING;
 			if (p->parents)
 				mark_parents_uninteresting(p);
@@ -1110,6 +1124,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->tree_objects = 1;
 		revs->blob_objects = 1;
 		revs->edge_hint = 1;
+	} else if (!strcmp(arg, "--ignore-missing-negative")) {
+		revs->ignore_missing_negative = 1;
 	} else if (!strcmp(arg, "--unpacked")) {
 		revs->unpacked = 1;
 		free(revs->ignore_packed);
diff --git i/revision.h w/revision.h
index 7cf8487..bb90399 100644
--- i/revision.h
+++ w/revision.h
@@ -48,6 +48,7 @@ struct rev_info {
 			tree_objects:1,
 			blob_objects:1,
 			edge_hint:1,
+			ignore_missing_negative:1,
 			limited:1,
 			unpacked:1, /* see also ignore_packed below */
 			boundary:2,

^ permalink raw reply related

* Re: [PATCH] mergetool merge/skip/abort at prompt
From: Caleb Cushing @ 2009-01-28  7:01 UTC (permalink / raw)
  To: Junio C Hamano, Charles Bailey, git, Nanako Shiraishi
In-Reply-To: <81bfc67a0901272256t726bf206k351bb6c8b2778bd5@mail.gmail.com>

better? I think I got the formatting fixed... got the imap working...
added a more descriptive commit message. hoping that everything looks
good.

^ permalink raw reply

* Re: git 1.6.1 on AIX 5.3
From: Jeff King @ 2009-01-28  7:01 UTC (permalink / raw)
  To: Perry Smith; +Cc: Mike Ralphson, git
In-Reply-To: <BAD975AD-323D-4278-8405-0B57E7202797@gmail.com>

On Tue, Jan 27, 2009 at 07:35:00PM -0600, Perry Smith wrote:

> Just to be sure we are on the same page.  My directory structure has a
> top/src/git-1.6.1 and top/build/git.1.6.1.  The src/git-1.6.1 is the
> tar ball.  The  build/git-1.6.1 starts out empty.  I cd into it and
> then do: ../../src/git-1.6.1/configure <options>  After this
> completes, you can do "make".

I don't see how this would work without automake support, which git does
not have. The configure script just generates a config.mak.autogen file,
which is included by the Makefile. So you have no Makefile in your build
directory.

> About 90% of the open source configure / autoconf code out there can
> do this with.  The other 10% you can not.  I like it because when
> things die, its easier to grep around the source tree and I blow away
> the build directory and start back over and I know that I'm starting
> fresh.

Another way of solving the same problem:

  cd untarred-sources
  git init
  git add .
  git commit -m 'pristine sources'

Now you can use "git clean" to clean up cruft, not to mention the usual
git stuff like tracking any changes you've made and submitting any
patches upstream.

> I get further.  But now test 10 of t0001.sh fails because test_cmp can
> not be found.
>
> Is that a GNU tool?  (I didn't see it in git or coreutils.)

It's a shell function defined in test-lib.sh (which is sourced by all of
the test scripts).

-Peff

^ permalink raw reply

* [PATCH] mergetool merge/skip/abort at prompt
From: Caleb Cushing @ 2009-01-28  6:56 UTC (permalink / raw)
  To: Junio C Hamano, Charles Bailey, git, Nanako Shiraishi

previously git mergetool when run with prompt only allowed the user to continue
merging. This changes git mergetool to allow the option of skipping a file or
aborting, and includes an addtional key to explicitly select merge.

Signed-off-by: Caleb Cushing <xenoterracide@gmail.com>
---
 git-mergetool.sh |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 00e1337..575fbb2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -177,8 +177,24 @@ merge_file () {
     describe_file "$local_mode" "local" "$LOCAL"
     describe_file "$remote_mode" "remote" "$REMOTE"
     if "$prompt" = true; then
-	printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
-	read ans
+	while true; do
+	    printf "Use (m)erge file or (s)kip file, or (a)bort? (%s): " \
+	    "$merge_tool"
+	    read ans
+	    case "$ans" in
+		[mM]*|"")
+		    break
+		    ;;
+		[sS]*)
+		    cleanup_temp_files
+		    return 0
+		    ;;
+		[aA]*)
+		    cleanup_temp_files
+		    exit 0
+		    ;;
+	    esac
+	done
     fi

     case "$merge_tool" in
-- 
1.6.1.1

^ permalink raw reply related

* Re: How to install and use a custom merge driver
From: Jeff King @ 2009-01-28  6:18 UTC (permalink / raw)
  To: Alec Clews; +Cc: git
In-Reply-To: <497FCC9A.9080008@gmail.com>

On Wed, Jan 28, 2009 at 02:10:18PM +1100, Alec Clews wrote:

> Setup:
>
> I have set up my ..git/info/gitattributes as follows
>
> *      merge=overwrite

Your setup looks right, except that the correct file is
".git/info/attributes".

-Peff

^ 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