[BUG] git-svn dcommit fails (connection closed unexpectedly)

[BUG] git-svn dcommit fails (connection closed unexpectedly)

[Matthieu Moy]

Hi,

I'm using git-svn, which usually works fine, but I occasionally get
this:

$ git-svn dcommit           
        A       file1
        A       file2
Network connection closed unexpectedly: Connection closed unexpectedly at /path/to/git-svn line 401
$

The failure seems to depend on the commit's I'm pushing, since this is
reproducible when running several times "dcommit" for the same commit,
but the same command also usually works fine on the same repositories
(same git repo, same svn target).

The svn repository is accessed with svn+ssh://user@machine/path/.

exporting the patch, applying it to an svn checkout, and "svn commit"
works fine.

Let me know if I can provide any other usefull information for
debugging.

Thanks,

Debian etch (stable)
Reproducible with both git 1.5.1.1 and the latest from git:

$ git-svn --version
git-svn version 1.5.2.rc3.3.ge347 (svn 1.4.2)

-- 
Matthieu

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Lars Hjemli]

On 5/11/07, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Hi,
>
> I'm using git-svn, which usually works fine, but I occasionally get
> this:
>
> $ git-svn dcommit
>         A       file1
>         A       file2
> Network connection closed unexpectedly: Connection closed unexpectedly at /path/to/git-svn line 401

Is this happening if you dcommit a file in a new directory?

The reason I'm asking is that I see the same problem in that
situation, but my svn repo is on a windows box, accessed over
svn://url, so I just assumed it was a problem on the remote end (the
service dies). But the problem always goes away if I commit the new
directory using the svn client before doing git-svn dcommit again....

--
larsh

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Lars Hjemli]

Hmm, your MUA fooled me to reply to the wrong people...

Can you please don't do this:

  Mail-Followup-To: git <git@vger.kernel.org>, Eric Wong <normalperson@yhbt.net>

--
larsh

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Matthieu Moy]

"Lars Hjemli" <lh@elementstorage.no> writes:

> On 5/11/07, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>> Hi,
>>
>> I'm using git-svn, which usually works fine, but I occasionally get
>> this:
>>
>> $ git-svn dcommit
>>         A       file1
>>         A       file2
>> Network connection closed unexpectedly: Connection closed unexpectedly at /path/to/git-svn line 401
>
> Is this happening if you dcommit a file in a new directory?

There seem to be a correlation, yes. Usually, simple commits will work
fine, and when I do something complex with the filesystem, it breaks.
I don't remember all the cases where it broke, but at least, last
time, it was about a new file in a new directory, yes.

-- 
Matthieu

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Matthieu Moy]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

"Lars Hjemli" <lh@elementstorage.no> writes:

> Is this happening if you dcommit a file in a new directory?

That's it indeed.

The attached script seems to reproducibly trigger the problem on my
box:

$ ../git-svn-test.sh
Checked out revision 0.
A         file
Adding         file
Transmitting file data .
Committed revision 1.
Initialized empty Git repository in .git/
moy@localhost's password: 
        A       file
r1 = 4357177f2d927931d31c8c49c0237561648001a7 (git-svn)

Checked out HEAD:
  svn+ssh://localhost//home/moy/tmp/test/test.26377/svnrepo r1
Created commit aa2db5a: newdir
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 newdir/newfile
moy@localhost's password: 
        A       newdir/newfile
Network connection closed unexpectedly: Connection closed unexpectedly at /home/moy/bin/local/verimag/git-svn line 405
$


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: git-svn-test.sh --]
[-- Type: text/x-sh, Size: 465 bytes --]

#! /bin/sh

mkdir test.$$
cd test.$$

svnadmin create svnrepo

svn co file://$PWD/svnrepo svnco

cd svnco
echo content > file
svn add file
svn commit -m "added file"
cd ../

# This triggers the bug.
git-svn clone svn+ssh://localhost/$PWD/svnrepo git-svn-co

# while this doesn't
# git-svn clone file://localhost/$PWD/svnrepo git-svn-co

cd git-svn-co

mkdir newdir
echo othercontent > newdir/newfile

git add newdir/newfile

git commit -m "newdir"

git-svn dcommit

[-- Attachment #3: Type: text/plain, Size: 15 bytes --]



-- 
Matthieu

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Eric Wong]

Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> "Lars Hjemli" <lh@elementstorage.no> writes:
> 
> > Is this happening if you dcommit a file in a new directory?
> 
> That's it indeed.
> 
> The attached script seems to reproducibly trigger the problem on my
> box:
> 
> $ ../git-svn-test.sh
> Checked out revision 0.
> A         file
> Adding         file
> Transmitting file data .
> Committed revision 1.
> Initialized empty Git repository in .git/
> moy@localhost's password: 
>         A       file
> r1 = 4357177f2d927931d31c8c49c0237561648001a7 (git-svn)
> 
> Checked out HEAD:
>   svn+ssh://localhost//home/moy/tmp/test/test.26377/svnrepo r1
> Created commit aa2db5a: newdir
>  1 files changed, 1 insertions(+), 0 deletions(-)
>  create mode 100644 newdir/newfile
> moy@localhost's password: 
>         A       newdir/newfile
> Network connection closed unexpectedly: Connection closed unexpectedly at /home/moy/bin/local/verimag/git-svn line 405
> $

Thanks to your script I can reproduce it.
svnserve (not git-svn) is segfaulting on me...

> #! /bin/sh
> 
> mkdir test.$$
> cd test.$$
> 
> svnadmin create svnrepo
> 
> svn co file://$PWD/svnrepo svnco
> 
> cd svnco
> echo content > file
> svn add file
> svn commit -m "added file"
> cd ../
> 
> # This triggers the bug.
> git-svn clone svn+ssh://localhost/$PWD/svnrepo git-svn-co
> 
> # while this doesn't
> # git-svn clone file://localhost/$PWD/svnrepo git-svn-co
> 
> cd git-svn-co
> 
> mkdir newdir
> echo othercontent > newdir/newfile
> 
> git add newdir/newfile
> 
> git commit -m "newdir"
> 
> git-svn dcommit

Thanks.  I didn't realize entirely new directories (and not empty
directories in svn but git thought were new) were what was causing
things to break.

-- 
Eric Wong

[PATCH] git-svn: avoid crashing svnserve when creating new directories

[Eric Wong]

When sorting directory names by depth (slash ("/") count) and
closing the deepest directories first (as the protocol
requires), we failed to put the root baton (with an empty string
as its key "") after top-level directories (which did not have
any slashes).

This resulted in svnserve being in a situation it couldn't
handle and caused a segmentation fault on the remote server.

This bug did not affect users of DAV and filesystem repositories.

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

diff --git a/git-svn.perl b/git-svn.perl
index e38811a..6d0cdac 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2841,8 +2841,10 @@ sub close_edit {
 	my ($self) = @_;
 	my ($p,$bat) = ($self->{pool}, $self->{bat});
 	foreach (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$bat) {
+		next if $_ eq '';
 		$self->close_directory($bat->{$_}, $p);
 	}
+	$self->close_directory($bat->{''}, $p);
 	$self->SUPER::close_edit($p);
 	$p->clear;
 }
-- 
Eric Wong

Re: [PATCH] git-svn: avoid crashing svnserve when creating new directories

[Matthieu Moy]

Eric Wong <normalperson@yhbt.net> writes:

>  	foreach (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$bat) {
> +		next if $_ eq '';
>  		$self->close_directory($bat->{$_}, $p);
>  	}
> +	$self->close_directory($bat->{''}, $p);

Works for me. Thanks a lot!

BTW, I didn't have time to write my script as a real testcase for
git-svn, but it should probably be added to the testsuite.

Thanks again,

-- 
Matthieu

Re: [PATCH] git-svn: avoid crashing svnserve when creating new directories

[Eric Wong]

Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> >  	foreach (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$bat) {
> > +		next if $_ eq '';
> >  		$self->close_directory($bat->{$_}, $p);
> >  	}
> > +	$self->close_directory($bat->{''}, $p);
> 
> Works for me. Thanks a lot!
> 
> BTW, I didn't have time to write my script as a real testcase for
> git-svn, but it should probably be added to the testsuite.

I have a modified version of the test I posted earlier based on your
test.  However, I'm not comfortable binding to any port (even without
ssh) on a users machine or potentially leaving a daemon running if the
test is interrupted.

>From bfbd67e31e952489098c2a10df259fbf1e7863fb Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 13 May 2007 17:38:21 -0700
Subject: [PATCH] git-svn: test for creating new directories over svn://

As reported by Matthieu Moy, this is causing svnserve to
terminate connections (because it segfaults) segfault.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/t9112-git-svn-dcommit-new-file.sh |   37 +++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100755 t/t9112-git-svn-dcommit-new-file.sh

diff --git a/t/t9112-git-svn-dcommit-new-file.sh b/t/t9112-git-svn-dcommit-new-file.sh
new file mode 100755
index 0000000..b186fce
--- /dev/null
+++ b/t/t9112-git-svn-dcommit-new-file.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+#
+
+test_description='git-svn dcommit new files over svn:// test'
+
+. ./lib-git-svn.sh
+
+# standard svnserve runs on 3690, so lets not conflict with that
+SVNSERVE_PORT=${SVNSERVE_PORT-'3691'}
+
+start_svnserve () {
+	svnserve --listen-port $SVNSERVE_PORT \
+	         --root $rawsvnrepo \
+	         --listen-once \
+	         --listen-host 127.0.0.1 &
+}
+
+test_expect_success 'start tracking an empty repo' "
+	svn mkdir -m 'empty dir' $svnrepo/empty-dir &&
+	echo anon-access = write >> $rawsvnrepo/conf/svnserve.conf &&
+	start_svnserve &&
+	git svn init svn://127.0.0.1:$SVNSERVE_PORT &&
+	git svn fetch
+	"
+
+test_expect_success 'create files in new directory with dcommit' "
+	mkdir git-new-dir &&
+	echo hello > git-new-dir/world &&
+	git update-index --add git-new-dir/world &&
+	git commit -m hello &&
+	start_svnserve &&
+	git svn dcommit
+	"
+
+test_done
-- 
Eric Wong

Re: [PATCH] git-svn: avoid crashing svnserve when creating new directories

[Matthieu Moy]

Eric Wong <normalperson@yhbt.net> writes:

> I have a modified version of the test I posted earlier based on your
> test.  However, I'm not comfortable binding to any port (even without
> ssh) on a users machine or potentially leaving a daemon running if the
> test is interrupted.

How about keeping the pid somewhere, and using "trap" to kill the svn
server process on interruption?

-- 
Matthieu

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Eric Wong]

Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Hi,
> 
> I'm using git-svn, which usually works fine, but I occasionally get
> this:
> 
> $ git-svn dcommit           
>         A       file1
>         A       file2
> Network connection closed unexpectedly: Connection closed unexpectedly at /path/to/git-svn line 401
> $
> 
> The failure seems to depend on the commit's I'm pushing, since this is
> reproducible when running several times "dcommit" for the same commit,
> but the same command also usually works fine on the same repositories
> (same git repo, same svn target).
> 
> The svn repository is accessed with svn+ssh://user@machine/path/.

Does the patch in
  http://permalink.gmane.org/gmane.comp.version-control.git/47126
help?

If so, I'm not sure why it should be failing sometimes and not
all the time.  If your local username the same as the username
you're using for svn it shouldn't fail for the reason that patch
above is needed.

Using the native svn or svn+ssh protocol is definitely flakier in my
experience, and there are still known issues with connections closing
during *fetch*.  dcommit is actually a lot more straightforward
as it only connects once, whereas fetch can reconnect to different
parts of the repo for --follow-parent.

> exporting the patch, applying it to an svn checkout, and "svn commit"
> works fine.
> 
> Let me know if I can provide any other usefull information for
> debugging.
> 
> Thanks,
> 
> Debian etch (stable)
> Reproducible with both git 1.5.1.1 and the latest from git:
> 
> $ git-svn --version
> git-svn version 1.5.2.rc3.3.ge347 (svn 1.4.2)

Which version of SVN is running on the server?

-- 
Eric Wong

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Martin Eisenhardt]

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

Hi Eric,
hi Matthieu,
hi list,

On Sunday 13 May 2007 19:17:07 Eric Wong wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> > I'm using git-svn, which usually works fine, but I occasionally get
> > this:
> >
> > $ git-svn dcommit
> >         A       file1
> >         A       file2
> > Network connection closed unexpectedly: Connection closed unexpectedly at
> > /path/to/git-svn line 401 $

I ran into that one, too ... :-(

> > The failure seems to depend on the commit's I'm pushing, since this is
> > reproducible when running several times "dcommit" for the same commit,
> > but the same command also usually works fine on the same repositories
> > (same git repo, same svn target).

I assume that the dcommit runs fine for *other* commits but not for that one 
that produced the error cited above.

In my case, it was simply a problem on my side: I had tried to add a file, 
that already existed in the repository - and subversion rejected the commit. 
Of course, it would be nice if git-svn could simply tell the user about his 
error, i.e., simply copy the output of libsvn or whatever it is using to 
connect to the subversion repo.

Another case where I stumbled upon said error message was when I added an 
empty (sub)directory to a project in subversion and then used git to track 
that project. Since git cannot track an empty directory, it did not appear in 
my git working copy. When I mkdir'd the directory locally (in my git wc) and 
then tried to add files within that repository, I got exactly the same error 
as Matthieu.

>
> Does the patch in
>   http://permalink.gmane.org/gmane.comp.version-control.git/47126
> help?

Not in my case, although this patch resolved the other issue, thanks again 
btw!

Kind regards
Martin Eisenhardt

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Karl Hasselström]

On 2007-05-13 19:49:34 +0200, Martin Eisenhardt wrote:

> Another case where I stumbled upon said error message was when I
> added an empty (sub)directory to a project in subversion and then
> used git to track that project. Since git cannot track an empty
> directory, it did not appear in my git working copy. When I mkdir'd
> the directory locally (in my git wc) and then tried to add files
> within that repository, I got exactly the same error as Matthieu.

It might be a net win to let git-svn handle empty directories by
creating an empty .git-svn-empty-dir file in them, instead of
pretending they don't exist.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Eric Wong]

Karl Hasselström <kha@treskal.com> wrote:
> On 2007-05-13 19:49:34 +0200, Martin Eisenhardt wrote:
> 
> > Another case where I stumbled upon said error message was when I
> > added an empty (sub)directory to a project in subversion and then
> > used git to track that project. Since git cannot track an empty
> > directory, it did not appear in my git working copy. When I mkdir'd
> > the directory locally (in my git wc) and then tried to add files
> > within that repository, I got exactly the same error as Matthieu.

Interesting.  I don't think git-svn currently checks for that case.

> It might be a net win to let git-svn handle empty directories by
> creating an empty .git-svn-empty-dir file in them, instead of
> pretending they don't exist.

No.  This is *WAY* too ugly.

-- 
Eric Wong

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Karl Hasselström <kha@treskal.com> wrote:
> > On 2007-05-13 19:49:34 +0200, Martin Eisenhardt wrote:
> > 
> > > Another case where I stumbled upon said error message was when I
> > > added an empty (sub)directory to a project in subversion and then
> > > used git to track that project. Since git cannot track an empty
> > > directory, it did not appear in my git working copy. When I mkdir'd
> > > the directory locally (in my git wc) and then tried to add files
> > > within that repository, I got exactly the same error as Matthieu.
> 
> Interesting.  I don't think git-svn currently checks for that case.

Ok, it already *does* check for this case and works for me(TM).

From 8222388080c05157e2926faffcd853bae500c521 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 13 May 2007 17:38:21 -0700
Subject: [PATCH] git-svn: test for creating new files in empty previously directories

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/t9112-git-svn-dcommit-new-file.sh |   40 +++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100755 t/t9112-git-svn-dcommit-new-file.sh

diff --git a/t/t9112-git-svn-dcommit-new-file.sh b/t/t9112-git-svn-dcommit-new-file.sh
new file mode 100755
index 0000000..e643c13
--- /dev/null
+++ b/t/t9112-git-svn-dcommit-new-file.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+#
+
+test_description='git-svn dcommit new files over svn:// test'
+
+. ./lib-git-svn.sh
+
+# standard svnserve runs on 3690, so lets not conflict with that
+SVNSERVE_PORT=${SVNSERVE_PORT-'3691'}
+
+start_svnserve () {
+	svnserve --listen-port $SVNSERVE_PORT \
+	         --root $rawsvnrepo \
+	         --listen-once \
+	         --listen-host 127.0.0.1 &
+}
+
+test_expect_success 'start tracking an empty repo' "
+	svn mkdir -m 'empty dir' $svnrepo/empty-dir &&
+	echo anon-access = write >> $rawsvnrepo/conf/svnserve.conf &&
+	start_svnserve &&
+	git svn init svn://127.0.0.1:$SVNSERVE_PORT &&
+	git svn fetch
+	"
+
+test_expect_success 'create two files in empty-directory with dcommit' "
+	mkdir empty-dir &&
+	echo hello > empty-dir/a &&
+	echo hello > empty-dir/b &&
+	echo hello > empty-dir/c &&
+	git update-index --add empty-dir/a empty-dir/b empty-dir/c &&
+	git commit -m hello &&
+	start_svnserve &&
+	git svn dcommit
+	"
+
+test_done
+
-- 
Eric Wong

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Linus Torvalds]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1682 bytes --]



On Sun, 13 May 2007, Eric Wong wrote:
> Karl Hasselström <kha@treskal.com> wrote:
> 
> > It might be a net win to let git-svn handle empty directories by
> > creating an empty .git-svn-empty-dir file in them, instead of
> > pretending they don't exist.
> 
> No.  This is *WAY* too ugly.

I wouldn't personally mind if somebody taught git to just track empty 
directories too.

There is no fundamental git database reason not to allow them: it's in 
fact quite easy to create an empty tree object. The problems with empty 
directories are in the *index*, and they shouldn't be insurmountable.

I never wanted empty directories for the kernel, but there is nothing 
really fundamentally wrong with them either. They do have some problems, 
which is why I don't particularly like tracking them, but if *not* 
tracking them causes more problems for people who import from SVN, I could 
easily see the balance moving towards "let people do it if they want".

In fact, I think the subproject support might have made it easier to track 
directories as empty directory entries. Pretty much every place that got 
impacted by subprojects would be where empty directory entries would be 
handled too.

In fact, one of the questions when tracking empty directories is whether 
you should track non-empty directories too. IOW, should we _always_ put 
the directory entry into the index (even when it's not empty), or should 
we do it only when somebody explicitly asks for it. I suspect the "always" 
case would is the right answer (ie a read-tree would always add a 
directory entry to the index before it starts adding the actual entries 
in the read-tree)

Anybody want to try it?

			Linus

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Matthieu Moy]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> I wouldn't personally mind if somebody taught git to just track empty 
> directories too.

Just to add my 2 cents: I just had a case where I would have needed
empty directories, and it seems reasonable as a workflow:

As a group of teachers, one created a skeleton, with stg like

./course/
./practical-works/day1
./practical-works/day2
./exercices/day1
./exercices/day2
...

He did it with svn, filled in only a few directories, I "git-svn
clone"d it, and I got almost nothing!

These empty directories are not interesting by themselves, but as a
placeholder to add files, I think it's a good thing to have them.

-- 
Matthieu

Re: [BUG] git-svn dcommit fails (connection closed unexpectedly)

[Matthieu Moy]

Eric Wong <normalperson@yhbt.net> writes:

> Does the patch in
>   http://permalink.gmane.org/gmane.comp.version-control.git/47126
> help?

I don't have the failure right now, I'll tell you when I get it again.

> If your local username the same as the username
> you're using for svn it shouldn't fail for the reason that patch
> above is needed.

It is the same on both machines.

> Which version of SVN is running on the server?

$ svn --version
svn, version 1.1.4 (r13838)
   compiled Apr 12 2005, 16:01:59
$ cat /etc/redhat-release
Red Hat Enterprise Linux ES release 4 (Nahant Update 5)

-- 
Matthieu