Frédéric Heitzmann <frederic.heitzmann@xxxxxxxxx> writes: > The 'pre-svn-dcommit' hook is called before 'git svn dcommit', which aborts > if return value is not zero. The only parameter given to the hook is the > reference given to 'git svn dcommit'. If no paramter was used, hook gets HEAD > as its only parameter. It appears that this is in the same spirit as the pre-commit hook used in "git commit", so it may not hurt but I do not know if having a separate hook is the optimal approach to achieve what it wants to do. I notice that git-svn users have been happily using the subsystem without need for any hook (not just pre-commit). Does "git svn" need an equivalent of pre-commit hook? If so, does it need equivalents to other hooks as well? I am not suggesting you to add support for a boatload of other hooks in this patch---I am trying to see if this is really a necessary change to begin with. Eric, do you want this one? > diff --git a/git-svn.perl b/git-svn.perl > index 89f83fd..a537858 100755 > --- a/git-svn.perl > +++ b/git-svn.perl > @@ -396,6 +396,25 @@ sub init_subdir { > $_repository = Git->repository(Repository => $ENV{GIT_DIR}); > } > > +sub pre_svn_dcommit_hook { > + my $head = shift; > + > + my $hook = "$ENV{GIT_DIR}/hooks/pre-svn-dcommit"; > + return 0 if ! -e $hook || ! -x $hook; Why force two stat(), instead of just "if ! -x $hook"? Doesn't it respond to a non-existing $hook with "there is nothing executable there" just fine? > + system($hook, $head); > + if ($? == -1) { > + print "[pre_svn_dcommit_hook] failed to execute $hook: $!\n"; > + return 1; > + } elsif ($? & 127) { > + printf "[pre_svn_dcommit_hook] child died with signal %d, %s coredump\n", > + ($? & 127), ($? & 128) ? 'with' : 'without'; > + return 1; > + } else { > + return $? >> 8; > + } > +} Should these messages go to the standard output? > sub cmd_clone { > my ($url, $path) = @_; > if (!defined $path && > @@ -505,6 +524,8 @@ sub cmd_dcommit { > . "or stash them with `git stash'.\n"; > $head ||= 'HEAD'; > > + return if pre_svn_dcommit_hook($head); > + > my $old_head; > if ($head ne 'HEAD') { > $old_head = eval { -- 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