[PATCH] git svn : hook before 'git svn dcommit'

[PATCH] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

The 'pre-svn-dcommit' hook si called before 'git svn dcommit', which aborts
if return value is not zero.
---
  git-svn.perl |   19 +++++++++++++++++++
  1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 89f83fd..e70afae 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -396,6 +396,23 @@ sub init_subdir {
         $_repository = Git->repository(Repository => $ENV{GIT_DIR});
  }

+sub pre_svn_dcommit_hook {
+       my $hook = "$ENV{GIT_DIR}/hooks/pre-svn-dcommit";
+       return 0 if ! -e $hook || ! -x $hook;
+
+       system($hook);
+       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;
+       }
+}
+
  sub cmd_clone {
         my ($url, $path) = @_;
         if (!defined $path &&
@@ -518,6 +535,8 @@ sub cmd_dcommit {
                 command(['checkout', $head], STDERR => 0);
         }

+       return if pre_svn_dcommit_hook();
+
         my @refs;
         my ($url, $rev, $uuid, $gs) = working_head_info('HEAD', \@refs);
         unless ($gs) {
-- 

Re: [PATCH] git svn : hook before 'git svn dcommit'

[Matthieu Moy]

Frédéric Heitzmann <frederic.heitzmann@gmail.com> writes:

> The 'pre-svn-dcommit' hook si called before 'git svn dcommit', which aborts
> if return value is not zero.
> ---
>  git-svn.perl |   19 +++++++++++++++++++

What about documentation?

> +       if ($? == -1) {
> +               print "[pre_svn_dcommit_hook] failed to execute $hook:
> $!\n";

whitespace damage (extra newline)

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

Re: [PATCH] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

Le 02/07/2011 15:54, Matthieu Moy a écrit :
> Frédéric Heitzmann<frederic.heitzmann@gmail.com>  writes:
>
>> The 'pre-svn-dcommit' hook si called before 'git svn dcommit', which aborts
>> if return value is not zero.
>> ---
>>   git-svn.perl |   19 +++++++++++++++++++
> What about documentation?
Should documentation be part of githooks or git-svn man page ?
I vote for the latest, in order to avoid
In any case, what do you think of the following help message ?

+HOOKS
+-----
+
+The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be 
used to
+prevent some diff to be committed to a SVN repository. It may typically be
+used to filter some intermediate patches, which were committed into git but
+must not find their way to the SVN repository.
+
+If the hook exists with a non zero-status, 'git svn dcommit' will abort.
+
+

[...]

  SEE ALSO
  --------
-linkgit:git-rebase[1]
+linkgit:git-rebase[1], linkgit:githooks[5]

-- 

>> +       if ($? == -1) {
>> +               print "[pre_svn_dcommit_hook] failed to execute $hook:
>> $!\n";
> whitespace damage (extra newline)
>

As for the 'whitespace damage", I do not understand what you mean.
The \n look mandatory to me.
You may look at 'info perlfunc' for the original code snippet (see 
'system' function).

--
Fred

Re: [PATCH] git svn : hook before 'git svn dcommit'

[Matthieu Moy]

Frédéric Heitzmann <frederic.heitzmann@gmail.com> writes:

> Le 02/07/2011 15:54, Matthieu Moy a écrit :
>> Frédéric Heitzmann<frederic.heitzmann@gmail.com>  writes:
>>
>>> The 'pre-svn-dcommit' hook si called before 'git svn dcommit', which aborts
>>> if return value is not zero.
>>> ---
>>>   git-svn.perl |   19 +++++++++++++++++++
>> What about documentation?
> Should documentation be part of githooks or git-svn man page ?
> I vote for the latest, in order to avoid

I'd also put it in the git-svn page, since git-svn is somehow not really
part of Git.

> +prevent some diff to be committed to a SVN repository.

I'd say "an SVN", not "a SVN", but the documentation already use both
forms.

>>> +       if ($? == -1) {
>>> +               print "[pre_svn_dcommit_hook] failed to execute $hook:
>>> $!\n";
>> whitespace damage (extra newline)
>>
>
>As for the 'whitespace damage", I do not understand what you mean.
>The \n look mandatory to me.
>You may look at 'info perlfunc' for the original code snippet (see
>system' function).

I'm not talking about the \n, but the fact that what used to be a single
line of code is broken in two parts in your message.

A patch hunk has lines starting with " ", "+" or "-", yours have a line
starting with $. It won't apply with "patch" or "git apply", hence extra
work for our maintainer.

Try using "git send-email" to avoid that.

And actually, read Documentation/SubmittingPatches, in particular the
part about Signed-off-by.

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

[PATCH] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
prevent some diff to be committed to a SVN repository. It may typically be
used to filter some intermediate patches, which were committed into git but
must not find their way to the SVN repository.

It takes a single parameter, the reference given to 'git svn dcommit'. If the
hook exists with a non zero-status, 'git svn dcommit' will abort.

Documentation/git-svn.txt |   14 +++++++++++++-
git-svn.perl              |   21 +++++++++++++++++++++
2 files changed, 34 insertions(+), 1 deletions(-)

[PATCH 1/2] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

The 'pre-svn-dcommit' hook is called before 'git svn dcommit', which aborts
if return value is not zero.

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
 git-svn.perl |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

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;
+
+	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;
+	}
+}
+
 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 {
-- 
1.7.6.133.gd3b55a

[PATCH 2/2] git svn : documentation of 'pre-svn-dcommit'

[Frédéric Heitzmann]

Update of the git-svn documentation: 'pre-svn-dcommit' hook.

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
 Documentation/git-svn.txt |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 713e523..ec87ed3 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -700,6 +700,18 @@ section because they affect the 'git-svn-id:' metadata line, except
 for rewriteRoot and rewriteUUID which can be used together.
 
 
+HOOKS
+-----
+
+The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
+prevent some diff to be committed to a SVN repository. It may typically be
+used to filter some intermediate patches, which were committed into git but
+must not find their way to the SVN repository.
+
+It takes a single parameter, the reference given to 'git svn dcommit'. If the
+hook exists with a non zero-status, 'git svn dcommit' will abort.
+
+
 BASIC EXAMPLES
 --------------
 
@@ -901,7 +913,7 @@ reset) branches-maxRev and/or tags-maxRev as appropriate.
 
 SEE ALSO
 --------
-linkgit:git-rebase[1]
+linkgit:git-rebase[1], linkgit:githooks[5]
 
 GIT
 ---
-- 
1.7.6.133.gd3b55a

Re: [PATCH] git svn : hook before 'git svn dcommit'

[Matthieu Moy]

Frédéric Heitzmann <frederic.heitzmann@gmail.com> writes:

> The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
> prevent some diff to be committed to a SVN repository. It may typically be
> used to filter some intermediate patches, which were committed into git but
> must not find their way to the SVN repository.

Why 2 patches?

We usually try to have each commit as correct as possible (e.g. when
sending several patches, each commit should still pass the testsuite).
With your 2-patches serie, the first commit has documentation for a
feature which doesn't exist yet.

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

Re: [PATCH] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

Le 03/07/2011 23:00, Matthieu Moy a écrit :
> Frédéric Heitzmann<frederic.heitzmann@gmail.com>  writes:
>
>> The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
>> prevent some diff to be committed to a SVN repository. It may typically be
>> used to filter some intermediate patches, which were committed into git but
>> must not find their way to the SVN repository.
> Why 2 patches?
>
> We usually try to have each commit as correct as possible (e.g. when
> sending several patches, each commit should still pass the testsuite).
> With your 2-patches serie, the first commit has documentation for a
> feature which doesn't exist yet.
I find it easier to separate commits on documentation from code patch, 
especially for rereading and dicussing.
However, if it is desirable to get them merged, I could do that easily.

As for the order :
patch 1/2 : perl magic
patch 2/2 : documentation update
=>  the serie looks in the right order to me.

[PATCH v2] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

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.

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---

This is the second iteration of the patch.
Previous 2 patches were merged into one. 

 Documentation/git-svn.txt |   14 +++++++++++++-
 git-svn.perl              |   21 +++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 713e523..ec87ed3 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -700,6 +700,18 @@ section because they affect the 'git-svn-id:' metadata line, except
 for rewriteRoot and rewriteUUID which can be used together.
 
 
+HOOKS
+-----
+
+The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
+prevent some diff to be committed to a SVN repository. It may typically be
+used to filter some intermediate patches, which were committed into git but
+must not find their way to the SVN repository.
+
+It takes a single parameter, the reference given to 'git svn dcommit'. If the
+hook exists with a non zero-status, 'git svn dcommit' will abort.
+
+
 BASIC EXAMPLES
 --------------
 
@@ -901,7 +913,7 @@ reset) branches-maxRev and/or tags-maxRev as appropriate.
 
 SEE ALSO
 --------
-linkgit:git-rebase[1]
+linkgit:git-rebase[1], linkgit:githooks[5]
 
 GIT
 ---
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;
+
+	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;
+	}
+}
+
 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 {
-- 
1.7.6.133.gd3b55a

[PATCH v2] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

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.

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
There was no remark about patch v2.
I suppose it should be OK for merging upstream.

 Documentation/git-svn.txt |   14 +++++++++++++-
 git-svn.perl              |   21 +++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 713e523..ec87ed3 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -700,6 +700,18 @@ section because they affect the 'git-svn-id:' metadata line, except
 for rewriteRoot and rewriteUUID which can be used together.
 
 
+HOOKS
+-----
+
+The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
+prevent some diff to be committed to a SVN repository. It may typically be
+used to filter some intermediate patches, which were committed into git but
+must not find their way to the SVN repository.
+
+It takes a single parameter, the reference given to 'git svn dcommit'. If the
+hook exists with a non zero-status, 'git svn dcommit' will abort.
+
+
 BASIC EXAMPLES
 --------------
 
@@ -901,7 +913,7 @@ reset) branches-maxRev and/or tags-maxRev as appropriate.
 
 SEE ALSO
 --------
-linkgit:git-rebase[1]
+linkgit:git-rebase[1], linkgit:githooks[5]
 
 GIT
 ---
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;
+
+	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;
+	}
+}
+
 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 {
-- 
1.7.6.133.gd3b55a

[PATCH v2] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

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.

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
I resend the same patch previously sent July 9th.
Apparently it did not graduated upstream, and I do not know why.
Please someone tell me if something needs to be improved.

 Documentation/git-svn.txt |   14 +++++++++++++-
 git-svn.perl              |   21 +++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 713e523..ec87ed3 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -700,6 +700,18 @@ section because they affect the 'git-svn-id:' metadata line, except
 for rewriteRoot and rewriteUUID which can be used together.
 
 
+HOOKS
+-----
+
+The 'pre-svn-dcommit' hook is called by 'git svn dcommit' and can be used to
+prevent some diff to be committed to a SVN repository. It may typically be
+used to filter some intermediate patches, which were committed into git but
+must not find their way to the SVN repository.
+
+It takes a single parameter, the reference given to 'git svn dcommit'. If the
+hook exists with a non zero-status, 'git svn dcommit' will abort.
+
+
 BASIC EXAMPLES
 --------------
 
@@ -901,7 +913,7 @@ reset) branches-maxRev and/or tags-maxRev as appropriate.
 
 SEE ALSO
 --------
-linkgit:git-rebase[1]
+linkgit:git-rebase[1], linkgit:githooks[5]
 
 GIT
 ---
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;
+
+	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;
+	}
+}
+
 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 {
-- 
1.7.6.133.gd3b55a

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Junio C Hamano]

Frédéric Heitzmann  <frederic.heitzmann@gmail.com> 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 {

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Frédéric Heitzmann  <frederic.heitzmann@gmail.com> 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?

I'm not sure.  I feel hooks should be avoided whenever possible, and
a git-svn-specific hook for dcommit wouldn't place the same restriction
as a server-side SVN hook for svn(1) users.

Preventing certain commits from accidentally hitting the SVN server can
be useful, I think.  On the other hand, I'm not sure if people who run
accidental dcommits would remember to the pre-dcommit hook, either.

Perhaps an interactive option for dcommit would be just as useful?

Test cases are required for any new features of git-svn, though.

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

Failure messages should definitely go to stderr.

-- 
Eric Wong

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

Hi all.

Maybe I should give some more context to explain why a hook could be a
potential improvement.

Let's consider the following workflow :
1) git svn clone from the SVN server, then git checkout -b topic
2) git commit some "reference data", before starting some optimization
or code refactoring.
** These reference data are not supposed to find their way to the SVN server **
Committing such "reference data" is just a convenience because git
does a great job to show how these data may or may not change during
the development process.
3) hack, test, commit ...
3 bis) it may happen that reference data change for some very good
reason (for instance some protocol change)
New reference data are then commited.

back to 3 ...

4) Before merging back to master and commitng to SVN, it is necessary
to remove commits with reference data (git rebase -i --onto master
master topic ...)
5) merge topic branch with master and git svn dcommit

-- end --

It is very easy to forget step 4, and svn commit lots of useless data.

Proposal 1)
* commit reference data with some specific mark in the commit message
(e.g. "NO_SVN")
* use pre-svn-dcommit hook to detect such commits

Proposal 2) (not fully feasable for what I know)
* git svn clone to a bare repo
* clone a working repo from the the bare repo.
* steps 2, 3, maybe 3bis, ... then 4
* push commits to the bare repo, while using pre-receive or update
hook to look for wrong commits, and abort if so.
* use post-receive hook to trigger git svn dcommit

Main drawback for proposal 2 (appart from needing 2 repo instead of
one) is that each time you want to update your working  repo, you have
to git svn rebase the bare repo, then git pull.

Proposal 2bis)
* add a pre-send hook on the bare repo, and trigger some git svn
rebase with this hook.
I am not sure to see all the potential consequences of such a hook though.

All things begin equal, proposal 1 seems to be the easier path, but it
is highly debatable.

--
Fred

ps : I had to resend this email because first attempt included HTML
(sic). Very sorry if you receive it twice.

2011/8/17 Eric Wong <normalperson@yhbt.net>
>
> Junio C Hamano <gitster@pobox.com> wrote:
> > Frédéric Heitzmann  <frederic.heitzmann@gmail.com> 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?
>
> I'm not sure.  I feel hooks should be avoided whenever possible, and
> a git-svn-specific hook for dcommit wouldn't place the same restriction
> as a server-side SVN hook for svn(1) users.
>
> Preventing certain commits from accidentally hitting the SVN server can
> be useful, I think.  On the other hand, I'm not sure if people who run
> accidental dcommits would remember to the pre-dcommit hook, either.
>
> Perhaps an interactive option for dcommit would be just as useful?
>
> Test cases are required for any new features of git-svn, though.
>
> > > +   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?
>
> Failure messages should definitely go to stderr.
>
> --
> Eric Wong

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Eric Wong]

Frédéric Heitzmann <frederic.heitzmann@gmail.com> wrote:
> 4) Before merging back to master and commitng to SVN, it is necessary
> to remove commits with reference data (git rebase -i --onto master
> master topic ...)
> 5) merge topic branch with master and git svn dcommit
> 
> -- end --
> 
> It is very easy to forget step 4, and svn commit lots of useless data.

I agree.

> Proposal 1)
> * commit reference data with some specific mark in the commit message
> (e.g. "NO_SVN")
> * use pre-svn-dcommit hook to detect such commits

The problem with this is hook standardization across committers and even
across different machines/directories a committer may use.

> Proposal 2) (not fully feasable for what I know)
> * git svn clone to a bare repo
> * clone a working repo from the the bare repo.
> * steps 2, 3, maybe 3bis, ... then 4
> * push commits to the bare repo, while using pre-receive or update
> hook to look for wrong commits, and abort if so.
> * use post-receive hook to trigger git svn dcommit
> 
> Main drawback for proposal 2 (appart from needing 2 repo instead of
> one) is that each time you want to update your working  repo, you have
> to git svn rebase the bare repo, then git pull.

Proposal 2 is way too complicated, I hate it.

> All things begin equal, proposal 1 seems to be the easier path, but it
> is highly debatable.

I had Proposal 3 in my original response:

> 2011/8/17 Eric Wong <normalperson@yhbt.net> wrote:
> > Perhaps an interactive option for dcommit would be just as useful?

1 and 3 can both implemented, but I think 3 would be easier to
use/setup/standardize.  I suspect it's also easier to train oneself to
always use "dcommit -i".  Perhaps even default to interactive mode
like git-send-email does nowadays.

Unfortunately interactive dcommit requires more effort to implement.

-- 
Eric Wong

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Peter Baumann]

On Wed, Aug 17, 2011 at 04:35:03PM +0200, Frédéric Heitzmann wrote:
> Hi all.
> 
> Maybe I should give some more context to explain why a hook could be a
> potential improvement.
> 
> Let's consider the following workflow :
> 1) git svn clone from the SVN server, then git checkout -b topic
> 2) git commit some "reference data", before starting some optimization
> or code refactoring.
> ** These reference data are not supposed to find their way to the SVN server **
> Committing such "reference data" is just a convenience because git
> does a great job to show how these data may or may not change during
> the development process.
> 3) hack, test, commit ...
> 3 bis) it may happen that reference data change for some very good
> reason (for instance some protocol change)
> New reference data are then commited.
> 
> back to 3 ...
> 
> 4) Before merging back to master and commitng to SVN, it is necessary
> to remove commits with reference data (git rebase -i --onto master
> master topic ...)
> 5) merge topic branch with master and git svn dcommit
> 
> -- end --
> 
> It is very easy to forget step 4, and svn commit lots of useless data.
> 
> Proposal 1)
> * commit reference data with some specific mark in the commit message
> (e.g. "NO_SVN")
> * use pre-svn-dcommit hook to detect such commits
> 
> Proposal 2) (not fully feasable for what I know)
> * git svn clone to a bare repo
> * clone a working repo from the the bare repo.
> * steps 2, 3, maybe 3bis, ... then 4
> * push commits to the bare repo, while using pre-receive or update
> hook to look for wrong commits, and abort if so.
> * use post-receive hook to trigger git svn dcommit
> 
> Main drawback for proposal 2 (appart from needing 2 repo instead of
> one) is that each time you want to update your working  repo, you have
> to git svn rebase the bare repo, then git pull.
> 
> Proposal 2bis)
> * add a pre-send hook on the bare repo, and trigger some git svn
> rebase with this hook.
> I am not sure to see all the potential consequences of such a hook though.
> 
> All things begin equal, proposal 1 seems to be the easier path, but it
> is highly debatable.
> 

I have written a local script for exactly the problem you described after looking
for a git svn dcommit hook I could use (as you did).
I attached it, so feel free to use it. Simply add it to your bin and run it
with   git dcommit    instead of  git svn dcommit

Pls read the comment for further explanation how this script is used.


#!/bin/bash
# Copyright © Peter Baumann, 2011
#
# Wrapper script around git svn dcommit, which adds some useful functionality
#
# This script will prevent accidentally commiting some commits not yet ready
# into SVN. Commits starting with (case insensitive) debug, wip, fixup are
# considered not appropriate for putting them into SVN. The main reason for
# this functionality is the specific workflow I use. I always have some 
# internal debug commits (e.g. enhanced debug logging) or simply work in progress
# commits which should never be put into SVN. 
#
# To avoid putting those into SVN, I rebase all commits so that my WIP/DEBUG commits 
# are on top of the commits ment for SVN.
# Calling this script via "git dcommit" after the rebase makes sure only commits
# beneath the WIP commits are considered for SVN. 
# Furthermore, a shortlog of commits ment for SVN is shown and the user has
# to confirm before actually putting them into SVN.
#
# If this script is called via a specific commit (e.g. via its SHA1) as parameter,
# then only commits beneath and including the commit itself are committet to SVN.


SUBDIRECTORY_OK=Yes

. git-sh-setup
require_work_tree
cd_to_toplevel || die "foo"

# Upstream ref
upstream=remotes/trunk

# Stop at this commit
last=
if [ ! -z $1 ]; then
	$(git rev-parse $1^{commit})
	echo $last
fi

# The latest git commit we want to commit SVN
commit=

# Remembers the original head
orig_head=
if branch=$(git symbolic-ref -q HEAD)
then
	orig_head=${branch#refs/heads/}
else
	orig_head='(detached head)'
fi


function run()
{
	#echo "DEBUG: $@"
	$@
}	

IFS='
'
for c in $(git log --reverse --pretty="%H %s" HEAD --not "${upstream}"); do
	# Split the log output into its fields
	sha1="${c:0:40}"
	msg="${c:41}"

	# Check if the commit subject matches (case insenstive) to one of the
	# following patterns. Leading whitespace is fine
	# 	debug
	#	wip
	#	fixup!
	if echo "$msg"|egrep -i -q '^\s*(debug|wip|fixup)'; then
		break
	fi
	commit=${sha1}

	if [ "x${commit}" = "x${last}" ]; then
		break
	fi
done


if [ "x${commit}" == "x" ]; then
	die "Nothing to commit - Perhaps you have only stuff not ready for SVN?"
fi

echo ">>>> Committing the folling GIT commits to SVN <<<<"
git --no-pager log --pretty=oneline ${commit} --not "${upstream}"
echo

# Show the latest commit we are going to submit to SVN
#git show ${commit}

echo
echo "Commiting to SVN (y/N)?"
read yesno || die "Aborting"

if [ "x${yesno}" == "xy" ] || [ "x$yesno" == "xY" ]; then
	run git checkout -q "${commit}" || die "Checkout failed"
	run git svn dcommit || "Aborting - git svn dcommit failed!"

	if [ "${orig_head}" != "(detached head)" ]; then
		echo "DO THIS:"
		run git checkout "${orig_head}" && run git rebase "${upstream}"
	else
		echo "You have started this script being on a detached HEAD."
		echo "Please rebase manually!"
	fi
fi

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Frédéric Heitzmann]

2011/8/17 Eric Wong <normalperson@yhbt.net>:
>> 2011/8/17 Eric Wong <normalperson@yhbt.net> wrote:
>> > Perhaps an interactive option for dcommit would be just as useful?
>
> 1 and 3 can both implemented, but I think 3 would be easier to
> use/setup/standardize.  I suspect it's also easier to train oneself to
> always use "dcommit -i".  Perhaps even default to interactive mode
> like git-send-email does nowadays.
>
> Unfortunately interactive dcommit requires more effort to implement.
>
> --
> Eric Wong

It seems that proposal 3 is somehow equivalent to
$ git rebase -i --onto remotes/trunk remotes/trunk
... check commits, maybe remove some of them, ...
$ git svn dcommit

note : is the SVN remote branch always named "remote/trunk" ? If not,
is there a way to guess its name ?

--
Fred

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Eric Wong]

Frédéric Heitzmann <frederic.heitzmann@gmail.com> wrote:
> 2011/8/17 Eric Wong <normalperson@yhbt.net>:
> >> 2011/8/17 Eric Wong <normalperson@yhbt.net> wrote:
> >> > Perhaps an interactive option for dcommit would be just as useful?
> >
> > 1 and 3 can both implemented, but I think 3 would be easier to
> > use/setup/standardize.  I suspect it's also easier to train oneself to
> > always use "dcommit -i".  Perhaps even default to interactive mode
> > like git-send-email does nowadays.
> >
> > Unfortunately interactive dcommit requires more effort to implement.
> 
> It seems that proposal 3 is somehow equivalent to
> $ git rebase -i --onto remotes/trunk remotes/trunk
> ... check commits, maybe remove some of them, ...
> $ git svn dcommit

No, it would just prompt before making every commit (showing the log
message), like git-send-email can before sending every message.

> note : is the SVN remote branch always named "remote/trunk" ? If not,
> is there a way to guess its name ?

git svn dcommit --dry-run will print the URL from which you can
infer the remote branch.  There might be an easier way, but I
can't remember... (I don't see SVN much nowadays, most projects
I've cared about migrated to git)

-- 
Eric Wong

Re: [PATCH v2] git svn : hook before 'git svn dcommit'

[Paul Young]

Hi there

I'm trying to set up a post git svn dcommit hook in order to automatically
deliver stories via Pivotal Tracker:

https://www.pivotaltracker.com/help/api?version=v3#subversion_post_commit_example

Do you think you could help?

Thanks.