git svn : hook before 'git svn dcommit'

PATCH In review Frédéric Heitzmann 2 participants 11 messages Jun 15, 2016 Documentation

lore

Share & tools

lore

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #0 lore raw

PATCH 1/1 git svn : hook before 'git svn dcommit'

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(-)

git-svn.perl +19 -0

index 89f83fd..e70afae 100755
@@ -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) {

Matthieu Moy

2016-06-15 22:51 UTC #1 lore raw

Quoted 4 lines · 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 +++++++++++++++++++

What about documentation?

Quoted 3 lines

(quoted diff) +2 -0

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

$!\n";

whitespace damage (extra newline)

Signature · 2 lines

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

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #2 lore raw

Quoted 7 lines · Matthieu Moy

Quoted 4 lines · 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 +++++++++++++++++++
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]

Quoted 5 lines

Quoted 3 lines
(quoted diff) +2 -0
+       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

Matthieu Moy

2016-06-15 22:51 UTC #3 lore raw

Quoted 10 lines · Frédéric Heitzmann

Quoted 7 lines · Matthieu Moy
Quoted 4 lines · 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 +++++++++++++++++++
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.

Quoted 1 line

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

Quoted 10 lines

Quoted 5 lines
Quoted 3 lines
(quoted diff) +2 -0
+       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.

Signature · 2 lines

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

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #4 lore raw

PATCH 1/1 git svn : hook before 'git svn dcommit'

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(-)

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #5 lore raw

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

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 <[email protected]>

---
 git-svn.perl |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

git-svn.perl +21 -0

index 89f83fd..a537858 100755
@@ -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 {

Signature · 1 line

1.7.6.133.gd3b55a

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #6 lore raw

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

In reply to Frédéric Heitzmann

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

Signed-off-by Frédéric Heitzmann <[email protected]>

---
 Documentation/git-svn.txt |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

Documentation/git-svn.txt +13 -1

index 713e523..ec87ed3 100644
@@ -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
 ---

Signature · 1 line

1.7.6.133.gd3b55a

Matthieu Moy

2016-06-15 22:51 UTC #7 lore raw

In reply to Frédéric Heitzmann

Quoted 4 lines · 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.

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.

Signature · 2 lines

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

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #8 lore raw

Quoted 6 lines · Matthieu Moy

Quoted 4 lines · 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.

Quoted 6 lines

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.

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #9 lore raw

PATCH v2 1/1 git svn : hook before 'git svn dcommit'

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 <[email protected]>

---

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(-)

Documentation/git-svn.txt +13 -1

index 713e523..ec87ed3 100644
@@ -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
 ---

git-svn.perl +21 -0

index 89f83fd..a537858 100755
@@ -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 {

Signature · 1 line

1.7.6.133.gd3b55a

Frédéric Heitzmann

author

2016-06-15 22:51 UTC #10 lore raw

PATCH v2 1/1 git svn : hook before 'git svn dcommit'

Signed-off-by Frédéric Heitzmann <[email protected]>

---
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(-)

Documentation/git-svn.txt +13 -1

index 713e523..ec87ed3 100644
@@ -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
 ---

git-svn.perl +21 -0

index 89f83fd..a537858 100755
@@ -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 {

Signature · 1 line

1.7.6.133.gd3b55a