On 05/05/2012 12:30 AM, Junio C Hamano wrote:
There are a handful of places where we call check_refname_format() on a
substring after "refs/" of a refname we are going to use, and filter out a
valid match with "refs/stash" with such a pathspec. Not sending a stash
may arguably be a feature (as stash is inherently a local workflow
element), but the code in the transport layer is oblivious to this
filtering performed by the lower layer of the code, and complains that the
other side did not send all the objects that needs to complete refs/stash
at the end, even though the code will not write refs/stash out anyway, and
making the whole command fail.
This is an attempt to "fix" it by using check_refname_format() on the
whole "refs/....." string and allowing refs/stash to be also copied.
Signed-off-by: Junio C Hamano<gitster@xxxxxxxxx>
---
* With this patch:
$ git checkout HEAD^0 ;# make sure we are on detached HEAD
$ git fetch $somewhere +refs/*:refs/*
and
victim$ git config receive.denyCurrentBranch warn
master$ git push victim +refs/*:refs/*
should work.
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
remote.c | 2 +-
t/t5516-fetch-push.sh | 30 ++++++++++++++++++++++++++++++
4 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 6207ecd..a3e3fa3 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -546,7 +546,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5)&&
- check_refname_format(ref->name + 5, 0))
+ check_refname_format(ref->name, 0))
The patch looks fine to me.
This combination "!memcmp(ref->name, "refs/", 5) &&
check_refname_format(ref->name, 0)" is the reason that I suggested
adding a REFNAME_FULL option [1], in which case it could be written
"check_refname_format(ref->name, REFNAME_FULL)". However, now I think
that the options should be constructed a little differently:
flags==0: Require refname to start with "refs/"
flags==REFNAME_ALLOW_SPECIAL: Also accept single-level ALL_CAPS refnames.
flags==REFNAME_ALLOW_PARTIAL: Don't check the namespace or require '/').
This could be used for checking partial names like "master" as
shorthand for "refs/master".
Does this sound reasonable to you?
Michael
[1] http://article.gmane.org/gmane.comp.version-control.git/196632
; /* trash */
else if (args.fetch_all&&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 7ec68a1..1935b80 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -401,7 +401,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_refname_format(name + 5, 0)) {
+ if (prefixcmp(name, "refs/") || check_refname_format(name, 0)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
diff --git a/remote.c b/remote.c
index e2ef991..eacd8ad 100644
--- a/remote.c
+++ b/remote.c
@@ -1595,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_refname_format(refname + 5, 0))
+ if (check_refname_format(refname, 0))
return 0;
len = strlen(refname) + 1;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b69cf57..1a45b19 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -929,6 +929,36 @@ test_expect_success 'push into aliased refs (inconsistent)' '
)
'
+test_expect_success 'push all hierarchies with stash' '
+ mk_empty&&
+ git stash clear&&
+ git reset --hard&&
+ echo>>path1&&
+ git stash save "Tweak path1"&&
+ git push testrepo "refs/*:refs/*"&&
+ git ls-remote .>expect&&
+ git ls-remote testrepo>actual&&
+ test_cmp actual expect
+'
+
+test_expect_success 'fetch all hierarchies with stash' '
+ mk_empty&&
+ git stash clear&&
+ git reset --hard&&
+ echo>>path1&&
+ git stash save "Tweak path1"&&
+ (
+ cd testrepo&&
+ git commit --allow-empty -m initial&&
+ git checkout HEAD^0&&
+ git fetch .. "+refs/*:refs/*"&&
+ git checkout master
+ )&&
+ git ls-remote .>expect&&
+ git ls-remote testrepo>actual&&
+ test_cmp actual expect
+'
+
test_expect_success 'push --porcelain' '
mk_empty&&
echo>.git/foo "To testrepo"&&
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html