Re: [PATCH v6] Add a remote helper to interact with mediawiki (fetch & push)

Sverre Rabbelier <srabbelier@gmail.com> writes:

I'm not sure I understood exactly how it was before, but the current protocol seems indeed at least reasonable:

* It's possible to specify a batch of imports, so the remote-helper has
  freedom to optimize the import of multiple refs.

* A batch of import is clearly delimited, both on stdin and stdout, so
  it is possible to alternate import batches and other commands.

I still have a few complaints, because even with a better doc, I still found the debugging a bit hard. To make it easy for remote-helpers authors, I think the transport-helper could have an explicit "done" command, so that the command stream look like

import foo import bar \n done

instead of

import foo import bar \n \n

and to let the remote-helper's code be like

while($cmd = <read command>) {
   if ($cmd eq "command1") {
       do something;
   } elsif ($cmd eq "command2") {
       something else;
   } elsif ($cmd eq "done") {
       exit properly;
   }
}

I'm not sure whether changing this now is worth the trouble though.

I'd have appreciated if the transport-helper had given me an explicit error message when writting to a broken pipe too. I finally got it with gdb, but lost some time trying to understand (especially painfull since there was a race condition between the remote-helper termination and git writting to it, so the bug wasn't reproducible).

Here:

+ for my $refspec (@refsspecs) { + unless ($refspec =~ m/^(\+?)([^:]*):([^:]*)$/) { + die("Invalid refspec for push. Expected <src>:<dst> or +<src>:<dst>"); + } + my ($force, $local, $remote) = ($1 eq "+", $2, $3);

At this point, $force is a boolean saying whether there were a +, and $local and $remote are as you can guess.

+ if ($force) { + print STDERR "Warning: forced push not allowed on a MediaWiki.\n"; + } + if ($local eq "") { + print STDERR "Cannot delete remote branch on a MediaWiki\n"; + print STDOUT "error $remote cannot delete\n";

print STDERR goes to the console (i.e. to the user), and print STDOUT goes to the Git's transport-helper.

+ next; + } + if ($remote ne "refs/heads/master") { + print STDERR "Only push to the branch 'master' is supported on a MediaWiki\n"; + print STDOUT "error $remote only master allowed\n"; + next; + }

CPB = ?

Actually, my case is very particular, since the only thing to do with branches is to make sure the user doesn't use them. In remote-testgit, there are actually branches.

And testgit use the undocumented "export" feature, which does not seem to support branch deletion:

git push origin :branch2
fatal: remote-helpers do not support ref deletion
moy@bauges:/tmp/clone$ Traceback (most recent call last):
  File "/home/moy/local/usr-squeeze/libexec/git-core/git-remote-testgit", line 252, in <module>
    sys.exit(main(sys.argv))
  File "/home/moy/local/usr-squeeze/libexec/git-core/git-remote-testgit", line 249, in main
    more = read_one_line(repo)
  File "/home/moy/local/usr-squeeze/libexec/git-core/git-remote-testgit", line 215, in read_one_line
    sys.stdout.flush()
IOError: [Errno 32] Broken pipe

(This comes from

transport-helper.c:750:                 die("remote-helpers do not support ref deletion");

called before starting the exporter)