error pushing stash ?

error pushing stash ?

[David Bryson]

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

I have a git mirror remote setup on a few of my repositories:

[remote "backup"]
    url = /users/dbryson/backup/janus.git/
    fetch = +refs/heads/*:refs/remotes/origin/*
    receivepack = sudo -u dbryson git-receive-pack
    mirror = 1

I send my refs to the backup with:

$ git push backup

Only to find some odd error messages:

Counting objects: 133, done.
Compressing objects: 100% (109/109), done.
Writing objects: 100% (109/109), 31.25 KiB, done.
Total 109 (delta 82), reused 0 (delta 0)
error: refusing to create funny ref 'refs/stash' remotely
To /users/dbryson/backup/janus.git/
   549f8a4..8e93d51  8654 -> 8654
   ef6195b..549f8a4  origin/8654 -> origin/8654
 + 623e7cb...63d7262 origin/master -> origin/master (forced update)
 ! [remote rejected] refs/stash -> refs/stash (funny refname)
error: failed to push some refs to '/users/dbryson/backup/janus.git/'

Should I be concnerned about this or is it normal ? To be honest the
fact that the stash isn't pushing doesn't bother me.  But maybe it is a
symptom of a larger problem ?

Dave


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

Re: error pushing stash ?

[Shawn O. Pearce]

David Bryson <david@statichacks.org> wrote:
> [remote "backup"]
...
>     mirror = 1
> 
> Only to find some odd error messages:
> 
> $ git push backup
> Counting objects: 133, done.
> Compressing objects: 100% (109/109), done.
> Writing objects: 100% (109/109), 31.25 KiB, done.
> Total 109 (delta 82), reused 0 (delta 0)
> error: refusing to create funny ref 'refs/stash' remotely
> To /users/dbryson/backup/janus.git/
>    549f8a4..8e93d51  8654 -> 8654
>    ef6195b..549f8a4  origin/8654 -> origin/8654
>  + 623e7cb...63d7262 origin/master -> origin/master (forced update)
>  ! [remote rejected] refs/stash -> refs/stash (funny refname)
> error: failed to push some refs to '/users/dbryson/backup/janus.git/'

refs/stash is a funny refname because it contains only 1 '/'.
Normally a valid ref has at least 2 '/', e.g. refs/heads/8654 or
refs/tags/v1.0.

Naming the stash refs/stash was perhaps funny in the first place
since it cannot be moved about on the transport protocol, but then
again the bulk of the stash data is actually in the reflog for the
stash (and not the stash ref itself) so there is basically no point
in pushing or fetching a stash directly.

-- 
Shawn.

Re: error pushing stash ?

[Jeff King]

On Mon, Oct 06, 2008 at 05:40:51PM -0700, Shawn O. Pearce wrote:

> >  ! [remote rejected] refs/stash -> refs/stash (funny refname)
> > error: failed to push some refs to '/users/dbryson/backup/janus.git/'
> 
> refs/stash is a funny refname because it contains only 1 '/'.
> Normally a valid ref has at least 2 '/', e.g. refs/heads/8654 or
> refs/tags/v1.0.

Since no version of receive-pack accepts these "funny refs", perhaps we
should mirror the check when considering the list of refs to send. IOW,
don't even make them eligible for matching or mirroring. Patch is below.

> Naming the stash refs/stash was perhaps funny in the first place
> since it cannot be moved about on the transport protocol, but then
> again the bulk of the stash data is actually in the reflog for the
> stash (and not the stash ref itself) so there is basically no point
> in pushing or fetching a stash directly.

I agree there is not much point in pushing it, since the useful bit is
in the reflog. So perhaps a "funny" refname is a good place to put it,
since it easily tells us that it is not a useful thing to push.

---
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index bbf6e0a..298bd71 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -140,7 +140,13 @@ static struct ref *remote_refs, **remote_tail;
 static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct ref *ref;
-	int len = strlen(refname) + 1;
+	int len;
+
+	/* we already know it starts with refs/ to get here */
+	if (check_ref_format(refname + 5))
+		return 0;
+
+	len = strlen(refname) + 1;
 	ref = xcalloc(1, sizeof(*ref) + len);
 	hashcpy(ref->new_sha1, sha1);
 	memcpy(ref->name, refname, len);

Re: error pushing stash ?

[Jeff King]

On Tue, Oct 28, 2008 at 05:17:55PM -0400, Jeff King wrote:

> Since no version of receive-pack accepts these "funny refs", perhaps we
> should mirror the check when considering the list of refs to send. IOW,
> don't even make them eligible for matching or mirroring. Patch is below.
> [...]
> +	/* we already know it starts with refs/ to get here */
> +	if (check_ref_format(refname + 5))
> +		return 0;

It occurs to me that since I didn't give a good commit message, and
since I replied to a several-weeks-old message, this might be confusing.
But what I am suggesting is that git-push should not bother trying to
send something that it knows git-receive-pack will refuse. So this check
goes into builtin-send-pack.c, and is an exact mirror of the one in
builtin-receive-pack.c:

 $ sed -n 177,181p builtin-receive-pack.c
        /* only refs/... are allowed */
        if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
                error("refusing to create funny ref '%s' remotely", name);
                return "funny refname";
        }

-Peff

[PATCH] Added test case for mirror to not push stash refs

[david]

From: David Bryson <david@statichacks.org>

This test case checks to make sure mirror does not push stashed refs

---
 t/t5517-push-mirror.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index ea49ded..bb263cd 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -123,6 +123,16 @@ test_expect_success 'push mirror adds, updates and removes branches together' '
 
 '
 
+test_expect_success 'push mirror does not push stash refs' '
+
+	mk_repo_pair &&
+	(
+		cd master &&
+		echo foo >foo && git add foo && git commit -m 'foo' &&
+		echo bar >foo && git stash save 'WIP' &&
+		git push --mirror up
+	)
+'
 
 # TAG tests
 test_expect_success 'push mirror creates new tags' '
-- 
1.6.0.1

Re: [PATCH] Added test case for mirror to not push stash refs

[Sverre Rabbelier]

> +               git push --mirror up
> +       )
> +'

I don't quite get how this works, I don't see a test here anywhere to
actually test that the stash refs were not pushed?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] Added test case for mirror to not push stash refs

[Junio C Hamano]

"Sverre Rabbelier" <alturin@gmail.com> writes:

>> +               git push --mirror up
>> +       )
>> +'
>
> I don't quite get how this works, I don't see a test here anywhere to
> actually test that the stash refs were not pushed?

I agree that this test should check the receiving end.

The patch is relying on the fact that the receiving end would reject the
push if the sending end tries to push refs/$foo where $foo does not have
any slash.

[PATCH] Added test case for mirror to not push stash refs

[david]

From: David Bryson <david@statichacks.org>

This test case checks to make sure mirror does not push stashed refs

---
 t/t5517-push-mirror.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index ea49ded..bb263cd 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -123,6 +123,16 @@ test_expect_success 'push mirror adds, updates and removes branches together' '
 
 '
 
+test_expect_success 'push mirror does not push stash refs' '
+
+	mk_repo_pair &&
+	(
+		cd master &&
+		echo foo >foo && git add foo && git commit -m 'foo' &&
+		echo bar >foo && git stash save 'WIP' &&
+		git push --mirror up
+	)
+'
 
 # TAG tests
 test_expect_success 'push mirror creates new tags' '
-- 
1.6.0.1