Sverre Rabbelier <srabbelier@xxxxxxxxx> writes: > Heya, > > 2011/8/31 Matthieu Moy <Matthieu.Moy@xxxxxxx>: >> So, after understanding better how import works, here's an updated >> patch that gets rid of the hacky workaround to terminate and send the >> "done" command at the right time. > > So what do you think of the way the protocol works now? Do you agree > that (modulo lacking docs) it is better than previously? 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). >> Actually, push had the same problem but it just went unnoticed (the >> remote has just one branch, so it's silly to try to push multiple >> branches at the same time ...). This version handles push more >> cleanly, giving accurate error message in cases like >> >> git push origin :master >> git push origin foo bar master >> >> or perhaps more commonly >> >> git push --all >> >> in a repository with branches other than master. > > My perl skills are minimal, but I'm curious how/where you implemented > this? 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; + } > Is this something that we can port to remote-testgit to document the > CPB on handling such things? 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) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html