threads / patch / 19276

patchFix git-push --mirror also mirroring refs/remotes/

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

## tl;dr

7 messages between May 10, 2009 and May 10, 2009. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Bryan Drewery· May 10, 2009, 19:35 UTC · lore

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(-)

Show changes to 2 files +24 −1

remote.c, t/t5517-push-mirror.sh

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

Markus Heidelberg· May 10, 2009, 20:25 UTC · re: Bryan Drewery · lore

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

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

Bryan Drewery· May 10, 2009, 20:50 UTC · re: Markus Heidelberg · lore

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

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:

Show 22 quoted lines

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

Markus Heidelberg· May 10, 2009, 21:13 UTC · re: Bryan Drewery · lore

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

Bryan Drewery, 10.05.2009:

Show 5 quoted lines

> 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

Bryan Drewery· May 10, 2009, 21:31 UTC · re: Markus Heidelberg · lore

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

Markus Heidelberg wrote:

Show 13 quoted lines

> 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

Junio C Hamano· May 10, 2009, 21:56 UTC · re: Markus Heidelberg · lore

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

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

Show 9 quoted lines

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

Markus Heidelberg· May 10, 2009, 22:15 UTC · re: Junio C Hamano · lore

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

Junio C Hamano, 10.05.2009:

Show 19 quoted lines

> 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