threads / patch / 28106

v2git svn : hook before 'git svn dcommit'

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

## tl;dr

9 messages between Aug 15, 2011 and Sep 1, 2011. Diffs are folded; open one to read it.

replies: 8people: 5as markdown or json

Frédéric Heitzmann· Aug 15, 2011, 20:04 UTC · lore

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(-)
Show changes to 2 files +34 −1

Documentation/git-svn.txt, git-svn.perl

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
Junio C Hamano· Aug 15, 2011, 21:14 UTC · re: Frédéric Heitzmann · lore

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

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?
Show 13 quoted lines
> 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?

Show 12 quoted lines
> +	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?
Show 12 quoted lines
>  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 {
Eric Wong· Aug 17, 2011, 00:30 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> 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.
Show 14 quoted lines
> > +	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
Frédéric Heitzmann· Aug 17, 2011, 14:35 UTC · lore

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

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>
Show 53 quoted lines
>
> 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
Eric Wong· Aug 17, 2011, 20:37 UTC · re: Frédéric Heitzmann · lore

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

Frédéric Heitzmann <frederic.heitzmann@gmail.com> wrote:
Show 8 quoted lines
> 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.

Show 11 quoted lines
> 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
Frédéric Heitzmann· Aug 18, 2011, 13:43 UTC · re: Eric Wong · lore

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

2011/8/17 Eric Wong <normalperson@yhbt.net>:
Show 12 quoted lines
>> 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

Eric Wong· Aug 20, 2011, 18:41 UTC · re: Frédéric Heitzmann · lore

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

Frédéric Heitzmann <frederic.heitzmann@gmail.com> wrote:
Show 15 quoted lines
> 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
Peter Baumann· Aug 18, 2011, 09:12 UTC · re: Frédéric Heitzmann · lore

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

On Wed, Aug 17, 2011 at 04:35:03PM +0200, Frédéric Heitzmann wrote:
Show 54 quoted lines
> 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
Paul Young· Sep 1, 2011, 16:58 UTC · re: Peter Baumann · lore

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

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.

← back to recent threads