[PATCH] send-pack: respect '+' on wildcard refspecs

[PATCH] send-pack: respect '+' on wildcard refspecs

[Jeff King]

When matching source and destination refs, we were failing
to pull the 'force' parameter from wildcard respects (but
not explicit ones) and attach it to the ref struct.

This adds a test for explicit and wildcard refspecs; the
latter fails without this patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote.c             |    2 ++
 t/t5400-send-pack.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/remote.c b/remote.c
index b20e2be..170015a 100644
--- a/remote.c
+++ b/remote.c
@@ -762,6 +762,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			hashcpy(dst_peer->new_sha1, src->new_sha1);
 		}
 		dst_peer->peer_ref = src;
+		if (pat)
+			dst_peer->force = pat->force;
 	free_name:
 		free(dst_name);
 	}
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 57c6397..2d0c07f 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -123,4 +123,52 @@ test_expect_success \
 	git-branch -a >branches && ! grep -q origin/master branches
 '
 
+rewound_push_setup() {
+	rm -rf parent child &&
+	mkdir parent && cd parent &&
+	git-init && echo one >file && git-add file && git-commit -m one &&
+	echo two >file && git-commit -a -m two &&
+	cd .. &&
+	git-clone parent child && cd child && git-reset --hard HEAD^
+}
+
+rewound_push_succeeded() {
+	cmp ../parent/.git/refs/heads/master .git/refs/heads/master
+}
+
+rewound_push_failed() {
+	if rewound_push_succeeded
+	then
+		false
+	else
+		true
+	fi
+}
+
+test_expect_success \
+	'pushing explicit refspecs respects forcing' '
+	rewound_push_setup &&
+	if git-send-pack ../parent/.git refs/heads/master:refs/heads/master
+	then
+		false
+	else
+		true
+	fi && rewound_push_failed &&
+	git-send-pack ../parent/.git +refs/heads/master:refs/heads/master &&
+	rewound_push_succeeded
+'
+
+test_expect_success \
+	'pushing wildcard refspecs respects forcing' '
+	rewound_push_setup &&
+	if git-send-pack ../parent/.git refs/heads/*:refs/heads/*
+	then
+		false
+	else
+		true
+	fi && rewound_push_failed &&
+	git-send-pack ../parent/.git +refs/heads/*:refs/heads/* &&
+	rewound_push_succeeded
+'
+
 test_done
-- 
1.5.3.4.1254.gc1ca9-dirty

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Jeff King]

On Fri, Oct 19, 2007 at 05:04:00AM -0400, Jeff King wrote:

> to pull the 'force' parameter from wildcard respects (but

Urgh, that should be "refspecs", not "respects". Maybe I should quit for
the night.

-Peff

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Dan McGee]

On 10/19/07, Jeff King <peff@peff.net> wrote:
> When matching source and destination refs, we were failing
> to pull the 'force' parameter from wildcard respects (but
> not explicit ones) and attach it to the ref struct.
>
> This adds a test for explicit and wildcard refspecs; the
> latter fails without this patch.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  remote.c             |    2 ++
>  t/t5400-send-pack.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index b20e2be..170015a 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -762,6 +762,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
>                         hashcpy(dst_peer->new_sha1, src->new_sha1);
>                 }
>                 dst_peer->peer_ref = src;
> +               if (pat)
> +                       dst_peer->force = pat->force;
>         free_name:
>                 free(dst_name);
>         }
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 57c6397..2d0c07f 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -123,4 +123,52 @@ test_expect_success \
>         git-branch -a >branches && ! grep -q origin/master branches
>  '
>
> +rewound_push_setup() {
> +       rm -rf parent child &&
> +       mkdir parent && cd parent &&
> +       git-init && echo one >file && git-add file && git-commit -m one &&
> +       echo two >file && git-commit -a -m two &&
> +       cd .. &&
> +       git-clone parent child && cd child && git-reset --hard HEAD^
> +}
> +
> +rewound_push_succeeded() {
> +       cmp ../parent/.git/refs/heads/master .git/refs/heads/master
> +}
> +
> +rewound_push_failed() {
> +       if rewound_push_succeeded
> +       then
> +               false
> +       else
> +               true
> +       fi
> +}
> +
> +test_expect_success \
> +       'pushing explicit refspecs respects forcing' '
> +       rewound_push_setup &&
> +       if git-send-pack ../parent/.git refs/heads/master:refs/heads/master
> +       then
> +               false
> +       else
> +               true
> +       fi && rewound_push_failed &&
> +       git-send-pack ../parent/.git +refs/heads/master:refs/heads/master &&
> +       rewound_push_succeeded
> +'
> +
> +test_expect_success \
> +       'pushing wildcard refspecs respects forcing' '
> +       rewound_push_setup &&
> +       if git-send-pack ../parent/.git refs/heads/*:refs/heads/*
> +       then
> +               false
> +       else
> +               true
> +       fi && rewound_push_failed &&
> +       git-send-pack ../parent/.git +refs/heads/*:refs/heads/* &&
> +       rewound_push_succeeded
> +'
> +
>  test_done
> --
> 1.5.3.4.1254.gc1ca9-dirty
>

Hmm. For some reason this passes with your test case, but not with my
original bash test script[1]. Did you try it with this?

-Dan

[1] http://www.toofishes.net/uploads/

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Jeff King]

On Fri, Oct 19, 2007 at 07:10:42AM -0500, Dan McGee wrote:

> Hmm. For some reason this passes with your test case, but not with my
> original bash test script[1]. Did you try it with this?
>
> [1] http://www.toofishes.net/uploads/

[please trim quoted text; I had a hard time finding your message amidst
the patch]

I didn't try it until you sent your message, but your test seems to work
fine for me. My patch is on top of 'next', which is what I usually run.
I haven't looked into 'master' (I assumed since the bug was reproducible
in both, it would be the same in both, but that is perhaps not the
case).

-Peff

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Dan McGee]

On 10/19/07, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 07:10:42AM -0500, Dan McGee wrote:
>
> > Hmm. For some reason this passes with your test case, but not with my
> > original bash test script[1]. Did you try it with this?
> >
> > [1] http://www.toofishes.net/uploads/
>
> I didn't try it until you sent your message, but your test seems to work
> fine for me. My patch is on top of 'next', which is what I usually run.
> I haven't looked into 'master' (I assumed since the bug was reproducible
> in both, it would be the same in both, but that is perhaps not the
> case).

Still getting this error:
error: remote 'refs/heads/working' is not a strict subset of local ref
'refs/heads/working'. maybe you are not up-to-date and need to pull
first?
error: failed to push to '/tmp/testpush'

I've tried applying the patch on the following commits, and maybe I'm
smoking something but I can't get it to pass my test script.

origin(junio)/master: 58ba4f6
origin(junio)/next: fe96ee67ec5840
spearce/master: 7840ce6cb24a9d
spearce/next: 2fe5433b416f0df

Can you let me know what commit you based the patch off of? I'm at
work for the next 8 hours or so, so I can't look in to this a whole
lot until tonight.

-Dan

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Jeff King]

On Fri, Oct 19, 2007 at 08:38:06AM -0500, Dan McGee wrote:

> origin(junio)/master: 58ba4f6
> origin(junio)/next: fe96ee67ec5840
> spearce/master: 7840ce6cb24a9d
> spearce/next: 2fe5433b416f0df
> 
> Can you let me know what commit you based the patch off of? I'm at
> work for the next 8 hours or so, so I can't look in to this a whole
> lot until tonight.

It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
something silly like executing an older version of git that is in your
PATH?

-Peff

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Dan McGee]

Ahh, shoot. Forgot to reply to all.

On 10/19/07, Jeff King <peff@peff.net> wrote:
> It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
> something silly like executing an older version of git that is in your
> PATH?

Yeah, just tried that again, definitely using the right version of
git. Before I apply your patch, both my test script and your addition
to t5400 fail. After applying your patch, my test script fails but
your addition to t5400 succeeds. Could this be something where
git-push and git-send-pack are not interacting correctly?

-Dan

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Dan McGee]

On 10/19/07, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 08:38:06AM -0500, Dan McGee wrote:
>
> > origin(junio)/master: 58ba4f6
> > origin(junio)/next: fe96ee67ec5840
> > spearce/master: 7840ce6cb24a9d
> > spearce/next: 2fe5433b416f0df
> >
> > Can you let me know what commit you based the patch off of? I'm at
> > work for the next 8 hours or so, so I can't look in to this a whole
> > lot until tonight.
>
> It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
> something silly like executing an older version of git that is in your
> PATH?

Turns out I didn't have GIT_EXEC_PATH set up right. Once I do that,
everything seems to work just fine.

Thanks for looking into this Jeff, and git-bisect just won me over. It
made easy work of finding the commit that broke this.

-Dan

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Jeff King]

On Fri, Oct 19, 2007 at 08:00:37PM -0500, Dan McGee wrote:

> Turns out I didn't have GIT_EXEC_PATH set up right. Once I do that,
> everything seems to work just fine.
> 
> Thanks for looking into this Jeff, and git-bisect just won me over. It
> made easy work of finding the commit that broke this.

Huzzah, success! Shawn, this should probably go on 'maint'. Although it
is probably not high priority (it has been broken since May; I think
wildcard push refspecs must not be that common), it is a fairly trivial
fix that shouldn't impact anyone else.

-Peff

Re: [PATCH] send-pack: respect '+' on wildcard refspecs

[Shawn O. Pearce]

Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 08:00:37PM -0500, Dan McGee wrote:
> 
> > Turns out I didn't have GIT_EXEC_PATH set up right. Once I do that,
> > everything seems to work just fine.
> > 
> > Thanks for looking into this Jeff, and git-bisect just won me over. It
> > made easy work of finding the commit that broke this.
> 
> Huzzah, success! Shawn, this should probably go on 'maint'. Although it
> is probably not high priority (it has been broken since May; I think
> wildcard push refspecs must not be that common), it is a fairly trivial
> fix that shouldn't impact anyone else.

Yea, its already queued on maint.  Both the patch and the
tests are obviously correct.  Its broken without the patch.
Its documented/expected behavior fixed by the patch.  IMHO it
belongs in maint.

-- 
Shawn.