git-svn problem: unexpected files/diffs in commit

git-svn problem: unexpected files/diffs in commit

[Seth Falcon]

Hi,

I just made an svn commit using 'git-svn commit' and ended up with a
quite unexpected result.  

I'm using svn, version 1.3.2 (r19776) _with_ the SVN:: lib and 
git is git version 1.4.2.g7099c (actually, I just updated and so I was
a few commits back, but git-svn.perl didn't change, so I'm pretty
confident that I'm current w.r.t. git-svn).

Here is a copy/paste of my session (some edits since some history was lost):

## Check for changes in svn repos and merge them in.

    ziti:~/proj/graph-git seth$ git-svn fetch
    ziti:~/proj/graph-git seth$ git pull . remotes/git-svn
    Trying really trivial in-index merge...
    Wonderful.
    In-index merge
     DESCRIPTION            |    2 +-
     R/AllGenerics.R        |    2 +-
     R/clustergraph.R       |    4 ++--
     man/distGraph-class.Rd |    6 ++++--
     4 files changed, 8 insertions(+), 6 deletions(-)

## I'm really where I think I am:

   ziti:~/proj/graph-git seth$ git branch
   * master

## Let's see what I _would_ commit if I did the normal git-svn commit
## thing

   ziti:~/proj/graph-git seth$ git diff --stat remotes/git-svn..master
    inst/unitTests/graphNEL_test.R |    2 +-
    inst/unitTests/runalltests.R   |    6 +++---
    2 files changed, 4 insertions(+), 4 deletions(-)

## Yeah, that looks right.

## ok, go for the commit

    ziti:~/proj/graph-git seth$ git-svn commit remotes/git-svn..master
    diff-tree f5ebf17f7e460d3bc3de72ab381c72dc76d26936 0681f7614c342b85b91d909ff02a9a966a44c3f4
            M       DESCRIPTION
            M       R/AllGenerics.R
            M       R/clustergraph.R
            M       inst/unitTests/graphNEL_test.R
            M       inst/unitTests/runalltests.R
            M       man/distGraph-class.Rd
    r19467 = 1b75d81a95da328f0b0d06b7562fdb48970b4c98
    RA layer request failed: OPTIONS request failed on '/bioconductor': OPTIONS of '/bioconductor': Could not read status line: SSL error: decryption failed or bad record mac (https://hedgehog.fhcrc.org) at /Users/seth/scm/bin/git-svn line 526
    65280 at /Users/seth/scm/bin/git-svn line 547
            main::commit_lib('0681f7614c342b85b91d909ff02a9a966a44c3f4', '0cccf3753b472b52a93154ed8021499055bb3923') called at /Users/seth/scm/bin/git-svn line 457
            main::commit('remotes/git-svn..master') called at /Users/seth/scm/bin/git-svn line 149


## GAAAHH! That isn't what I wanted at all.  It looks as if I didn't
## really do the pull.  What is going on?

Despite the SSL error, the commit to svn actually went through and I
had to back it out.  Did I do something wrong?  I did the git-svn
fetch and pull to sync up, then reapplied my patch and git-svn commit
"worked" although I got the same SSL error.

If I can provide more info, let me know.  

Thanks,

Re: git-svn problem: unexpected files/diffs in commit

[Eric Wong]

Seth Falcon <sethfalcon@gmail.com> wrote:
> Hi,
> 
> I just made an svn commit using 'git-svn commit' and ended up with a
> quite unexpected result.  
> 
> I'm using svn, version 1.3.2 (r19776) _with_ the SVN:: lib and 
> git is git version 1.4.2.g7099c (actually, I just updated and so I was
> a few commits back, but git-svn.perl didn't change, so I'm pretty
> confident that I'm current w.r.t. git-svn).

Cool that it works for you, I've yet to get SVN:: libs working with a
repository I didn't have full read access to.  I assume you have full
read access?

> Here is a copy/paste of my session (some edits since some history was lost):
> 
> ## Check for changes in svn repos and merge them in.
> 
>     ziti:~/proj/graph-git seth$ git-svn fetch
>     ziti:~/proj/graph-git seth$ git pull . remotes/git-svn
>     Trying really trivial in-index merge...
>     Wonderful.
>     In-index merge
>      DESCRIPTION            |    2 +-
>      R/AllGenerics.R        |    2 +-
>      R/clustergraph.R       |    4 ++--
>      man/distGraph-class.Rd |    6 ++++--
>      4 files changed, 8 insertions(+), 6 deletions(-)
> 
> ## I'm really where I think I am:
> 
>    ziti:~/proj/graph-git seth$ git branch
>    * master
> 
> ## Let's see what I _would_ commit if I did the normal git-svn commit
> ## thing
> 
>    ziti:~/proj/graph-git seth$ git diff --stat remotes/git-svn..master
>     inst/unitTests/graphNEL_test.R |    2 +-
>     inst/unitTests/runalltests.R   |    6 +++---
>     2 files changed, 4 insertions(+), 4 deletions(-)
> 
> ## Yeah, that looks right.

I usually check with git log remotes/git-svn..HEAD instead of git diff.
Perhaps adding --no-merges would be more correct?

> ## ok, go for the commit
> 
>     ziti:~/proj/graph-git seth$ git-svn commit remotes/git-svn..master
>     diff-tree f5ebf17f7e460d3bc3de72ab381c72dc76d26936 0681f7614c342b85b91d909ff02a9a966a44c3f4
>             M       DESCRIPTION
>             M       R/AllGenerics.R
>             M       R/clustergraph.R
>             M       inst/unitTests/graphNEL_test.R
>             M       inst/unitTests/runalltests.R
>             M       man/distGraph-class.Rd
>     r19467 = 1b75d81a95da328f0b0d06b7562fdb48970b4c98
>     RA layer request failed: OPTIONS request failed on '/bioconductor': OPTIONS of '/bioconductor': Could not read status line: SSL error: decryption failed or bad record mac (https://hedgehog.fhcrc.org) at /Users/seth/scm/bin/git-svn line 526
>     65280 at /Users/seth/scm/bin/git-svn line 547
>             main::commit_lib('0681f7614c342b85b91d909ff02a9a966a44c3f4', '0cccf3753b472b52a93154ed8021499055bb3923') called at /Users/seth/scm/bin/git-svn line 457
>             main::commit('remotes/git-svn..master') called at /Users/seth/scm/bin/git-svn line 149
> 
> 
> ## GAAAHH! That isn't what I wanted at all.  It looks as if I didn't
> ## really do the pull.  What is going on?
> 
> Despite the SSL error, the commit to svn actually went through and I
> had to back it out.  Did I do something wrong?  I did the git-svn
> fetch and pull to sync up, then reapplied my patch and git-svn commit
> "worked" although I got the same SSL error.
 
I haven't been able to reproduce the SSL error message consistently,
but I have seen it[1].  It could be SSL having state information that gets
screwed up with the forking git-svn does to avoid memory leaks in SVN::

Outside of the SSL problems, the mis-commit isn't strictly user-error,
but git-svn is confusing in this case, as the documentation is unclear
about what git-svn should do in this case :x

Simple answer: instead of pull, you should've used git rebase.  But I
don't think the documentation makes it clear at all.

I've been really slacking on the git-svn documentation the past few
months, help would be much appreciated.

Here's an in-depth explanation:

This is what git-svn does when issued "commit remotes/git-svn..master":
1. git-rev-list remotes/git-svn..master | tac =>
	0681f7614c342b85b91d909ff02a9a966a44c3f4
	0cccf3753b472b52a93154ed8021499055bb3923

0cccf3753b472b52a93154ed8021499055bb3923 is the result of your
'git pull . remotes/git-svn', correct?
And 0681f7614c342b85b91d909ff02a9a966a44c3f4 was made to git before
the pull.

So this is what git-svn does, it commits the output of:
diff-tree f5ebf17f7e460d3bc3de72ab381c72dc76d26936 0681f7614c342b85b91d909ff02a9a966a44c3f4
(f5eb... is remotes/git-svn at that point).

If the SVN/SSL connection had not died, it would've then proceeded to
commit the output of:

diff-tree 1b75d81a95da328f0b0d06b7562fdb48970b4c98 0cccf3753b472b52a93154ed8021499055bb3923
Where 1b75d81a95da328f0b0d06b7562fdb48970b4c98 is the output of your
previous commit (r19467)

Personally, I've been starting to favor 'git-svn commit-diff' myself
over 'git-svn commit', as it leaves cleaner history and makes git-svn
fetch results reproducable on different machines.

[1] - unfortunately, I seem to have forgotten about it since I use
commit-diff more often these days :x

-- 
Eric Wong

[PATCH] git-svn: establish new connections on commit after fork

[Eric Wong]

SVN seems to have a problem with https:// repositories from
time-to-time when doing multiple, sequential commits.  This
problem is not consistently reproducible without the patch,
but it should go away entirely with this patch...

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

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

diff --git a/git-svn.perl b/git-svn.perl
index 0d58bb9..b311c3d 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -500,6 +500,8 @@ sub commit_lib {
 	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
 	my $commit_msg = "$GIT_SVN_DIR/.svn-commit.tmp.$$";
 
+	my $repo;
+	($repo, $SVN_PATH) = repo_path_split($SVN_URL);
 	set_svn_commit_env();
 	foreach my $c (@revs) {
 		my $log_msg = get_commit_message($c, $commit_msg);
@@ -508,6 +510,8 @@ sub commit_lib {
 		# can't track down... (it's probably in the SVN code)
 		defined(my $pid = open my $fh, '-|') or croak $!;
 		if (!$pid) {
+			$SVN_LOG = libsvn_connect($repo);
+			$SVN = libsvn_connect($repo);
 			my $ed = SVN::Git::Editor->new(
 					{	r => $r_last,
 						ra => $SVN,
-- 
1.4.2.g7c9b

[PATCH] git-svn: recommend rebase for syncing against an SVN repo

[Eric Wong]

Does this make sense to other git-svn users out there?

pull can give funky history unless you understand how git-svn works
internally, which users should not be expected to do.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-svn.txt |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 7d86809..9fce4d3 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -212,12 +212,26 @@ # Commit only the git commits you want t
 	git-svn commit <tree-ish> [<tree-ish_2> ...]
 # Commit all the git commits from my-branch that don't exist in SVN:
 	git-svn commit remotes/git-svn..my-branch
-# Something is committed to SVN, pull the latest into your branch:
-	git-svn fetch && git pull . remotes/git-svn
+# Something is committed to SVN, rebase the latest into your branch:
+	git-svn fetch && git rebase remotes/git-svn
 # Append svn:ignore settings to the default git exclude file:
 	git-svn show-ignore >> .git/info/exclude
 ------------------------------------------------------------------------
 
+REBASE VS. PULL
+---------------
+
+Originally, git-svn recommended that the remotes/git-svn branch be
+pulled from.  This is because the author favored 'git-svn commit B'
+to commit a single head rather than the 'git-svn commit A..B' notation
+to commit multiple commits.
+
+If you use 'git-svn commit A..B' to commit several diffs and you do not
+have the latest remotes/git-svn merged into my-branch, you should use
+'git rebase' to update your work branch instead of 'git pull'.  'pull'
+can cause non-linear history to be flattened when committing into SVN,
+which can lead to merge commits reversing previous commits in SVN.
+
 DESIGN PHILOSOPHY
 -----------------
 Merge tracking in Subversion is lacking and doing branched development
@@ -310,6 +324,10 @@ the possible corner cases (git doesn't d
 copied files are fully supported if they're similar enough for git to
 detect them.
 
+SEE ALSO
+--------
+gitlink:git-rebase[1]
+
 Author
 ------
 Written by Eric Wong <normalperson@yhbt.net>.
-- 
1.4.2.g7c9b

Re: git-svn problem: unexpected files/diffs in commit

[Seth Falcon]

Eric Wong <normalperson@yhbt.net> writes:
> Cool that it works for you, I've yet to get SVN:: libs working with a
> repository I didn't have full read access to.  I assume you have full
> read access?

Yes, I have full rw access on that part of the svn repos.  The 
SVN:: libs are noticeably faster.

Aside:
   For others tracking this: the "SVN::" libs in question are the Perl
   bindings that come with Subversion and are optionally built when
   you build from source.  I wasted some google time searching CPAN
   for SVN:: so this might help someone. :-)

> Outside of the SSL problems, the mis-commit isn't strictly user-error,
> but git-svn is confusing in this case, as the documentation is unclear
> about what git-svn should do in this case :x

> I usually check with git log remotes/git-svn..HEAD instead of git
> diff.  Perhaps adding --no-merges would be more correct?

I'm not sure how to reproduce the situation I was in, but what would
git log have shown me that git diff didn't -- IOW, would it have been
obvious that the commit op was going to add extra stuff and
effectively undo a rev in svn?

> Simple answer: instead of pull, you should've used git rebase.  But I
> don't think the documentation makes it clear at all.

... reads git-rebase man page...
Ah, git-rebase does sound like what I want.

> I've been really slacking on the git-svn documentation the past few
> months, help would be much appreciated.

I will try to send some doc patches.  But I may have a few questions ;-)

> Here's an in-depth explanation:
>
> This is what git-svn does when issued "commit remotes/git-svn..master":
> 1. git-rev-list remotes/git-svn..master | tac =>
> 	0681f7614c342b85b91d909ff02a9a966a44c3f4
> 	0cccf3753b472b52a93154ed8021499055bb3923
>
> 0cccf3753b472b52a93154ed8021499055bb3923 is the result of your
> 'git pull . remotes/git-svn', correct?
> And 0681f7614c342b85b91d909ff02a9a966a44c3f4 was made to git before
> the pull.
>
> So this is what git-svn does, it commits the output of:
> diff-tree f5ebf17f7e460d3bc3de72ab381c72dc76d26936 0681f7614c342b85b91d909ff02a9a966a44c3f4
> (f5eb... is remotes/git-svn at that point).
>
> If the SVN/SSL connection had not died, it would've then proceeded to
> commit the output of:
>
> diff-tree 1b75d81a95da328f0b0d06b7562fdb48970b4c98 0cccf3753b472b52a93154ed8021499055bb3923
> Where 1b75d81a95da328f0b0d06b7562fdb48970b4c98 is the output of your
> previous commit (r19467)

I think I'm getting it.  Thank you very much for providing these details.

> Personally, I've been starting to favor 'git-svn commit-diff' myself
> over 'git-svn commit', as it leaves cleaner history and makes git-svn
> fetch results reproducable on different machines.
>
> [1] - unfortunately, I seem to have forgotten about it since I use
> commit-diff more often these days :x

I think commit-diff might be what I want to be using too, but I need
to contribute some documentation for it before I can read the man page
and start using it ;-)

An example call to git-svn commit-diff would be very helpful, I
suspect.

I will have a look...

While I'm thinking of it, it would be really nice for git-svn's commit and
commit-diff to have a dry-run type of flag.  Commits to your own git
repos are easy to correct, but those made on some other public scm are
less pretty.

Cheers,

+ seth

[PATCH] git-svn: add the 'dcommit' command

[Eric Wong]

This is a high-level wrapper around the 'commit-diff' command
and used to produce cleaner history against the mirrored repository
through rebase/reset usage.

It's basically a more polished version of this:

for i in `git rev-list --no-merges remotes/git-svn..HEAD | tac`; do
	git-svn commit-diff $i~1 $i
done
git reset --hard remotes/git-svn

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-svn.txt |   25 +++++++++++++++++++++++++
 git-svn.perl              |   35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 9fce4d3..38e73ba 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -53,6 +53,15 @@ fetch::
 	See 'Additional Fetch Arguments' if you are interested in
 	manually joining branches on commit.
 
+dcommit::
+	Commit all diffs from the current HEAD directly to the SVN
+	repository, and then rebase or reset (depending on whether or
+	not there is a diff between SVN and HEAD).  It is recommended
+	that you run git-svn fetch and rebase (not pull) your commits
+	against the latest changes in the SVN repository.
+	This is advantageous over 'commit' (below) because it produces
+	cleaner, more linear history.
+
 commit::
 	Commit specified commit or tree objects to SVN.  This relies on
 	your imported fetch data being up-to-date.  This makes
@@ -146,6 +155,22 @@ loginname = Joe User <user@example.com>
 
 	repo-config key: svn.authors-file
 
+-m::
+--merge::
+-s<strategy>::
+--strategy=<strategy>::
+	These are only used with the 'dcommit' command.
+
+	Passed directly to git-rebase when using 'dcommit' if a
+	'git-reset' cannot be used (see dcommit).
+
+-n::
+--dry-run::
+	This is only used with the 'dcommit' command.
+
+	Print out the series of git arguments that would show
+	which diffs would be committed to SVN.
+
 ADVANCED OPTIONS
 ----------------
 -b<refname>::
diff --git a/git-svn.perl b/git-svn.perl
index b311c3d..9382a15 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -51,7 +51,8 @@ my ($_revision,$_stdin,$_no_ignore_ext,$
 	$_message, $_file, $_follow_parent, $_no_metadata,
 	$_template, $_shared, $_no_default_regex, $_no_graft_copy,
 	$_limit, $_verbose, $_incremental, $_oneline, $_l_fmt, $_show_commit,
-	$_version, $_upgrade, $_authors, $_branch_all_refs, @_opt_m);
+	$_version, $_upgrade, $_authors, $_branch_all_refs, @_opt_m,
+	$_merge, $_strategy, $_dry_run);
 my (@_branch_from, %tree_map, %users, %rusers, %equiv);
 my ($_svn_co_url_revs, $_svn_pg_peg_revs);
 my @repo_path_split_cache;
@@ -118,6 +119,11 @@ my %cmd = (
 			{ 'message|m=s' => \$_message,
 			  'file|F=s' => \$_file,
 			%cmt_opts } ],
+	dcommit => [ \&dcommit, 'Commit several diffs to merge with upstream',
+			{ 'merge|m|M' => \$_merge,
+			  'strategy|s=s' => \$_strategy,
+			  'dry-run|n' => \$_dry_run,
+			%cmt_opts } ],
 );
 
 my $cmd;
@@ -561,6 +567,33 @@ sub commit_lib {
 	unlink $commit_msg;
 }
 
+sub dcommit {
+	my $gs = "refs/remotes/$GIT_SVN";
+	chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..HEAD"));
+	foreach my $d (reverse @refs) {
+		if ($_dry_run) {
+			print "diff-tree $d~1 $d\n";
+		} else {
+			commit_diff("$d~1", $d);
+		}
+	}
+	return if $_dry_run;
+	fetch();
+	my @diff = safe_qx(qw/git-diff-tree HEAD/, $gs);
+	my @finish;
+	if (@diff) {
+		@finish = qw/rebase/;
+		push @finish, qw/--merge/ if $_merge;
+		push @finish, "--strategy=$_strategy" if $_strategy;
+		print STDERR "W: HEAD and $gs differ, using @finish:\n", @diff;
+	} else {
+		print "No changes between current HEAD and $gs\n",
+		      "Hard resetting to the latest $gs\n";
+		@finish = qw/reset --hard/;
+	}
+	sys('git', @finish, $gs);
+}
+
 sub show_ignore {
 	$SVN_URL ||= file_to_s("$GIT_SVN_DIR/info/url");
 	$_use_lib ? show_ignore_lib() : show_ignore_cmd();
-- 
1.4.2.g7c9b

Re: git-svn problem: unexpected files/diffs in commit

[Eric Wong]

Seth Falcon <sethfalcon@gmail.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:

> > Outside of the SSL problems, the mis-commit isn't strictly user-error,
> > but git-svn is confusing in this case, as the documentation is unclear
> > about what git-svn should do in this case :x
> 
> > I usually check with git log remotes/git-svn..HEAD instead of git
> > diff.  Perhaps adding --no-merges would be more correct?

No, --no-merges is not correct (see below).

> I'm not sure how to reproduce the situation I was in, but what would
> git log have shown me that git diff didn't -- IOW, would it have been
> obvious that the commit op was going to add extra stuff and
> effectively undo a rev in svn?

> > I've been really slacking on the git-svn documentation the past few
> > months, help would be much appreciated.
> 
> I will try to send some doc patches.  But I may have a few questions ;-)

> I think commit-diff might be what I want to be using too, but I need
> to contribute some documentation for it before I can read the man page
> and start using it ;-)
> 
> An example call to git-svn commit-diff would be very helpful, I
> suspect.

git-svn commit-diff <a> <b>
(git diff-tree <a> <b> to check the result)

My new command, 'dcommit' wraps it.

'commit-diff' very low-level command that does exactly what you tell
it to do, compute the delta of two trees, and send it to the SVN repo.
The <a> tree doesn't even have to strictly match what's in the SVN repo
at the given moment if you're using the SVN:: libs.

'commit' was the original command, that meant: write this tree to SVN.
Period.  This was before SVN:: library support was available, so it had
to be stupidly simple.

foo..bar notation was supported, which in hindsight was a mistake
that has probably confused lots of people....

> I will have a look...
> 
> While I'm thinking of it, it would be really nice for git-svn's commit and
> commit-diff to have a dry-run type of flag.  Commits to your own git
> repos are easy to correct, but those made on some other public scm are
> less pretty.

I think everything is somewhat documented in the commit logs, but most
of those were back when git-svn was in contrib/

Otherwise, git-svn help attempts to be reasonably self-documenting, and
I've tried to keep the code to git-svn fairly readable (except the code
to generate the aforementioned help output :)

-- 
Eric Wong

[PATCH] git-svn: stop repeatedly reusing the first commit message with dcommit

[Eric Wong]

Excessive use of global variables got me into trouble.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 9382a15..0290850 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -819,6 +819,7 @@ sub commit_diff {
 	} else {
 		$ed->close_edit;
 	}
+	$_message = $_file = undef;
 }
 
 ########################### utility functions #########################
-- 
1.4.2.g7c9b