[RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Git-mediawiki's "dumb push" sends the local revisions to the remote wiki,
but does not update the local metadata to reflect the push (hence, the
next pull will have to re-import the exported revisions).

The previous implementation was simply omitting the update to the private
ref after a dumb push. This was broken by 664059fb62 (Felipe Contreras,
Apr 17 2013, transport-helper: update remote helper namespace), which
does an automatic update of the private ref (not just the
remote-tracking) on push.

This patch fixes git-remote-mediawiki to reset the private ref after the
push is completed, cancelling the automatic update triggered by
664059fb62.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Felipe: Is this the right fix for git-remote-mediawiki? Any better idea?

Thanks,

 contrib/mw-to-git/git-remote-mediawiki.perl | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index f8d7d2c..13919ad 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -53,6 +53,7 @@ if (@ARGV != 2) {
 
 my $remotename = $ARGV[0];
 my $url = $ARGV[1];
+my $reset_private_ref_to = undef;
 
 # Accept both space-separated and multiple keys in config file.
 # Spaces should be written as _ anyway because we'll use chomp.
@@ -161,6 +162,9 @@ sub parse_command {
 	my ($line) = @_;
 	my @cmd = split(/ /, $line);
 	if (!defined $cmd[0]) {
+		if ($reset_private_ref_to) {
+			run_git("update-ref -m \"Git-MediaWiki non-dumb push\" refs/mediawiki/$remotename/master $reset_private_ref_to");
+		}
 		return 0;
 	}
 	if ($cmd[0] eq 'capabilities') {
@@ -1209,9 +1213,10 @@ sub mw_push_revision {
 				die("Unknown error from mw_push_file()\n");
 			}
 		}
-		if (!$dumb_push) {
+		if ($dumb_push) {
+			$reset_private_ref_to = $remoteorigin_sha1;
+		} else {
 			run_git(qq(notes --ref=${remotename}/mediawiki add -f -m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
-			run_git(qq(update-ref -m "Git-MediaWiki push" refs/mediawiki/${remotename}/master ${sha1_commit} ${sha1_child}));
 		}
 	}
 
-- 
1.8.2.1.349.ge888d28.dirty

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Felipe Contreras]

On Tue, Aug 13, 2013 at 8:31 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>
> Git-mediawiki's "dumb push" sends the local revisions to the remote wiki,
> but does not update the local metadata to reflect the push (hence, the
> next pull will have to re-import the exported revisions).
>
> The previous implementation was simply omitting the update to the private
> ref after a dumb push. This was broken by 664059fb62 (Felipe Contreras,
> Apr 17 2013, transport-helper: update remote helper namespace), which
> does an automatic update of the private ref (not just the
> remote-tracking) on push.
>
> This patch fixes git-remote-mediawiki to reset the private ref after the
> push is completed, cancelling the automatic update triggered by
> 664059fb62.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Felipe: Is this the right fix for git-remote-mediawiki? Any better idea?

Why not keep track of the revisions yourself? You can have file where
you store which was the last revision that was fetched.

-- 
Felipe Contreras

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, Aug 13, 2013 at 8:31 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>
>> Felipe: Is this the right fix for git-remote-mediawiki? Any better idea?
>
> Why not keep track of the revisions yourself? You can have file where
> you store which was the last revision that was fetched.

I don't really understand the point of the "private namespace" anymore I
guess. Why do we have both refs/remotes/$remote and
refs/$foreign_vcs/$remote, if they are always kept in sync?

Keeping the last imported revision in a separate file would be possible,
but then we'd have information about the remote in one file plus two
refs, and I don't understand why we need to split the information in so
many places. A ref seemed the right tool to store a revision.

(These are genuine questions, you have far more experience than me with
remote-helpers)

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Felipe Contreras]

On Wed, Aug 21, 2013 at 4:36 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Aug 13, 2013 at 8:31 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>>
>>> Felipe: Is this the right fix for git-remote-mediawiki? Any better idea?
>>
>> Why not keep track of the revisions yourself? You can have file where
>> you store which was the last revision that was fetched.
>
> I don't really understand the point of the "private namespace" anymore I
> guess. Why do we have both refs/remotes/$remote and
> refs/$foreign_vcs/$remote, if they are always kept in sync?

They are not always in sync; if a push fails, the private namespace is
not updated.

> Keeping the last imported revision in a separate file would be possible,
> but then we'd have information about the remote in one file plus two
> refs, and I don't understand why we need to split the information in so
> many places. A ref seemed the right tool to store a revision.

As I said, they are not exactly the same. It is possible refs/remotes
point to a mercurial revision on the remote server, and refs/hg points
to a mercurial revision on the local internal repository, and they are
not the same.

-- 
Felipe Contreras

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Aug 21, 2013 at 4:36 PM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> On Tue, Aug 13, 2013 at 8:31 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>>>
>>>> Felipe: Is this the right fix for git-remote-mediawiki? Any better idea?
>>>
>>> Why not keep track of the revisions yourself? You can have file where
>>> you store which was the last revision that was fetched.
>>
>> I don't really understand the point of the "private namespace" anymore I
>> guess. Why do we have both refs/remotes/$remote and
>> refs/$foreign_vcs/$remote, if they are always kept in sync?
>
> They are not always in sync; if a push fails, the private namespace is
> not updated.
[...]
> As I said, they are not exactly the same. It is possible refs/remotes
> point to a mercurial revision on the remote server, and refs/hg points
> to a mercurial revision on the local internal repository, and they are
> not the same.

This is assuming you follow the scheme

  git -> local repo for other vcs -> remote repo for other vcs

which itself more or less assumes that the other VCS is a decentralized
VCS. I understand this is a good idea for hg or bzr remote helpers, but
not applicable for git-remote-mediawiki.

I find this very unclear in the doc. How about adding something like
this in the "'refspec' <refspec>::" part?

--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -176,6 +176,12 @@ applicable refspec takes precedence.  The left-hand of refspecs
 advertised with this capability must cover all refs reported by
 the list command.  If no 'refspec' capability is advertised,
 there is an implied `refspec *:*`.
++
+When writing remote-helpers for decentralized version control
+systems, it is advised to keep a local copy of the repository to
+interact with, and to let the private namespace refs point to this
+local repository, while the refs/remotes namespace is used to track
+the remote repository.
 
 'bidi-import'::
        This modifies the 'import' capability.

So, now, I understand the point of the private namespace in the case of
DVCS, but still note really for interactions with centralized VCS.

Back to my original problem: the point of dumb push is to make sure the
next import will re-import the pushed revisions. if I use something
other than the private namespace to keep track of the last imported,
then I'll need to do a non-fast forward update to the private ref. I
could do that by sending "feature force\n" it the beginning of the
fast-import stream when importing, but that loses one safety feature
(e.g. I detected the breakage thanks to the non-fast forward error
message, while the tests incorrectly pass if I enable "force").

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Felipe Contreras]

On Fri, Aug 23, 2013 at 3:25 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:

> This is assuming you follow the scheme
>
>   git -> local repo for other vcs -> remote repo for other vcs
>
> which itself more or less assumes that the other VCS is a decentralized
> VCS. I understand this is a good idea for hg or bzr remote helpers, but
> not applicable for git-remote-mediawiki.

A centralized repository is a subset of decentralized workflows.

> Back to my original problem: the point of dumb push is to make sure the
> next import will re-import the pushed revisions. if I use something
> other than the private namespace to keep track of the last imported,
> then I'll need to do a non-fast forward update to the private ref. I
> could do that by sending "feature force\n" it the beginning of the
> fast-import stream when importing, but that loses one safety feature
> (e.g. I detected the breakage thanks to the non-fast forward error
> message, while the tests incorrectly pass if I enable "force").

I don't know if a dumb push is the right thing to do here, but
supposing it is, you can still report non-fast-forward errors:

https://github.com/felipec/git/commit/0f7f0a223d710d29a4f1a03fc27348a8690d8a19
https://github.com/felipec/git/commit/b67a8914bc1bb3ad23591875611165b84135aaf9

If it's too much hassle to find non-fast-forward situations by
yourself, then perhaps it would make sense to update the remote
namespace only when a certain feature has been flagged.

-- 
Felipe Contreras

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Fri, Aug 23, 2013 at 3:25 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>
>> This is assuming you follow the scheme
>>
>>   git -> local repo for other vcs -> remote repo for other vcs
>>
>> which itself more or less assumes that the other VCS is a decentralized
>> VCS. I understand this is a good idea for hg or bzr remote helpers, but
>> not applicable for git-remote-mediawiki.
>
> A centralized repository is a subset of decentralized workflows.

Yes, a strict subset. You can't keep a local copy of a remote repository
with a centralized VCS, so the way to write remote-helpers for DVCS does
not apply for centralized VCS.

> I don't know if a dumb push is the right thing to do here, but
> supposing it is, you can still report non-fast-forward errors:

That would be really complicated, as you would need to check whether
non-fast forward errors are serious. The check is done on pull, and the
reason the check should be disabled is dumb push.

> https://github.com/felipec/git/commit/0f7f0a223d710d29a4f1a03fc27348a8690d8a19
> https://github.com/felipec/git/commit/b67a8914bc1bb3ad23591875611165b84135aaf9

Err, don't they apply when pushing? In my case, the non-fast forward
occurs on pull.

> If it's too much hassle to find non-fast-forward situations by
> yourself, then perhaps it would make sense to update the remote
> namespace only when a certain feature has been flagged.

How would I do that? The update to the remote namespace is done by Git,
not by the remote-helper.

OK, I'm now convinced that my solution is the right one. The
alternatives are far more complex and I still fail to see the benefits.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> How would I do that? The update to the remote namespace is done by Git,
> not by the remote-helper.
>
> OK, I'm now convinced that my solution is the right one. The
> alternatives are far more complex and I still fail to see the benefits.

Sounds like a plan, even though it smells like the "update is done
by Git" that does not have any way to opt-out may be the real design
mistake and your "solution" is a work-around to that.

Would it be a possibility to make it tunable, perhaps by introducing
a capability on the remote-interface side that allows you to tell it
not to mess with the remote namespace?  If we were doing this from
scratch, I would suspect that we would have made the capability work
the other way around (Git does not do the update by default, but
helpers c an ask Git to do so).

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> How would I do that? The update to the remote namespace is done by Git,
>> not by the remote-helper.
>>
>> OK, I'm now convinced that my solution is the right one. The
>> alternatives are far more complex and I still fail to see the benefits.
>
> Sounds like a plan, even though it smells like the "update is done
> by Git" that does not have any way to opt-out may be the real design
> mistake and your "solution" is a work-around to that.
>
> Would it be a possibility to make it tunable, perhaps by introducing
> a capability on the remote-interface side that allows you to tell it
> not to mess with the remote namespace?

Ideally, it would be possible to ask for a non-update without a fatal
error on old Git versions, but this is not possible (hence, my fix is
the "portable" one, that works on Git 1.8.4).

But that's probably the best we can do now.

> If we were doing this from scratch, I would suspect that we would have
> made the capability work the other way around (Git does not do the
> update by default, but helpers c an ask Git to do so).

Not updating was the default until 664059fb62 (i.e. until 1.8.4), so the
default already changed once. I tend to agree with Felipe that doing the
update by default makes sense.

git-remote-mediawiki is kind of a special case, as the remote side uses
a very different data model, hence it can make sense to export+reimport
to get locally what the remote side received. But when interacting with
something closer to Git (bzr, hg, ...), the mapping should be close to
1-1 and re-importing wouldn't make sense.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> How would I do that? The update to the remote namespace is done by Git,
>>> not by the remote-helper.
>>>
>>> OK, I'm now convinced that my solution is the right one. The
>>> alternatives are far more complex and I still fail to see the benefits.
>>
>> Sounds like a plan, even though it smells like the "update is done
>> by Git" that does not have any way to opt-out may be the real design
>> mistake and your "solution" is a work-around to that.
>>
>> Would it be a possibility to make it tunable, perhaps by introducing
>> a capability on the remote-interface side that allows you to tell it
>> not to mess with the remote namespace?
>
> Ideally, it would be possible to ask for a non-update without a fatal
> error on old Git versions, but this is not possible (hence, my fix is
> the "portable" one, that works on Git 1.8.4).
>
> But that's probably the best we can do now.

... and a patch implementing that would look like:

commit e438ddc58a3cedcf66332bddda792954c5905cea
Author: Matthieu Moy <Matthieu.Moy@imag.fr>
Date:   Mon Aug 26 11:10:30 2013 +0200

    transport-helper: add dont-update-private capability

diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..639b0e3 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
                push : 1,
                connect : 1,
                signed_tags : 1,
-               no_disconnect_req : 1;
+               no_disconnect_req : 1,
+               dont_update_private : 1;
        char *export_marks;
        char *import_marks;
        /* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport *transport)
                        strbuf_addstr(&arg, "--import-marks=");
                        strbuf_addstr(&arg, capname + strlen("import-marks "));
                        data->import_marks = strbuf_detach(&arg, NULL);
+               } else if (!prefixcmp(capname, "dont-update-private")) {
+                       data->dont_update_private = 1;
                } else if (mandatory) {
                        die("Unknown mandatory capability %s. This remote "
                            "helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data *data,
                if (push_update_ref_status(&buf, &ref, remote_refs))
                        continue;
 
-               if (!data->refspecs)
+               if (!data->refspecs || data->dont_update_private)
                        continue;
 
                /* propagate back the update to the remote namespace */


(Plus tests and docs)

Based on this, git-remote-mediawiki could simply use:

commit aa745c3ccb6681df8dc6406b00e0d31b26e72d50
Author: Matthieu Moy <Matthieu.Moy@imag.fr>
Date:   Mon Aug 26 11:09:55 2013 +0200

    git-remote-mediawiki: use dont-update-private capability on dumb push

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index a2e71d6..b29682e 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -601,6 +601,9 @@ sub mw_capabilities {
        print {*STDOUT} "import\n";
        print {*STDOUT} "list\n";
        print {*STDOUT} "push\n";
+       if ($dumb_push) {
+               print {*STDOUT} "dont-update-private\n";
+       }
        print {*STDOUT} "\n";
        return;
 }

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Felipe Contreras]

On Mon, Aug 26, 2013 at 4:16 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>>
>>>> How would I do that? The update to the remote namespace is done by Git,
>>>> not by the remote-helper.
>>>>
>>>> OK, I'm now convinced that my solution is the right one. The
>>>> alternatives are far more complex and I still fail to see the benefits.
>>>
>>> Sounds like a plan, even though it smells like the "update is done
>>> by Git" that does not have any way to opt-out may be the real design
>>> mistake and your "solution" is a work-around to that.
>>>
>>> Would it be a possibility to make it tunable, perhaps by introducing
>>> a capability on the remote-interface side that allows you to tell it
>>> not to mess with the remote namespace?
>>
>> Ideally, it would be possible to ask for a non-update without a fatal
>> error on old Git versions, but this is not possible (hence, my fix is
>> the "portable" one, that works on Git 1.8.4).
>>
>> But that's probably the best we can do now.
>
> ... and a patch implementing that would look like:

This is exactly what I meant by only update when a feature has been flagged.

-- 
Felipe Contreras

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Mon, Aug 26, 2013 at 4:16 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>>
>>> Ideally, it would be possible to ask for a non-update without a fatal
>>> error on old Git versions, but this is not possible (hence, my fix is
>>> the "portable" one, that works on Git 1.8.4).
>>>
>>> But that's probably the best we can do now.
>>
>> ... and a patch implementing that would look like:
>
> This is exactly what I meant by only update when a feature has been
> flagged.

OK, I didn't understand you meant patching Git itself for that. So it
makes sense to turn my toy patch into a real patch I guess. Any comment
on the capability name? I used dont-update-private, which is a bit long.
The actual precise name would be dont-update-private-for-push, but
that's really long. Any better idea?

Just to be sure: you originally wrote "update the remote namespace only
when a certain feature has been flagged". My patch differs in two ways:
it's opt-out, not opt-in, and it's about updating the _private_
namespace, not the remote. Any comment on what's the best behavior?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH 1/4] git-remote-mediawiki: add test and check Makefile targets

[Matthieu Moy]

There are a few level 4 and 2 perlcritic issues in the current code. We
make level 5 fatal, and keep level 2 as warnings.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 contrib/mw-to-git/Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 76fcd4d..f206f96 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -24,6 +24,11 @@ INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \
 
 all: build
 
+test: all
+	$(MAKE) -C t
+
+check: perlcritic test
+
 install_pm:
 	install $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
 
@@ -41,4 +46,7 @@ clean:
 	rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
 
 perlcritic:
-	perlcritic -2 *.perl
+	perlcritic -5 $(SCRIPT_PERL)
+	-perlcritic -2 $(SCRIPT_PERL)
+
+.PHONY: all test check install_pm install clean perlcritic
-- 
1.8.4.12.g98a4f55.dirty

[PATCH 2/4] transport-helper: add dont-update-private capability

[Matthieu Moy]

Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
remote helper namespace), a 'push' operation on a remote helper updates
the private ref by default. This is often a good thing, but it can also
be desirable to disable this update to force the next 'pull' to re-import
the pushed revisions.

Allow remote-helpers to disable the automatic update by introducing a new
capability.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Essentially the same as the patch I sent earlier, with doc and test.

 Documentation/gitremote-helpers.txt |  6 ++++++
 git-remote-testgit.sh               |  1 +
 t/t5801-remote-helpers.sh           | 11 +++++++++++
 transport-helper.c                  |  7 +++++--
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 0827f69..3085823 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -120,6 +120,12 @@ connecting (see the 'connect' command under COMMANDS).
 When choosing between 'push' and 'export', Git prefers 'push'.
 Other frontends may have some other order of preference.
 
+'dont-update-private'::
+	When using the 'refspec' capability, git normally updates the
+	private ref on successful push. This update is disabled when
+	the remote-helper declares the capability
+	'dont-update-private'.
+
 
 Capabilities for Fetching
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 2109070..a81aee8 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -38,6 +38,7 @@ do
 			echo "*export-marks $gitmarks"
 		fi
 		test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
+		test -n "$GIT_REMOTE_TESTGIT_DONT_UPDATE_PRIVATE" && echo "dont-update-private"
 		echo
 		;;
 	list)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..171cae3 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,6 +182,17 @@ test_expect_success 'push update refs' '
 	)
 '
 
+test_expect_success 'push update refs disabled by dont-update-private' '
+	(cd local &&
+	echo more-update >>file &&
+	git commit -a -m more-update &&
+	git rev-parse --verify testgit/origin/heads/update >expect &&
+	GIT_REMOTE_TESTGIT_DONT_UPDATE_PRIVATE=t git push origin update &&
+	git rev-parse --verify testgit/origin/heads/update >actual &&
+	test_cmp expect actual
+	)
+'
+
 test_expect_success 'push update refs failure' '
 	(cd local &&
 	git checkout update &&
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..639b0e3 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
 		push : 1,
 		connect : 1,
 		signed_tags : 1,
-		no_disconnect_req : 1;
+		no_disconnect_req : 1,
+		dont_update_private : 1;
 	char *export_marks;
 	char *import_marks;
 	/* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport *transport)
 			strbuf_addstr(&arg, "--import-marks=");
 			strbuf_addstr(&arg, capname + strlen("import-marks "));
 			data->import_marks = strbuf_detach(&arg, NULL);
+		} else if (!prefixcmp(capname, "dont-update-private")) {
+			data->dont_update_private = 1;
 		} else if (mandatory) {
 			die("Unknown mandatory capability %s. This remote "
 			    "helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs)
+		if (!data->refspecs || data->dont_update_private)
 			continue;
 
 		/* propagate back the update to the remote namespace */
-- 
1.8.4.12.g98a4f55.dirty

Re: [PATCH 2/4] transport-helper: add dont-update-private capability

[Felipe Contreras]

On Thu, Aug 29, 2013 at 1:58 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
> remote helper namespace), a 'push' operation on a remote helper updates
> the private ref by default. This is often a good thing, but it can also
> be desirable to disable this update to force the next 'pull' to re-import
> the pushed revisions.
>
> Allow remote-helpers to disable the automatic update by introducing a new
> capability.

Looks good to me, but how about 'no-private-update'?

-- 
Felipe Contreras

[PATCH v2 1/4] git-remote-mediawiki: add test and check Makefile targets

[Matthieu Moy]

There are a few level 4 and 2 perlcritic issues in the current code. We
make level 5 fatal, and keep level 2 as warnings.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 contrib/mw-to-git/Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 76fcd4d..f206f96 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -24,6 +24,11 @@ INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \
 
 all: build
 
+test: all
+	$(MAKE) -C t
+
+check: perlcritic test
+
 install_pm:
 	install $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
 
@@ -41,4 +46,7 @@ clean:
 	rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
 
 perlcritic:
-	perlcritic -2 *.perl
+	perlcritic -5 $(SCRIPT_PERL)
+	-perlcritic -2 $(SCRIPT_PERL)
+
+.PHONY: all test check install_pm install clean perlcritic
-- 
1.8.4.12.g98a4f55.dirty

[PATCH v2 2/4] transport-helper: add no-private-update capability

[Matthieu Moy]

Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
remote helper namespace), a 'push' operation on a remote helper updates
the private ref by default. This is often a good thing, but it can also
be desirable to disable this update to force the next 'pull' to re-import
the pushed revisions.

Allow remote-helpers to disable the automatic update by introducing a new
capability.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Change since v1: just changed the capability name.

 Documentation/gitremote-helpers.txt |  6 ++++++
 git-remote-testgit.sh               |  1 +
 t/t5801-remote-helpers.sh           | 11 +++++++++++
 transport-helper.c                  |  7 +++++--
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 0827f69..1eacf1e 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -120,6 +120,12 @@ connecting (see the 'connect' command under COMMANDS).
 When choosing between 'push' and 'export', Git prefers 'push'.
 Other frontends may have some other order of preference.
 
+'no-private-update'::
+	When using the 'refspec' capability, git normally updates the
+	private ref on successful push. This update is disabled when
+	the remote-helper declares the capability
+	'no-private-update'.
+
 
 Capabilities for Fetching
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 2109070..6d2f282 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -38,6 +38,7 @@ do
 			echo "*export-marks $gitmarks"
 		fi
 		test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
+		test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
 		echo
 		;;
 	list)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..613f69a 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,6 +182,17 @@ test_expect_success 'push update refs' '
 	)
 '
 
+test_expect_success 'push update refs disabled by no-private-update' '
+	(cd local &&
+	echo more-update >>file &&
+	git commit -a -m more-update &&
+	git rev-parse --verify testgit/origin/heads/update >expect &&
+	GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE=t git push origin update &&
+	git rev-parse --verify testgit/origin/heads/update >actual &&
+	test_cmp expect actual
+	)
+'
+
 test_expect_success 'push update refs failure' '
 	(cd local &&
 	git checkout update &&
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..3328394 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
 		push : 1,
 		connect : 1,
 		signed_tags : 1,
-		no_disconnect_req : 1;
+		no_disconnect_req : 1,
+		no_private_update : 1;
 	char *export_marks;
 	char *import_marks;
 	/* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport *transport)
 			strbuf_addstr(&arg, "--import-marks=");
 			strbuf_addstr(&arg, capname + strlen("import-marks "));
 			data->import_marks = strbuf_detach(&arg, NULL);
+		} else if (!prefixcmp(capname, "no-private-update")) {
+			data->no_private_update = 1;
 		} else if (mandatory) {
 			die("Unknown mandatory capability %s. This remote "
 			    "helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs)
+		if (!data->refspecs || data->no_private_update)
 			continue;
 
 		/* propagate back the update to the remote namespace */
-- 
1.8.4.12.g98a4f55.dirty

Re: [PATCH v2 2/4] transport-helper: add no-private-update capability

[Felipe Contreras]

On Mon, Sep 2, 2013 at 2:19 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
> remote helper namespace), a 'push' operation on a remote helper updates
> the private ref by default. This is often a good thing, but it can also
> be desirable to disable this update to force the next 'pull' to re-import
> the pushed revisions.
>
> Allow remote-helpers to disable the automatic update by introducing a new
> capability.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>

Acked-by: Felipe Contreras <felipe.contreras@gmail.com>

Or Reviewed-by, or whatever.

> ---
> Change since v1: just changed the capability name.
>
>  Documentation/gitremote-helpers.txt |  6 ++++++
>  git-remote-testgit.sh               |  1 +
>  t/t5801-remote-helpers.sh           | 11 +++++++++++
>  transport-helper.c                  |  7 +++++--
>  4 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
> index 0827f69..1eacf1e 100644
> --- a/Documentation/gitremote-helpers.txt
> +++ b/Documentation/gitremote-helpers.txt
> @@ -120,6 +120,12 @@ connecting (see the 'connect' command under COMMANDS).
>  When choosing between 'push' and 'export', Git prefers 'push'.
>  Other frontends may have some other order of preference.
>
> +'no-private-update'::
> +       When using the 'refspec' capability, git normally updates the
> +       private ref on successful push. This update is disabled when
> +       the remote-helper declares the capability
> +       'no-private-update'.
> +
>

There's an extra unnecessary space here, no?

-- 
Felipe Contreras

[PATCH v3 2/4] transport-helper: add dont-update-private capability

[Matthieu Moy]

Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
remote helper namespace), a 'push' operation on a remote helper updates
the private ref by default. This is often a good thing, but it can also
be desirable to disable this update to force the next 'pull' to re-import
the pushed revisions.

Allow remote-helpers to disable the automatic update by introducing a new
capability.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> There's an extra unnecessary space here, no?

Indeed (the new option is shorter, and I didn't rewrap). It doesn't
really matter, but in case, here's an updated version.

 Documentation/gitremote-helpers.txt |  5 +++++
 git-remote-testgit.sh               |  1 +
 t/t5801-remote-helpers.sh           | 11 +++++++++++
 transport-helper.c                  |  7 +++++--
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 0827f69..ee9e134 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -120,6 +120,11 @@ connecting (see the 'connect' command under COMMANDS).
 When choosing between 'push' and 'export', Git prefers 'push'.
 Other frontends may have some other order of preference.
 
+'no-private-update'::
+	When using the 'refspec' capability, git normally updates the
+	private ref on successful push. This update is disabled when
+	the remote-helper declares the capability 'no-private-update'.
+
 
 Capabilities for Fetching
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 2109070..6d2f282 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -38,6 +38,7 @@ do
 			echo "*export-marks $gitmarks"
 		fi
 		test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
+		test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
 		echo
 		;;
 	list)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..613f69a 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,6 +182,17 @@ test_expect_success 'push update refs' '
 	)
 '
 
+test_expect_success 'push update refs disabled by no-private-update' '
+	(cd local &&
+	echo more-update >>file &&
+	git commit -a -m more-update &&
+	git rev-parse --verify testgit/origin/heads/update >expect &&
+	GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE=t git push origin update &&
+	git rev-parse --verify testgit/origin/heads/update >actual &&
+	test_cmp expect actual
+	)
+'
+
 test_expect_success 'push update refs failure' '
 	(cd local &&
 	git checkout update &&
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..3328394 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
 		push : 1,
 		connect : 1,
 		signed_tags : 1,
-		no_disconnect_req : 1;
+		no_disconnect_req : 1,
+		no_private_update : 1;
 	char *export_marks;
 	char *import_marks;
 	/* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport *transport)
 			strbuf_addstr(&arg, "--import-marks=");
 			strbuf_addstr(&arg, capname + strlen("import-marks "));
 			data->import_marks = strbuf_detach(&arg, NULL);
+		} else if (!prefixcmp(capname, "no-private-update")) {
+			data->no_private_update = 1;
 		} else if (mandatory) {
 			die("Unknown mandatory capability %s. This remote "
 			    "helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs)
+		if (!data->refspecs || data->no_private_update)
 			continue;
 
 		/* propagate back the update to the remote namespace */
-- 
1.8.4.12.g98a4f55.dirty

[PATCH v4 2/4] transport-helper: add no-private-update capability

[Matthieu Moy]

Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
remote helper namespace), a 'push' operation on a remote helper updates
the private ref by default. This is often a good thing, but it can also
be desirable to disable this update to force the next 'pull' to re-import
the pushed revisions.

Allow remote-helpers to disable the automatic update by introducing a new
capability.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

I feel really stupid for sending so many versions now :-(. v3 was
fixing a whitespace issue, but forgot to re-apply the change I did
manually while sending v2, hence the commit message was wrong
(dont-update-private -> no-private-update).

Not terribly important, but if it's still time, this one should be
correct.

 Documentation/gitremote-helpers.txt |  5 +++++
 git-remote-testgit.sh               |  1 +
 t/t5801-remote-helpers.sh           | 11 +++++++++++
 transport-helper.c                  |  7 +++++--
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 0827f69..ee9e134 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -120,6 +120,11 @@ connecting (see the 'connect' command under COMMANDS).
 When choosing between 'push' and 'export', Git prefers 'push'.
 Other frontends may have some other order of preference.
 
+'no-private-update'::
+	When using the 'refspec' capability, git normally updates the
+	private ref on successful push. This update is disabled when
+	the remote-helper declares the capability 'no-private-update'.
+
 
 Capabilities for Fetching
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 2109070..6d2f282 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -38,6 +38,7 @@ do
 			echo "*export-marks $gitmarks"
 		fi
 		test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
+		test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
 		echo
 		;;
 	list)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..613f69a 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,6 +182,17 @@ test_expect_success 'push update refs' '
 	)
 '
 
+test_expect_success 'push update refs disabled by no-private-update' '
+	(cd local &&
+	echo more-update >>file &&
+	git commit -a -m more-update &&
+	git rev-parse --verify testgit/origin/heads/update >expect &&
+	GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE=t git push origin update &&
+	git rev-parse --verify testgit/origin/heads/update >actual &&
+	test_cmp expect actual
+	)
+'
+
 test_expect_success 'push update refs failure' '
 	(cd local &&
 	git checkout update &&
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..3328394 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -27,7 +27,8 @@ struct helper_data {
 		push : 1,
 		connect : 1,
 		signed_tags : 1,
-		no_disconnect_req : 1;
+		no_disconnect_req : 1,
+		no_private_update : 1;
 	char *export_marks;
 	char *import_marks;
 	/* These go from remote name (as in "list") to private name */
@@ -205,6 +206,8 @@ static struct child_process *get_helper(struct transport *transport)
 			strbuf_addstr(&arg, "--import-marks=");
 			strbuf_addstr(&arg, capname + strlen("import-marks "));
 			data->import_marks = strbuf_detach(&arg, NULL);
+		} else if (!prefixcmp(capname, "no-private-update")) {
+			data->no_private_update = 1;
 		} else if (mandatory) {
 			die("Unknown mandatory capability %s. This remote "
 			    "helper probably needs newer version of Git.",
@@ -723,7 +726,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs)
+		if (!data->refspecs || data->no_private_update)
 			continue;
 
 		/* propagate back the update to the remote namespace */
-- 
1.8.4.12.g98a4f55.dirty

[PATCH v2 3/4] git-remote-mediawiki: use no-private-update capability on dumb push

[Matthieu Moy]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index f8d7d2c..22300a1 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -590,6 +590,9 @@ sub mw_capabilities {
 	print {*STDOUT} "import\n";
 	print {*STDOUT} "list\n";
 	print {*STDOUT} "push\n";
+	if ($dumb_push) {
+		print {*STDOUT} "no-private-update\n";
+	}
 	print {*STDOUT} "\n";
 	return;
 }
-- 
1.8.4.12.g98a4f55.dirty

[PATCH v2 4/4] git-remote-mediawiki: no need to update private ref in non-dumb push

[Matthieu Moy]

We used to update the private ref ourselves, but this update is now done
by default (since 664059fb62).

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 22300a1..c9a4805 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1214,7 +1214,6 @@ sub mw_push_revision {
 		}
 		if (!$dumb_push) {
 			run_git(qq(notes --ref=${remotename}/mediawiki add -f -m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
-			run_git(qq(update-ref -m "Git-MediaWiki push" refs/mediawiki/${remotename}/master ${sha1_commit} ${sha1_child}));
 		}
 	}
 
-- 
1.8.4.12.g98a4f55.dirty

[PATCH 3/4] git-remote-mediawiki: use dont-update-private capability on dumb push

[Matthieu Moy]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

I prefer not to bother with compatibility of git-remote-mediawiki with
older Git versions. The recommanded way is to install Git and
git-remote-mediawiki from the same source tree. People who want to
keep an old Git version can still use old versions of
git-remote-mediawiki.

 contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index f8d7d2c..cdc278c 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -590,6 +590,9 @@ sub mw_capabilities {
 	print {*STDOUT} "import\n";
 	print {*STDOUT} "list\n";
 	print {*STDOUT} "push\n";
+	if ($dumb_push) {
+		print {*STDOUT} "dont-update-private\n";
+	}
 	print {*STDOUT} "\n";
 	return;
 }
-- 
1.8.4.12.g98a4f55.dirty

Re: [PATCH 3/4] git-remote-mediawiki: use dont-update-private capability on dumb push

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>
> I prefer not to bother with compatibility of git-remote-mediawiki with
> older Git versions. The recommanded way is to install Git and
> git-remote-mediawiki from the same source tree. People who want to
> keep an old Git version can still use old versions of
> git-remote-mediawiki.

I think that is a sane assumption.

>
>  contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
> index f8d7d2c..cdc278c 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -590,6 +590,9 @@ sub mw_capabilities {
>  	print {*STDOUT} "import\n";
>  	print {*STDOUT} "list\n";
>  	print {*STDOUT} "push\n";
> +	if ($dumb_push) {
> +		print {*STDOUT} "dont-update-private\n";
> +	}
>  	print {*STDOUT} "\n";
>  	return;
>  }

[PATCH 4/4] git-remote-mediawiki: no need to update private ref in non-dumb push

[Matthieu Moy]

We used to update the private ref ourselves, but this update is now done
by default (since 664059fb62).

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 contrib/mw-to-git/git-remote-mediawiki.perl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index cdc278c..0b920cd 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1214,7 +1214,6 @@ sub mw_push_revision {
 		}
 		if (!$dumb_push) {
 			run_git(qq(notes --ref=${remotename}/mediawiki add -f -m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
-			run_git(qq(update-ref -m "Git-MediaWiki push" refs/mediawiki/${remotename}/master ${sha1_commit} ${sha1_child}));
 		}
 	}
 
-- 
1.8.4.12.g98a4f55.dirty

Re: [PATCH 4/4] git-remote-mediawiki: no need to update private ref in non-dumb push

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> We used to update the private ref ourselves, but this update is now done
> by default (since 664059fb62).
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

Thanks; will queue all four.

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Not updating was the default until 664059fb62 (i.e. until 1.8.4), so the
> default already changed once. I tend to agree with Felipe that doing the
> update by default makes sense.

OK.  Thanks.

> git-remote-mediawiki is kind of a special case, as the remote side uses
> a very different data model, hence it can make sense to export+reimport
> to get locally what the remote side received.

Hmm, I am not sure how export+reimport would not make sense for
others like cvs and p4.

> But when interacting with something closer to Git (bzr, hg, ...),
> the mapping should be close to 1-1 and re-importing wouldn't make
> sense.

I agree if your argument is that re-importing should result in an
identitical result hence is unnecessary.

Re: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> git-remote-mediawiki is kind of a special case, as the remote side uses
>> a very different data model, hence it can make sense to export+reimport
>> to get locally what the remote side received.
>
> Hmm, I am not sure how export+reimport would not make sense for
> others like cvs and p4.

Actually, it is essentially what "git svn rebase" does. I guess it would
make even more sense for CVS (I don't know p4, nor git-p4).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/