error pushing stash ?

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

David Bryson [off-list ref] 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.

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

quoted

 ! [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);

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

From: David Bryson <redacted>

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

+               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

"Sverre Rabbelier" [off-list ref] writes:

quoted

+               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.

From: David Bryson <redacted>

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