[bug in next ?] git-fetch/git-push issue

[bug in next ?] git-fetch/git-push issue

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

With the current tip of next[0], I have this bizare issue:

  * I have two branches say master, and next, I'm on next.

  * my master lags behind origin/master, but next is a fast-forward wrt
    origin/next.

Now I git push:

    ┌─(18:16)──<~/some/repo next>──
    └[artemis] git push
    error: remote 'refs/heads/master' is not an ancestor of
     local  'refs/heads/master'.
     Maybe you are not up-to-date and need to pull first?
    updating 'refs/heads/next'
      from xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
      to   yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
    Counting objects: 24, done.
    Compressing objects: 100% (14/14), done.
    Writing objects: 100% (14/14), done.
    Total 14 (delta 12), reused 0 (delta 0)
    refs/heads/next: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -> yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
    updating local tracking ref 'refs/remotes/origin/master'
    updating local tracking ref 'refs/remotes/origin/next'

And then I fetch, and here happens something really awkward:

    ┌─(18:17)──<~/some/repo next>──
    └[artemis] git fetch
    From ssh://some.host/some/repo.git
     * [new branch]      master -> origin/master

I believe there is something rotten in the kingdom of Denmark… though
my heads seems to always be OK, I think it's just an output issue.

The fun part is that if next has nothing to push, nothing happens:

    ┌─(18:55)──<~/dev/mmsx next>──
    └[artemis] git push
    error: remote 'refs/heads/master' is not an ancestor of
     local  'refs/heads/master'.
     Maybe you are not up-to-date and need to pull first?
    error: failed to push to 'ssh://some.host/some/repo.git'
    ┌─(18:55)──<~/dev/mmsx next>──
    └[artemis] git fetch && echo $?
    0


  [0] actually it's a bit farther than the current next, but for
      parseopt thingies that are irrelevant here. My current origin/next
      is 76374a65c41b80fa83f27b4dd924bd3967a07d69
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [bug in next ?] git-fetch/git-push issue

[Daniel Barkalow]

On Mon, 5 Nov 2007, Pierre Habouzit wrote:

> With the current tip of next[0], I have this bizare issue:
> 
>   * I have two branches say master, and next, I'm on next.
> 
>   * my master lags behind origin/master, but next is a fast-forward wrt
>     origin/next.
> 
> Now I git push:
> 
>     error: remote 'refs/heads/master' is not an ancestor of
>      local  'refs/heads/master'.
>      Maybe you are not up-to-date and need to pull first?
>     updating 'refs/heads/next'
>       from xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>       to   yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
>     Counting objects: 24, done.
>     Compressing objects: 100% (14/14), done.
>     Writing objects: 100% (14/14), done.
>     Total 14 (delta 12), reused 0 (delta 0)
>     refs/heads/next: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -> yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
>     updating local tracking ref 'refs/remotes/origin/master'

I think this is the bit that's wrong. I blame Jeff, in 334f4831. :)

The issue is that, in the previous version, we'd hit a continue on the 
not-an-ancestor message and not reach the update_tracking_ref() section 
for that ref. In 334f4831, all of the updating is after the loop, and it 
doesn't filter out the refs that didn't actually get pushed.

Probably, all of the "continue;"s in do_send_pack() should also strip off 
the peer_ref. Something like (totally untested):

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 947c42b..6141672 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -254,12 +254,16 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
 		if (will_delete_ref && !allow_deleting_refs) {
 			error("remote does not support deleting refs");
 			ret = -2;
+			free(ref->peer_ref);
+			ref->peer_ref = NULL;
 			continue;
 		}
 		if (!will_delete_ref &&
 		    !hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
 			if (args.verbose)
 				fprintf(stderr, "'%s': up-to-date\n", ref->name);
+			free(ref->peer_ref);
+			ref->peer_ref = NULL;
 			continue;
 		}
 
@@ -303,6 +307,8 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
 				      ref->name,
 				      ref->peer_ref->name);
 				ret = -2;
+				free(ref->peer_ref);
+				ref->peer_ref = NULL;
 				continue;
 			}
 		}

Re: [bug in next ?] git-fetch/git-push issue

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

On Mon, Nov 05, 2007 at 05:56:54PM +0000, Pierre Habouzit wrote:
> With the current tip of next[0], I have this bizare issue:
> 
>   * I have two branches say master, and next, I'm on next.
> 
>   * my master lags behind origin/master, but next is a fast-forward wrt
>     origin/next.
> 
> Now I git push:
> 
>     ┌─(18:16)──<~/some/repo next>──
>     └[artemis] git push
>     error: remote 'refs/heads/master' is not an ancestor of
>      local  'refs/heads/master'.
>      Maybe you are not up-to-date and need to pull first?
>     updating 'refs/heads/next'
>       from xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>       to   yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
>     Counting objects: 24, done.
>     Compressing objects: 100% (14/14), done.
>     Writing objects: 100% (14/14), done.
>     Total 14 (delta 12), reused 0 (delta 0)
>     refs/heads/next: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -> yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
>     updating local tracking ref 'refs/remotes/origin/master'
>     updating local tracking ref 'refs/remotes/origin/next'
[...]
> I believe there is something rotten in the kingdom of Denmark… though
> my heads seems to always be OK, I think it's just an output issue.

  Okay I'm wrong. it happened again, in fact after the git-push I don't
have a origin/master anymore.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [bug in next ?] git-fetch/git-push issue

[Jeff King]

On Mon, Nov 05, 2007 at 01:17:14PM -0500, Daniel Barkalow wrote:

> I think this is the bit that's wrong. I blame Jeff, in 334f4831. :)
> 
> The issue is that, in the previous version, we'd hit a continue on the 
> not-an-ancestor message and not reach the update_tracking_ref() section 
> for that ref. In 334f4831, all of the updating is after the loop, and it 
> doesn't filter out the refs that didn't actually get pushed.

Nope, that's not the problem. We _only_ update any tracking refs at all
if ret == 0, and if we fail to push, then we are setting ret to -2.

Hrm. Oh wait, it looks like we then totally write over the current value
of 'ret' when we do pack_objects. Oops.

I'm unclear how to fix this, as I'm not really sure what ret is
_supposed_ to be communicating. What does the '-2' mean, as compared to
a '-4'? Should we be doing a 'ret += pack_objects(out, remote_refs)' or
some other bit-masking magic?

-Peff

Re: [bug in next ?] git-fetch/git-push issue

[Daniel Barkalow]

On Mon, 5 Nov 2007, Jeff King wrote:

> On Mon, Nov 05, 2007 at 01:17:14PM -0500, Daniel Barkalow wrote:
> 
> > I think this is the bit that's wrong. I blame Jeff, in 334f4831. :)
> > 
> > The issue is that, in the previous version, we'd hit a continue on the 
> > not-an-ancestor message and not reach the update_tracking_ref() section 
> > for that ref. In 334f4831, all of the updating is after the loop, and it 
> > doesn't filter out the refs that didn't actually get pushed.
> 
> Nope, that's not the problem. We _only_ update any tracking refs at all
> if ret == 0, and if we fail to push, then we are setting ret to -2.

That's an odd combination of behavior: we update some of the refs, leave 
the ones that didn't work alone, report success on the ones that worked, 
but then we forget that some things worked?

If we're going to refuse to update local tracking refs, whose state 
doesn't matter much, we should certainly refuse to update the remote refs, 
which are probably public and extremely important. If we just pushed and 
we fetch, we should see exclusively changes that somebody else (including 
hooks remotely) did, not anything that we ourselves did.

> Hrm. Oh wait, it looks like we then totally write over the current value
> of 'ret' when we do pack_objects. Oops.
> 
> I'm unclear how to fix this, as I'm not really sure what ret is
> _supposed_ to be communicating. What does the '-2' mean, as compared to
> a '-4'? Should we be doing a 'ret += pack_objects(out, remote_refs)' or
> some other bit-masking magic?

I'd guess -2 is supposed to indicate that there were some errors but some 
things may have worked. If pack_objects() or receive_status() fails, we 
shouldn't update anything locally (because it won't have been accepted 
remotely); otherwise, we should make local updates of every remote efect 
we had, even if we end up returning non-zero to indicate that we didn't do 
everything asked.

	-Daniel
*This .sig left intentionally blank*

Re: [bug in next ?] git-fetch/git-push issue

[Jeff King]

On Mon, Nov 05, 2007 at 04:41:37PM -0500, Daniel Barkalow wrote:

> > Nope, that's not the problem. We _only_ update any tracking refs at all
> > if ret == 0, and if we fail to push, then we are setting ret to -2.
> 
> That's an odd combination of behavior: we update some of the refs, leave 
> the ones that didn't work alone, report success on the ones that worked, 
> but then we forget that some things worked?

I think the current behavior is more odd. We mark some errors, and then
if there were any possible pushes, we replace the marked errors with the
status of the actual push, forgetting about the previous errors. Thus
the behavior where if 'next' needs pushing, then we don't mark any errors
at all (even though we spewed an error to stderr), but if it doesn't,
then we return an error.

I don't mind being conservative with updating tracking refs; they really
are just an optimization to avoid an extra git-fetch. But the most
sensible behavior would be to mark errors for _each_ ref individually,
try to push or update tracking branches where appropriate, and then
return an error status based on all refs (whether they had an error in
prep time or at push time).

Which I guess is what you were trying to accomplish by removing the
peer_ref, though I think that doesn't distinguish between "didn't match
a remote ref" and "had an error." Perhaps we just need an error flag in
the ref struct?

> If we're going to refuse to update local tracking refs, whose state 
> doesn't matter much, we should certainly refuse to update the remote refs, 
> which are probably public and extremely important. If we just pushed and 

I would also be fine with that: if your intended push has _any_
problems, then abort the push.

> we fetch, we should see exclusively changes that somebody else (including 
> hooks remotely) did, not anything that we ourselves did.

I don't necessarily agree, just because the notion of "we" and "somebody
else" is sort of pointless in a distributed system. I consider local
tracking ref updates to be a "best guess" attempt to optimize and avoid
a fetch (but one that will always be overwritten by the _actual_
contents when you do fetch).

> I'd guess -2 is supposed to indicate that there were some errors but some 
> things may have worked. If pack_objects() or receive_status() fails, we 

In that case, I think the simple fix is:

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 947c42b..f773dc8 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -338,7 +338,7 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
 
 	packet_flush(out);
 	if (new_refs && !args.dry_run)
-		ret = pack_objects(out, remote_refs);
+		ret |= pack_objects(out, remote_refs);
 	close(out);
 
 	if (expect_status_report) {

and then we accept that we don't know _which_ refs shouldn't have their
tracking branches updated (and we don't update any). But at least we
don't forget that the error occured.

And the better solution is an error flag (or bitfield) in struct ref.

-Peff

Re: [bug in next ?] git-fetch/git-push issue

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Which I guess is what you were trying to accomplish by removing the
> peer_ref, though I think that doesn't distinguish between "didn't match
> a remote ref" and "had an error." Perhaps we just need an error flag in
> the ref struct?

I agree that makes the most sense.

As Steffen has been advocating on another thread, depending on
your workflow, you do not care about some classes of push errors
per pushed refs.  The update of the remote and local tracking
refs should be done in sync (i.e. if the remote wasn't updated,
never update the corresponding local), but it can depend on the
nature of the failure if a failure to update a remote ref should
result in the non-zero exit status from git-push as a whole.

And to implement that, per-ref error flag would be a good way to
go, methinks.

Re: [bug in next ?] git-fetch/git-push issue

[Daniel Barkalow]

On Mon, 5 Nov 2007, Jeff King wrote:

> On Mon, Nov 05, 2007 at 04:41:37PM -0500, Daniel Barkalow wrote:
> 
> > > Nope, that's not the problem. We _only_ update any tracking refs at all
> > > if ret == 0, and if we fail to push, then we are setting ret to -2.
> > 
> > That's an odd combination of behavior: we update some of the refs, leave 
> > the ones that didn't work alone, report success on the ones that worked, 
> > but then we forget that some things worked?
> 
> I think the current behavior is more odd. We mark some errors, and then
> if there were any possible pushes, we replace the marked errors with the
> status of the actual push, forgetting about the previous errors. Thus
> the behavior where if 'next' needs pushing, then we don't mark any errors
> at all (even though we spewed an error to stderr), but if it doesn't,
> then we return an error.
> 
> I don't mind being conservative with updating tracking refs; they really
> are just an optimization to avoid an extra git-fetch. But the most
> sensible behavior would be to mark errors for _each_ ref individually,
> try to push or update tracking branches where appropriate, and then
> return an error status based on all refs (whether they had an error in
> prep time or at push time).
> 
> Which I guess is what you were trying to accomplish by removing the
> peer_ref, though I think that doesn't distinguish between "didn't match
> a remote ref" and "had an error." Perhaps we just need an error flag in
> the ref struct?

That's possible, but I think we currently only care about per-ref stuff 
for whether we actually did a push to that ref, and we care about whether 
we had a failure that affects all refs, and we care (for the return value) 
whether we have any errors at all.

> > If we're going to refuse to update local tracking refs, whose state 
> > doesn't matter much, we should certainly refuse to update the remote refs, 
> > which are probably public and extremely important. If we just pushed and 
> 
> I would also be fine with that: if your intended push has _any_
> problems, then abort the push.
> 
> > we fetch, we should see exclusively changes that somebody else (including 
> > hooks remotely) did, not anything that we ourselves did.
> 
> I don't necessarily agree, just because the notion of "we" and "somebody
> else" is sort of pointless in a distributed system. I consider local
> tracking ref updates to be a "best guess" attempt to optimize and avoid
> a fetch (but one that will always be overwritten by the _actual_
> contents when you do fetch).

I think it's actually very significant what commits made locally have 
or haven't been made public.

> > I'd guess -2 is supposed to indicate that there were some errors but some 
> > things may have worked. If pack_objects() or receive_status() fails, we 
> 
> In that case, I think the simple fix is:
> 
> diff --git a/builtin-send-pack.c b/builtin-send-pack.c
> index 947c42b..f773dc8 100644
> --- a/builtin-send-pack.c
> +++ b/builtin-send-pack.c
> @@ -338,7 +338,7 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
>  
>  	packet_flush(out);
>  	if (new_refs && !args.dry_run)
> -		ret = pack_objects(out, remote_refs);
> +		ret |= pack_objects(out, remote_refs);
>  	close(out);
>  
>  	if (expect_status_report) {
> 
> and then we accept that we don't know _which_ refs shouldn't have their
> tracking branches updated (and we don't update any). But at least we
> don't forget that the error occured.
> 
> And the better solution is an error flag (or bitfield) in struct ref.

How about this, in addition to my previous dropping peer_ref:

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 947c42b..1b7206c 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -337,8 +343,10 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
 	}
 
 	packet_flush(out);
-	if (new_refs && !args.dry_run)
-		ret = pack_objects(out, remote_refs);
+	if (new_refs && !args.dry_run) {
+		if (pack_objects(out, remote_refs))
+			ret = -4;
+	}
 	close(out);
 
 	if (expect_status_report) {
@@ -346,7 +354,7 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
 			ret = -4;
 	}
 
-	if (!args.dry_run && remote && ret == 0) {
+	if (!args.dry_run && remote && ret != -4) {
 		for (ref = remote_refs; ref; ref = ref->next)
 			update_tracking_ref(remote, ref);
 	}

That is, -2 means that we've done less than was asked, but nothing blew 
up; -4 means something blew up. When we skip something, we drop peer_ref 
from it, so there's nothing to update (and it's dropped from the set of 
mappings, in case we cared further about it with respect to reporting the 
actions we actually took). Then we update all refs that were acted on if 
ret isn't -4, and we return non-zero if ret is either -2 or -4.

	-Daniel
*This .sig left intentionally blank*

Re: [bug in next ?] git-fetch/git-push issue

[Daniel Barkalow]

On Mon, 5 Nov 2007, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Which I guess is what you were trying to accomplish by removing the
> > peer_ref, though I think that doesn't distinguish between "didn't match
> > a remote ref" and "had an error." Perhaps we just need an error flag in
> > the ref struct?
> 
> I agree that makes the most sense.
> 
> As Steffen has been advocating on another thread, depending on
> your workflow, you do not care about some classes of push errors
> per pushed refs.  The update of the remote and local tracking
> refs should be done in sync (i.e. if the remote wasn't updated,
> never update the corresponding local), but it can depend on the
> nature of the failure if a failure to update a remote ref should
> result in the non-zero exit status from git-push as a whole.
> 
> And to implement that, per-ref error flag would be a good way to
> go, methinks.

I think dropping peer_ref may be the clearest semantics. If push decides 
not to actually perform a push, we can just remove it from the list of 
operations we're performing. Independant of this, we can decide whether to 
signal an error, and whether an error means that the remote end will have 
done nothing at all (in which case, we must not update tracking refs).

That is, on top of my changes in other email, Steffan would have the 
strictly behind case just not have the "ret = -2" and everything else 
would be fine.

	-Daniel
*This .sig left intentionally blank*

Re: [bug in next ?] git-fetch/git-push issue

[Jeff King]

On Mon, Nov 05, 2007 at 06:46:08PM -0500, Daniel Barkalow wrote:

> -	if (!args.dry_run && remote && ret == 0) {
> +	if (!args.dry_run && remote && ret != -4) {
>  		for (ref = remote_refs; ref; ref = ref->next)
>  			update_tracking_ref(remote, ref);
>  	}
> 
> That is, -2 means that we've done less than was asked, but nothing blew 
> up; -4 means something blew up. When we skip something, we drop peer_ref 
> from it, so there's nothing to update (and it's dropped from the set of 
> mappings, in case we cared further about it with respect to reporting the 
> actions we actually took). Then we update all refs that were acted on if 
> ret isn't -4, and we return non-zero if ret is either -2 or -4.

This is OK, I guess. But it really doesn't accomplish the useful thing
we might get by noting per-ref errors, which is that we _can_ update the
tracking refs for those refs where it is appropriate.

We would have to parse the lines from receive_status and match them with
refs. I started a patch to do this, but I wonder if it is really worth
it. I would think that 99% of the time you have a failure at the sending
level, it's because the connection is broken, and you have no idea
_which_ refs were updated anyway, and you have to assume that none were.
So perhaps it is OK to just treat the two classes of errors differently,
and only cover per-ref errors for the "-2" case.

-Peff

Re: [bug in next ?] git-fetch/git-push issue

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

  On the same vein, with today's next:

    $ git push origin :teaser
    To ssh://git.corp/srv/git/mmsx.git
     - [deleting]        teaser
    refs/heads/teaser: 05518bc7df1af680447f58b034b108f66668db03 -> deleted
    Everything up-to-date
    fatal: Invalid revision range 05518bc7df1af680447f58b034b108f66668db03..0000000000000000000000000000000000000000
    fatal: ambiguous argument 'refs/heads/teaser': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [bug in next ?] git-fetch/git-push issue

[Jeff King]

On Tue, Nov 06, 2007 at 06:56:27PM +0100, Pierre Habouzit wrote:

>   On the same vein, with today's next:
> 
>     $ git push origin :teaser
>     To ssh://git.corp/srv/git/mmsx.git
>      - [deleting]        teaser
>     refs/heads/teaser: 05518bc7df1af680447f58b034b108f66668db03 -> deleted
>     Everything up-to-date
>     fatal: Invalid revision range 05518bc7df1af680447f58b034b108f66668db03..0000000000000000000000000000000000000000
>     fatal: ambiguous argument 'refs/heads/teaser': unknown revision or path not in the working tree.
>     Use '--' to separate paths from revisions

I can't replicate this output; can you provide more information on your
test repo?

If it is related to the bug we have been discussing, there haven't been
any fixes applied yet. :)

-Peff

Re: [bug in next ?] git-fetch/git-push issue

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Nov 06, 2007 at 06:56:27PM +0100, Pierre Habouzit wrote:
>
>>   On the same vein, with today's next:
>> 
>>     $ git push origin :teaser
>>     To ssh://git.corp/srv/git/mmsx.git
>>      - [deleting]        teaser
>>     refs/heads/teaser: 05518bc7df1af680447f58b034b108f66668db03 -> deleted
>>     Everything up-to-date
>>     fatal: Invalid revision range 05518bc7df1af680447f58b034b108f66668db03..0000000000000000000000000000000000000000
>>     fatal: ambiguous argument 'refs/heads/teaser': unknown revision or path not in the working tree.
>>     Use '--' to separate paths from revisions

Isn't this coming from a loosely written post-receive hook that
wants to send mail or something and forgets that a ref could be
removed?

Re: [bug in next ?] git-fetch/git-push issue

[Jeff King]

On Tue, Nov 06, 2007 at 10:23:45AM -0800, Junio C Hamano wrote:

> >>     $ git push origin :teaser
> >>     To ssh://git.corp/srv/git/mmsx.git
> >>      - [deleting]        teaser
> >>     refs/heads/teaser: 05518bc7df1af680447f58b034b108f66668db03 -> deleted
> >>     Everything up-to-date
> >>     fatal: Invalid revision range 05518bc7df1af680447f58b034b108f66668db03..0000000000000000000000000000000000000000
> >>     fatal: ambiguous argument 'refs/heads/teaser': unknown revision or path not in the working tree.
> >>     Use '--' to separate paths from revisions
> 
> Isn't this coming from a loosely written post-receive hook that
> wants to send mail or something and forgets that a ref could be
> removed?

That would make sense. I was wondering how in the world he managed to
provoke output from git-push that came after the send_pack call. So I
think this is unrelated to the bug we were discussing (although the
'Everything up-to-date' message is easily reproducible and seems a bit
silly given that we did push some changes).

-Peff

Re: [bug in next ?] git-fetch/git-push issue

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]

On Tue, Nov 06, 2007 at 06:23:45PM +0000, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Nov 06, 2007 at 06:56:27PM +0100, Pierre Habouzit wrote:
> >
> >>   On the same vein, with today's next:
> >> 
> >>     $ git push origin :teaser
> >>     To ssh://git.corp/srv/git/mmsx.git
> >>      - [deleting]        teaser
> >>     refs/heads/teaser: 05518bc7df1af680447f58b034b108f66668db03 -> deleted
> >>     Everything up-to-date
> >>     fatal: Invalid revision range 05518bc7df1af680447f58b034b108f66668db03..0000000000000000000000000000000000000000
> >>     fatal: ambiguous argument 'refs/heads/teaser': unknown revision or path not in the working tree.
> >>     Use '--' to separate paths from revisions
> 
> Isn't this coming from a loosely written post-receive hook that
> wants to send mail or something and forgets that a ref could be
> removed?

  oooh you may be right indeed. it's probably it, I was too quick in
assuming this was a new issue with git push, I never removed a branch on
that remote yet, and it indeed has a post-receive hook.

  thanks and sorry for the noise.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [bug in next ?] git-fetch/git-push issue

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]


oh and while we're at it, someone reminded me on IRC that when you:

  git push origin :somebranch

it does not removes origin/somebranch from your branches.  Now that git
push updates our knowledge of the remote branch, I believe that it
should also try to perform the equivalent of a:

  git branch -r -d origin/somebranch

And to be fair, I'd also say that git fetch <some-remote> should
complain about remote heads that match <some-remote> refspec that have
no corresponding reference _on_ the remote so that the user knows that
the branches have been removed.

I wanted to write a patch about that long time ago, but my plate is
already full with the diff option things.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]