[PATCH] Fix git-push --mirror also mirroring refs/remotes/

[PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Bryan Drewery]

git-push --mirror was incorrectly mirroring refs/remotes/
previously. This patch fixes it to follow the documentation
which is to mirror only refs/heads/ and refs/tags/.

Signed-off-by: Bryan Drewery <bryan@shatow.net>
---
 remote.c               |    4 +++-
 t/t5517-push-mirror.sh |   21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/remote.c b/remote.c
index d66e2f3..70cfab8 100644
--- a/remote.c
+++ b/remote.c
@@ -1123,8 +1123,10 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			 * "matching refs"; traditionally we pushed everything
 			 * including refs outside refs/heads/ hierarchy, but
 			 * that does not make much sense these days.
+			 * Push only refs/heads/ and refs/tags/ when mirroring
+			 * otherwise only matching refs/heads/
 			 */
-			if (!send_mirror && prefixcmp(src->name, "refs/heads/"))
+			if ((!send_mirror || prefixcmp(src->name, "refs/tags/")) && prefixcmp(src->name, "refs/heads/"))
 				continue;
 			dst_name = xstrdup(src->name);
 
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index ea49ded..3fc7dc2 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -264,4 +264,25 @@ test_expect_success 'remote.foo.mirror=no has no effect' '
 
 '
 
+# REMOTES test
+test_expect_success 'push mirror does not push remotes' '
+
+	mk_repo_pair &&
+	(
+		cd master &&
+		echo one >foo && git add foo && git commit -m one &&
+		git push --mirror up &&
+		git remote update &&
+		git push --mirror up
+	) &&
+	master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+	mirror_master=$(cd mirror && git show-ref -s --verify refs/heads/master) &&
+	test "$master_master" = "$mirror_master" &&
+	(
+		cd mirror &&
+		invert git show-ref -s --verify refs/remotes/up/master
+	)
+'
+
+
 test_done
-- 
1.6.2.5

Re: [PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Markus Heidelberg]

Bryan Drewery, 10.05.2009:
> git-push --mirror was incorrectly mirroring refs/remotes/

No, that's correct.

> previously. This patch fixes it to follow the documentation
> which is to mirror only refs/heads/ and refs/tags/.

Where have you read this? In the git-push docs at the --mirror
description there is written "all refs under $GIT_DIR/refs/".

Markus

Re: [PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Bryan Drewery]

My git is outdated on my development server, 1.5.6.5:

       --mirror
           Instead of naming each ref to push, specifies that all refs
under $GIT_DIR/refs/heads/ and $GIT_DIR/refs/tags/ be mirrored to the
remote repository. Newly created
           local refs will be pushed to the remote end, locally updated
refs will be force updated on the remote end, and deleted refs will be
removed from the remote end.
           This is the default if the configuration option
remote.<remote>.mirror is set.

So, it's a bug in the docs in 1.5.6.5.

This behavior makes no sense to me. Can we at least add a config option
to make it not push remotes? I push my development repository
to a remote private repository only as a backup, and push my master/next
to a public repository. I see no reason to push my 8 remotes and
all their branches to my backup repository.

Bryan

Markus Heidelberg wrote:
> Bryan Drewery, 10.05.2009:
>   
>> git-push --mirror was incorrectly mirroring refs/remotes/
>>     
>
> No, that's correct.
>
>   
>> previously. This patch fixes it to follow the documentation
>> which is to mirror only refs/heads/ and refs/tags/.
>>     
>
> Where have you read this? In the git-push docs at the --mirror
> description there is written "all refs under $GIT_DIR/refs/".
>
> Markus
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: [PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Markus Heidelberg]

Bryan Drewery, 10.05.2009:
> This behavior makes no sense to me. Can we at least add a config option
> to make it not push remotes? I push my development repository
> to a remote private repository only as a backup, and push my master/next
> to a public repository. I see no reason to push my 8 remotes and
> all their branches to my backup repository.

Why are you disturbed by the remotes, if it's only a backup repository
where you don't look into in the normal case?

Markus

Re: [PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Bryan Drewery]

Markus Heidelberg wrote:
> Bryan Drewery, 10.05.2009:
>   
>> This behavior makes no sense to me. Can we at least add a config option
>> to make it not push remotes? I push my development repository
>> to a remote private repository only as a backup, and push my master/next
>> to a public repository. I see no reason to push my 8 remotes and
>> all their branches to my backup repository.
>>     
>
> Why are you disturbed by the remotes, if it's only a backup repository
> where you don't look into in the normal case?
>
>   
If it's going to push remote heads, shouldn't it (in line with "mirror")
also push all
remote settings for all remotes from the config to the destination
repository, as
well as other config settings?

If not, then why push remote heads at all?

Bryan

Re: [PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

> Bryan Drewery, 10.05.2009:
>> This behavior makes no sense to me. Can we at least add a config option
>> to make it not push remotes? I push my development repository
>> to a remote private repository only as a backup, and push my master/next
>> to a public repository. I see no reason to push my 8 remotes and
>> all their branches to my backup repository.
>
> Why are you disturbed by the remotes, if it's only a backup repository
> where you don't look into in the normal case?

I suspect you got a wrong "backup".

"push --mirror" is about backing up the local repository so that other
people (most likely "you working on some other machine") who usually
interact with the current repository can instead get history from the
repository that you are pushing into, when the current repository is not
reachable for whatever reason.

In other words, "push --mirror" is not a backup in the "archive to tape
for disaster recovery" sense, but is a backup in the "you can go there in
case this is not reachable" sense.  The refspec "remote add --mirror"
creates is set up to support a backup repository in the same sense in the
opposite direction [*1*].

Now, these other people may not have any business looking into remotes
hierarchy of this repository, and we could argue that there is not much
point pushing remotes hierarchy over there (nor stash or topgit for that
matter) in a normal workflow.

But these other people can say "git fetch $here remotes/origin/master"
explicitly when fetching from this repository.  By mirroring everything
under refs/ to its --mirror backup repository, these other people can
substitute $here with its --mirror backup repository and expect it to
still work.  It would be a regression if you stop pushing the refs/remotes
hierarchy, and such a proposal must be backed by an argument that says the
benefit such a change brings in far outweighs the regression it causes.


[Footnote]

*1* If you want a "disaster recovery backup", there is always "tar".  My
recent favourite is to append into an existing squashfs image, though ;-)

Re: [PATCH] Fix git-push --mirror also mirroring refs/remotes/

[Markus Heidelberg]

Junio C Hamano, 10.05.2009:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > Bryan Drewery, 10.05.2009:
> >> This behavior makes no sense to me. Can we at least add a config option
> >> to make it not push remotes? I push my development repository
> >> to a remote private repository only as a backup, and push my master/next
> >> to a public repository. I see no reason to push my 8 remotes and
> >> all their branches to my backup repository.
> >
> > Why are you disturbed by the remotes, if it's only a backup repository
> > where you don't look into in the normal case?
> 
> I suspect you got a wrong "backup".
> 
> [...]
> 
> It would be a regression if you stop pushing the refs/remotes
> hierarchy, and such a proposal must be backed by an argument that says the
> benefit such a change brings in far outweighs the regression it causes.

I'm not sure if this is addressed to me, but I'm not for removing
refs/remotes from push --mirror, maybe I was a bit confusing above.

Markus