Fwd: Error with git-svn pushing a rename

Fwd: Error with git-svn pushing a rename

[Benjamin Pabst]

Hi guys,

I'm using git as a subversion client, which works great so far. But
today I tried to push back a rename to the subversion server, which
resulted in the following error:

perl: subversion/libsvn_subr/dirent_uri.c:2489:
svn_fspath__skip_ancestor: Assertion
`svn_fspath__is_canonical(child_fspath)' failed.
error: git-svn died of signal 6

After searching the web I found a similar problem at stackoverflow:
http://stackoverflow.com/questions/17693255/git-svn-dcommit-fails-because-of-assertion-error-svn-fspath-is-canonicalchildj

So I tried to downgrade subversion (1.7.x) which didn't help. Then I
also tried to downgrade git (1.7.x) which resulted in a different
error:
'tempfile' can't be called as a method at
/usr/share/perl5/vendor_perl/Git.pm line 1042.

So I updated both packages to the current version:
$ git --version
git version 1.8.4.2

$ svn --version
svn, version 1.8.3 (r1516576)
   compiled Sep 13 2013, 00:34:15 on x86_64-unknown-linux-gnu

Copyright (C) 2013 The Apache Software Foundation.
This software consists of contributions made by many people;
see the NOTICE file for more information.
Subversion is open source software, see http://subversion.apache.org/

The following repository access (RA) modules are available:

* ra_svn : Module for accessing a repository using the svn network protocol.
  - with Cyrus SASL authentication
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme
* ra_serf : Module for accessing a repository via WebDAV protocol using serf.
  - using serf 1.3.1
  - handles 'http' scheme
  - handles 'https' scheme

But now I'm back at the first error. Just for completeness, the error
occurs on the following operation:
$ git svn dcommit
Committing to https://xxxxx ...
R xxxx/xxxx/SomeFile => xxxx/xxxx/SomeOtherFile
perl: subversion/libsvn_subr/dirent_uri.c:2489:
svn_fspath__skip_ancestor: Assertion
`svn_fspath__is_canonical(child_fspath)' failed.
error: git-svn died of signal 6

Any ideas on handling this error?

Sorry for the (wrongly sent) first mail (incomplete).

Would be happy to hear from you.

Greetings
Ben

Re: Fwd: Error with git-svn pushing a rename

[Andreas Stricker]

Hi

> But today I tried to push back a rename to the subversion server,
> which resulted in the following error:>
> perl: subversion/libsvn_subr/dirent_uri.c:2489:
> svn_fspath__skip_ancestor: Assertion
> `svn_fspath__is_canonical(child_fspath)' failed.
> error: git-svn died of signal 6

I also observed this issue with a rename. My workaround was to downgrade
subversion to 1.7.x. That worked, but I'm searching for a real solution.

> After searching the web I found a similar problem at stackoverflow:
>
http://stackoverflow.com/questions/17693255/git-svn-dcommit-fails-because-of-assertion-error-svn-fspath-is-canonicalchildj

I'll add this one to the list: https://trac.macports.org/ticket/39986

It looks like I'm not the only one experiencing this.

Regards, Andy

Re: Fwd: Error with git-svn pushing a rename

[Andreas Stricker]

Hi Benjamin

> thanks for your link. Can you give me the exact version you
> downgraded svn to?

svn, Version 1.7.10 (r1485443)

I tried to reproduce the problem with git version 1.8.4.2 and
Subversion version 1.8.4 (r1534716) with a fresh and pristine
subversion repo and a git-svn clone of it: I didn't manage to
reproduce the rename issue. Then I switched subversion back to
1.7.10, created both the repo and the git-svn clone, switched
againt to 1.8.4.2 and then got an error. Unfortunately I didn't
check if the subversion perlbindings were regenerated, so I'm
not exactly sure. I'll repeat the test again, as soon I've find
the time.

It looks like a fresh git svn clone may fix the problem.

Regards, Andy

Re: Fwd: Error with git-svn pushing a rename

[Jonathan Nieder]

Andreas Stricker wrote:

> svn, Version 1.7.10 (r1485443)
>
> I tried to reproduce the problem with git version 1.8.4.2 and
> Subversion version 1.8.4 (r1534716) with a fresh and pristine
> subversion repo and a git-svn clone of it: I didn't manage to
> reproduce the rename issue. Then I switched subversion back to
> 1.7.10, created both the repo and the git-svn clone, switched
> againt to 1.8.4.2 and then got an error. Unfortunately I didn't
> check if the subversion perlbindings were regenerated, so I'm
> not exactly sure.
[...]
> It looks like a fresh git svn clone may fix the problem.

Yuck.

Can you give an exact sequence of steps (including "Upgrade Subversion
at this step") to reproduce the problem?  That would help immensely
--- if at all possible, I would very much like to keep existing
git-svn repos working on upgrade.

Thanks for your work so far on this.

Sincerely,
Jonathan

Re: Fwd: Error with git-svn pushing a rename

[Andreas Stricker]

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

Hi Jonathan

> Can you give an exact sequence of steps (including "Upgrade Subversion
> at this step") to reproduce the problem?  That would help immensely
> --- if at all possible, I would very much like to keep existing
> git-svn repos working on upgrade.

Of course. I've attached a text file with the commands required to
reproduce this error.

>From my experiments it looks like after the subversion is upgraded
to 1.8 the problem only occurs if "git svn fetch" fetches new changes
from the subversion repository. Without changes in upstream subversion
repository I couldn't reproduce the error. And a rename is required too.

Hope this helps.

Regards, Andy

[-- Attachment #2: gitsvnplay.txt --]
[-- Type: text/plain, Size: 14049 bytes --]

andy@m:r $ git
-bash: /opt/local/bin/git: No such file or directory
andy@m:r $ svn
-bash: /opt/local/bin/svn: No such file or directory
andy@m:r $ sudo port install subversion--->  Computing dependencies for subversion
--->  Fetching archive for subversion
--->  Attempting to fetch subversion-1.7.10_1.darwin_11.x86_64.tbz2 from http://lil.fr.packages.macports.org/subversion
--->  Attempting to fetch subversion-1.7.10_1.darwin_11.x86_64.tbz2.rmd160 from http://lil.fr.packages.macports.org/subversion
--->  Installing subversion @1.7.10_1
--->  Activating subversion @1.7.10_1
--->  Cleaning subversion
--->  Updating database of binaries: 100.0%
--->  Scanning binaries for linking errors: 100.0%
--->  Found 15 broken file(s), matching files to ports
--->  Found 1 broken port(s), determining rebuild order
--->  Rebuilding in order
     subversion @1.7.10 
--->  Computing dependencies for subversion
--->  Cleaning subversion
--->  Scanning binaries for linking errors: 100.0%
--->  Found 15 broken file(s), matching files to ports
--->  Found 1 broken port(s), determining rebuild order
--->  Rebuilding in order
     subversion @1.7.10 
--->  Computing dependencies for subversion
--->  Fetching distfiles for subversion
--->  Verifying checksums for subversion
--->  Extracting subversion
--->  Applying patches to subversion
--->  Configuring subversion
--->  Building subversion
--->  Staging subversion into destroot
--->  Deactivating subversion @1.7.10_1
--->  Cleaning subversion
--->  Uninstalling subversion @1.7.10_1
--->  Cleaning subversion
--->  Computing dependencies for subversion
--->  Installing subversion @1.7.10_1
--->  Activating subversion @1.7.10_1
--->  Cleaning subversion
--->  Updating database of binaries: 100.0%
--->  Scanning binaries for linking errors: 100.0%
--->  No broken files found.
andy@m:r $ sudo port install git-core +bash_completion +credential_osxkeychain +doc +pcre +python27 +svn
Password:
--->  Computing dependencies for git-core
--->  Dependencies to be installed: p5.12-svn-simple subversion-perlbindings-5.12
--->  Fetching archive for subversion-perlbindings-5.12
--->  Attempting to fetch subversion-perlbindings-5.12-1.7.10_0.darwin_11.x86_64.tbz2 from http://lil.fr.packages.macports.org/subversion-perlbindings-5.12
--->  Attempting to fetch subversion-perlbindings-5.12-1.7.10_0.darwin_11.x86_64.tbz2.rmd160 from http://lil.fr.packages.macports.org/subversion-perlbindings-5.12
--->  Installing subversion-perlbindings-5.12 @1.7.10_0
--->  Activating subversion-perlbindings-5.12 @1.7.10_0
--->  Cleaning subversion-perlbindings-5.12
--->  Fetching archive for p5.12-svn-simple
--->  Attempting to fetch p5.12-svn-simple-0.280.0_4.darwin_11.noarch.tbz2 from http://lil.fr.packages.macports.org/p5.12-svn-simple
--->  Attempting to fetch p5.12-svn-simple-0.280.0_4.darwin_11.noarch.tbz2.rmd160 from http://lil.fr.packages.macports.org/p5.12-svn-simple
--->  Installing p5.12-svn-simple @0.280.0_4
--->  Activating p5.12-svn-simple @0.280.0_4
--->  Cleaning p5.12-svn-simple
--->  Fetching archive for git-core
--->  Attempting to fetch git-core-1.8.4.2_0+bash_completion+credential_osxkeychain+doc+pcre+python27+svn.darwin_11.x86_64.tbz2 from http://lil.fr.packages.macports.org/git-core
--->  Attempting to fetch git-core-1.8.4.2_0+bash_completion+credential_osxkeychain+doc+pcre+python27+svn.darwin_11.x86_64.tbz2 from http://mse.uk.packages.macports.org/sites/packages.macports.org/git-core
--->  Attempting to fetch git-core-1.8.4.2_0+bash_completion+credential_osxkeychain+doc+pcre+python27+svn.darwin_11.x86_64.tbz2 from http://packages.macports.org/git-core
--->  Fetching distfiles for git-core
--->  Verifying checksums for git-core
--->  Extracting git-core
--->  Applying patches to git-core
--->  Configuring git-core
--->  Building git-core
--->  Staging git-core into destroot
--->  Installing git-core @1.8.4.2_0+bash_completion+credential_osxkeychain+doc+pcre+python27+svn
--->  Activating git-core @1.8.4.2_0+bash_completion+credential_osxkeychain+doc+pcre+python27+svn
--->  Cleaning git-core
--->  Updating database of binaries: 100.0%
--->  Scanning binaries for linking errors: 100.0%
--->  No broken files found.
andy@m:r $ git version
git version 1.8.4.2
andy@m:r $ svn --version |head -n2svn, Version 1.7.10 (r1485443)
   übersetzt Nov 17 2013, 22:58:41
andy@m:r $ svnadmin create svnrepo
andy@m:r $ svn checkout file://$PWD/svnrepo svnwork
Ausgecheckt, Revision 0.
andy@m:r $ cd svnwork/
andy@m:svnwork $ echo 'original content' > content.txt
andy@m:svnwork $ git add content.txt 
fatal: Not a git repository (or any of the parent directories): .git
andy@m:svnwork $ svn add content.txt 
A         content.txt
andy@m:svnwork $ svn commit -m 'initial commit'
Hinzufügen     content.txt
Übertrage Daten .
Revision 1 übertragen.
andy@m:svnwork $ cd ..
andy@m:r $ git svn clone file://$PWD/svnrepo gitwork
Initialisierte leeres Git-Repository in /private/tmp/r/gitwork/.git/
	A	content.txt
r1 = e27a31ad2100f7080dcb2c63eaf8bc7b74130bd5 (refs/remotes/git-svn)
Checked out HEAD:
  file:///tmp/r/svnrepo r1
andy@m:r $ cd gitwork/
andy@m:gitwork (master) $ echo 'another line' >> content.txt 
andy@m:gitwork (master) $ echo 'new file' > newfile.txt
andy@m:gitwork (master) $ git add content.txt newfile.txt 
andy@m:gitwork (master) $ git commit -m 'changed content'
[master cd1a8ee] changed content
 2 files changed, 2 insertions(+)
 create mode 100644 newfile.txt
andy@m:gitwork (master) $ git svn dcommit
Committing to file:///tmp/r/svnrepo ...
	A	newfile.txt
	M	content.txt
Committed r2
	A	newfile.txt
	M	content.txt
r2 = 4bc308cdb2684887844e8dde973c9aa26b80e2ca (refs/remotes/git-svn)
No changes between cd1a8eeeba8cc2cd4f452eba922a09780658e096 and refs/remotes/git-svn
Resetting to the latest refs/remotes/git-svn
andy@m:gitwork (master) $ echo 'switch subversion 1.7 in local port repo off'
switch subversion 1.7 in local port repo off
andy@m:gitwork (master) $ sudo port -v upgrade subversion subversion-perlbindings-5.12
Password:
--->  Computing dependencies for subversion.
--->  Fetching archive for subversion
--->  subversion-1.8.4_1.darwin_11.x86_64.tbz2 doesn't seem to exist in /opt/local/var/macports/incoming/verified
--->  Attempting to fetch subversion-1.8.4_1.darwin_11.x86_64.tbz2 from http://lil.fr.packages.macports.org/subversion
--->  Attempting to fetch subversion-1.8.4_1.darwin_11.x86_64.tbz2.rmd160 from http://lil.fr.packages.macports.org/subversion
--->  Installing subversion @1.8.4_1
--->  Cleaning subversion
--->  Removing work directory for subversion
--->  Computing dependencies for subversion.
--->  Deactivating subversion @1.7.10_1
--->  Cleaning subversion
--->  Removing work directory for subversion
--->  Activating subversion @1.8.4_1
--->  Cleaning subversion
--->  Removing work directory for subversion
--->  Computing dependencies for subversion-perlbindings-5.12.
--->  Fetching archive for subversion-perlbindings-5.12
--->  subversion-perlbindings-5.12-1.8.4_0.darwin_11.x86_64.tbz2 doesn't seem to exist in /opt/local/var/macports/incoming/verified
--->  Attempting to fetch subversion-perlbindings-5.12-1.8.4_0.darwin_11.x86_64.tbz2 from http://lil.fr.packages.macports.org/subversion-perlbindings-5.12
--->  Attempting to fetch subversion-perlbindings-5.12-1.8.4_0.darwin_11.x86_64.tbz2.rmd160 from http://lil.fr.packages.macports.org/subversion-perlbindings-5.12
--->  Installing subversion-perlbindings-5.12 @1.8.4_0
--->  Cleaning subversion-perlbindings-5.12
--->  Removing work directory for subversion-perlbindings-5.12
--->  Computing dependencies for subversion-perlbindings-5.12.
--->  Deactivating subversion-perlbindings-5.12 @1.7.10_0
--->  Cleaning subversion-perlbindings-5.12
--->  Removing work directory for subversion-perlbindings-5.12
--->  Activating subversion-perlbindings-5.12 @1.8.4_0
--->  Cleaning subversion-perlbindings-5.12
--->  Removing work directory for subversion-perlbindings-5.12
--->  Updating database of binaries: 100.0%
--->  Scanning binaries for linking errors: 100.0%
--->  No broken files found.
andy@m:gitwork (master) $ git version
git version 1.8.4.2
andy@m:gitwork (master) $ svn --version |head -n2
svn, Version 1.8.4 (r1534716)
   übersetzt am Nov  1 2013, um 14:41:52 auf x86_64-apple-darwin11.4.2
andy@m:gitwork (master) $ cd ../svnwork/
andy@m:svnwork $ svn up
svn: E155036: Siehe Kommando »svn upgrade«
svn: E155036: Die Arbeitskopie in »/private/tmp/r/svnwork«
ist zu alt (Format 29) für die Verwendung mit der Version »1.8.4 (r1534716)« (erwartet Format 31). Sie müssen die Arbeitskopie zuerst in ein neues Format bringen.

andy@m:svnwork $ svn upgrade
In neues Format gebracht: ».«
andy@m:svnwork $ svn up
Aktualisiere ».«:
U    content.txt
A    newfile.txt
Aktualisiert zu Revision 2.
andy@m:svnwork $ cd ../gitwork/
andy@m:gitwork (master) $ git svn fetch
andy@m:gitwork (master) $ echo 'yet another line' >> content.txt 
andy@m:gitwork (master) $ git mv newfile.txt somefile.txt
andy@m:gitwork (master) $ git add content.txt 
andy@m:gitwork (master) $ git commit -m 'changed content, moved file'
[master c92f414] changed content, moved file
 2 files changed, 1 insertion(+)
 rename newfile.txt => somefile.txt (100%)
andy@m:gitwork (master) $ git svn dcommit
Committing to file:///tmp/r/svnrepo ...
	R	newfile.txt => somefile.txt
	M	content.txt
Committed r3
	D	newfile.txt
	M	content.txt
	A	somefile.txt
W: -empty_dir: newfile.txt
r3 = e46693b582c5c85710a9a3ea5179ac8fef2d2b22 (refs/remotes/git-svn)
No changes between c92f4143403de8088363551f92bca30499842a4c and refs/remotes/git-svn
Resetting to the latest refs/remotes/git-svn
andy@m:gitwork (master) $ cd ../svnwork/
andy@m:svnwork $ svn up
Aktualisiere ».«:
D    newfile.txt
A    somefile.txt
U    content.txt
Aktualisiert zu Revision 3.
andy@m:svnwork $ ls
content.txt  somefile.txt
andy@m:svnwork $ echo 'a line from svn repo' >> content.txt 
andy@m:svnwork $ echo 'a line from svn repo' >> somefile.txt 
andy@m:svnwork $ svn commit -m 'added something from svn repo'
Sende              content.txt
Sende              somefile.txt
Übertrage Daten ..
Revision 4 übertragen.
andy@m:svnwork $ cd ../gitwork/
andy@m:gitwork (master) $ git svn rebase
	M	somefile.txt
	M	content.txt
r4 = b629f9c35a718e99a811d7fd0ada607687aebde3 (refs/remotes/git-svn)
Zunächst wird der Branch zurückgespult, um Ihre Änderungen
darauf neu anzuwenden...
master zu refs/remotes/git-svn vorgespult.
andy@m:gitwork (master) $ echo 'add more content' >> content.txt 
andy@m:gitwork (master) $ git add content.txt 
andy@m:gitwork (master) $ git mv somefile.txt anyfile.txt
andy@m:gitwork (master) $ git commit -m 'change more content'
[master 453e050] change more content
 2 files changed, 1 insertion(+)
 rename somefile.txt => anyfile.txt (100%)
andy@m:gitwork (master) $ git svn dcommit
Committing to file:///tmp/r/svnrepo ...
	R	somefile.txt => anyfile.txt
	M	content.txt
Committed r5
	D	somefile.txt
	A	anyfile.txt
	M	content.txt
W: -empty_dir: somefile.txt
r5 = e038ff05f796c6f79c63556d6a75a7e86976f060 (refs/remotes/git-svn)
No changes between 453e050c2f6e4ca0de11b42bd9329aff43ec3da0 and refs/remotes/git-svn
Resetting to the latest refs/remotes/git-svn
andy@m:gitwork (master) $ cd ../svnwork/
andy@m:svnwork $ svn up
Aktualisiere ».«:
U    content.txt
D    somefile.txt
A    anyfile.txt
Aktualisiert zu Revision 5.
andy@m:svnwork $ echo 'more svn content' >> content.txt 
andy@m:svnwork $ echo 'new content' > newfile.txt
andy@m:svnwork $ svn add newfile.txt 
A         newfile.txt
andy@m:svnwork $ svn commit -m 'more data from svn'
Sende              content.txt
Füge hinzu         newfile.txt
Übertrage Daten ..
Revision 6 übertragen.
andy@m:svnwork $ cd ../gitwork/
andy@m:gitwork (master) $ git svn fetch
	A	newfile.txt
	M	content.txt
r6 = f98167f128945f5045342366ac9a84d72873c479 (refs/remotes/git-svn)
andy@m:gitwork (master) $ git mv anyfile.txt somefile.txt
andy@m:gitwork (master) $ echo 'changed too' >> somefile.txt 
andy@m:gitwork (master) $ git add somefile.txt 
andy@m:gitwork (master) $ echo 'replaced content' > content.txt 
andy@m:gitwork (master) $ git add content.txt 
andy@m:gitwork (master) $ git commit -m 'did some changes' 
[master 0cb4cd8] did some changes
 2 files changed, 2 insertions(+), 5 deletions(-)
 rename anyfile.txt => somefile.txt (71%)
andy@m:gitwork (master) $ git svn dcommit
Committing to file:///tmp/r/svnrepo ...
	R	anyfile.txt => somefile.txt

ERROR from SVN:
Transaktion ist veraltet: Datei »/content.txt« ist veraltet
W: 0cb4cd820d65cdf4d8a18146c54740e4af0dc533 and refs/remotes/git-svn differ, using rebase:
:000000 100644 0000000000000000000000000000000000000000 cd42f7309485e8df6f795807835ee1ef9d875556 A	anyfile.txt
:100644 100644 3d69ed67cef70be1a2357dcaaffcd5cc34aaa7ce 58fc0689b4691eb02c737f7cc1faf5a860b2a1d9 M	content.txt
:000000 100644 0000000000000000000000000000000000000000 b66ba06d315d46280bb09d54614cc52d1677809f A	newfile.txt
:100644 000000 949eb457147d5e2d187c4dd426b4a05df229900d 0000000000000000000000000000000000000000 D	somefile.txt
Zunächst wird der Branch zurückgespult, um Ihre Änderungen
darauf neu anzuwenden...
Wende an: did some changes
Verwende Informationen aus der Staging-Area um einen Basisverzeichnis nachzustellen
M	content.txt
Falle zurück zum Patchen der Basis und des 3-Wege-Merges...
automatischer Merge von somefile.txt
automatischer Merge von content.txt
KONFLIKT (Inhalt): Merge-Konflikt in content.txt
Merge der Änderungen fehlgeschlagen
Anwendung des Patches fehlgeschlagen bei 0001 did some changes
Die Kopie des fehlgeschlagenen Patches befindet sich in:
   /tmp/r/gitwork/.git/rebase-apply/patch

Wenn Sie das Problem aufgelöst haben, führen Sie "git rebase --continue" aus.
Falls Sie diesen Patch auslassen möchten, führen Sie stattdessen "git rebase --skip" aus.
Um den ursprünglichen Branch wiederherzustellen und den Rebase abzubrechen,
führen Sie "git rebase --abort" aus.

rebase refs/remotes/git-svn: command returned error: 1

andy@m:gitwork (master|REBASE 1/1) $ 

Re: Fwd: Error with git-svn pushing a rename

[Benjamin Pabst]

Hi Andy,

sadly I get the same error with a downgraded svn:

$ git --version
git version 1.8.4.2
$ svn --version
svn, version 1.7.10 (r1485443)
   compiled Nov 18 2013, 18:43:16

Copyright (C) 2013 The Apache Software Foundation.
This software consists of contributions made by many people; see the NOTICE
file for more information.
Subversion is open source software, see http://subversion.apache.org/

The following repository access (RA) modules are available:

* ra_svn : Module for accessing a repository using the svn network protocol.
  - with Cyrus SASL authentication
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme
* ra_serf : Module for accessing a repository via WebDAV protocol using serf.
  - handles 'http' scheme
  - handles 'https' scheme

$ git svn dcommit
Committing to https://xxxxx.xxx/xxxxx ...
R /some/file => /some/new/filename
perl: subversion/libsvn_subr/dirent_uri.c:2500: svn_fspath__is_child:
Assertion `svn_fspath__is_canonical(child_fspath)' failed.
error: git-svn died of signal 6

Any idea what I should do next to get this working? I also tried with
a "$ git svn rebase" first, which throws no error (just an "already
up-to-date")...

Thanks for your help!

Regards
Ben

2013/11/15 Andreas Stricker <astricker@futurelab.ch>:
> Hi Benjamin
>
>> thanks for your link. Can you give me the exact version you
>> downgraded svn to?
>
> svn, Version 1.7.10 (r1485443)
>
> I tried to reproduce the problem with git version 1.8.4.2 and
> Subversion version 1.8.4 (r1534716) with a fresh and pristine
> subversion repo and a git-svn clone of it: I didn't manage to
> reproduce the rename issue. Then I switched subversion back to
> 1.7.10, created both the repo and the git-svn clone, switched
> againt to 1.8.4.2 and then got an error. Unfortunately I didn't
> check if the subversion perlbindings were regenerated, so I'm
> not exactly sure. I'll repeat the test again, as soon I've find
> the time.
>
> It looks like a fresh git svn clone may fix the problem.
>
> Regards, Andy

Re: Fwd: Error with git-svn pushing a rename

[Benjamin Pabst]

Hi,

is it possible to debug git-svn or get a more verbose / debug output
from it? I already tried with the "GIT_TRACE" variable, but it does
not include any further output on the svn methods.

Regards, Benjamin

2013/11/17 Andreas Stricker <astricker@futurelab.ch>:
> Hi Jonathan
>
>> Can you give an exact sequence of steps (including "Upgrade Subversion
>> at this step") to reproduce the problem?  That would help immensely
>> --- if at all possible, I would very much like to keep existing
>> git-svn repos working on upgrade.
>
> Of course. I've attached a text file with the commands required to
> reproduce this error.
>
> From my experiments it looks like after the subversion is upgraded
> to 1.8 the problem only occurs if "git svn fetch" fetches new changes
> from the subversion repository. Without changes in upstream subversion
> repository I couldn't reproduce the error. And a rename is required too.
>
> Hope this helps.
>
> Regards, Andy

Re: Fwd: Error with git-svn pushing a rename

[Roman Kagan]

Benjamin Pabst <benjamin.pabst85 <at> gmail.com> writes:
> is it possible to debug git-svn or get a more verbose / debug output
> from it? I already tried with the "GIT_TRACE" variable, but it does
> not include any further output on the svn methods.

I've hit this problem too, and tracked it down to what I think is a
bug in svn.  It fails in libsvn_ra_serf/commit.c:close_file() on invalid
->copy_path on the file context.

AFAICT this is do to the ra_serf version of add_file() lacking
apr_pstrdup() on the "copy_path" argument passed to it; as a result it
gets overwritten with garbage on further use:

libsvn_ra_serf/commit.c:
...
1852 static svn_error_t *
1853 add_file(const char *path,
1854          void *parent_baton,
1855          const char *copy_path,
1856          svn_revnum_t copy_revision,
1857          apr_pool_t *file_pool,
1858          void **file_baton)
1859 {
...
1875   new_file->copy_path = copy_path;
..

You can apply this workaround to get it to work:

--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -304,8 +304,9 @@ sub C {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
 	my $pbat = $self->ensure_path($dir, $deletions);
+	my $upa = $self->url_path($m->{file_a});
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
-				$self->url_path($m->{file_a}), $self->{r});
+				$upa, $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
@@ -323,8 +324,9 @@ sub R {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
 	my $pbat = $self->ensure_path($dir, $deletions);
+	my $upa = $self->url_path($m->{file_a});
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
-				$self->url_path($m->{file_a}), $self->{r});
+				$upa, $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
 	$self->chg_file($fbat, $m);


What it does is store the value to be passed to add_file() in a local
variable, and rely on perl to keep it alive through the end of function
scope, beyond the call to close_file() where it's actually used.

I'm going to submit a patch adding apr_pstrdup() to subversion folks.
Meanwhile if people find the above workarond a sensible thing to do in
git, I can submit a properly formed patch here too.

Roman.

Re: Fwd: Error with git-svn pushing a rename

[Roman Kagan]

2013/12/25 Roman Kagan <rkagan@mail.ru>:
> I've hit this problem too, and tracked it down to what I think is a
> bug in svn.
> [...]
> I'm going to submit a patch adding apr_pstrdup() to subversion folks.

http://thread.gmane.org/gmane.comp.version-control.subversion.devel/145186

Roman.

Re: Fwd: Error with git-svn pushing a rename

[Roman Kagan]

2013/12/25 Roman Kagan <rkagan@mail.ru>:
> 2013/12/25 Roman Kagan <rkagan@mail.ru>:
>> I've hit this problem too, and tracked it down to what I think is a
>> bug in svn.
>> [...]
>> I'm going to submit a patch adding apr_pstrdup() to subversion folks.
>
> http://thread.gmane.org/gmane.comp.version-control.subversion.devel/145186

FWIW the patch was accepted and committed in subversion trunk.

Nevertheless I'll submit the workaround to git-svn, too, as it's
harmless and may help some people (depending on the release cycles of
git and subversion).

Roman.

Re: Fwd: Error with git-svn pushing a rename

[Thomas Rast]

Roman Kagan <rkagan@mail.ru> writes:

> --- a/perl/Git/SVN/Editor.pm
> +++ b/perl/Git/SVN/Editor.pm
> @@ -304,8 +304,9 @@ sub C {
>  	my ($self, $m, $deletions) = @_;
>  	my ($dir, $file) = split_path($m->{file_b});
>  	my $pbat = $self->ensure_path($dir, $deletions);
> +	my $upa = $self->url_path($m->{file_a});
>  	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
> -				$self->url_path($m->{file_a}), $self->{r});
> +				$upa, $self->{r});
>  	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
>  	$self->chg_file($fbat, $m);
>  	$self->close_file($fbat,undef,$self->{pool});
> @@ -323,8 +324,9 @@ sub R {
>  	my ($self, $m, $deletions) = @_;
>  	my ($dir, $file) = split_path($m->{file_b});
>  	my $pbat = $self->ensure_path($dir, $deletions);
> +	my $upa = $self->url_path($m->{file_a});
>  	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
> -				$self->url_path($m->{file_a}), $self->{r});
> +				$upa, $self->{r});
>  	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
>  	$self->apply_autoprops($file, $fbat);
>  	$self->chg_file($fbat, $m);
>
>
> What it does is store the value to be passed to add_file() in a local
> variable, and rely on perl to keep it alive through the end of function
> scope, beyond the call to close_file() where it's actually used.
>
> I'm going to submit a patch adding apr_pstrdup() to subversion folks.
> Meanwhile if people find the above workarond a sensible thing to do in
> git, I can submit a properly formed patch here too.

If you go this way, please add a comment that explains why we need the
local variable.

-- 
Thomas Rast
tr@thomasrast.ch

[PATCH] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

Subversion serf backend in versions 1.8.5 and below has a bug that the
function creating the descriptor of a file change -- add_file() --
doesn't make a copy of its 3d argument when storing it on the returned
descriptor.  As a result, by the time this field is used (in
transactions of file copying or renaming) it may well be released.

This patch works around this bug, by storing the value to be passed as
the 3d argument to add_file() in a local variable with the same scope as
the file change descriptor, making sure their lifetime is the same.

Cc: Benjamin Pabst <benjamin.pabst85@gmail.com>
Cc: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Roman Kagan <rkagan@mail.ru>
---
 perl/Git/SVN/Editor.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index b3bcd47..ae399c3 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -304,8 +304,12 @@ sub C {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
 	my $pbat = $self->ensure_path($dir, $deletions);
+	# workaround for a bug in svn serf backend (v1.8.5 and below):
+	# store 3d argument to ->add_file() in a local variable, to make it
+	# have the same lifetime as $fbat
+	my $upa = $self->url_path($m->{file_a});
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
-				$self->url_path($m->{file_a}), $self->{r});
+				$upa, $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
@@ -323,8 +327,10 @@ sub R {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
 	my $pbat = $self->ensure_path($dir, $deletions);
+	# workaround for a bug in svn serf backend, see comment in C() above
+	my $upa = $self->url_path($m->{file_a});
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
-				$self->url_path($m->{file_a}), $self->{r});
+				$upa, $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
 	$self->chg_file($fbat, $m);
-- 
1.8.4.2

Re: [PATCH] git-svn: workaround for a bug in svn serf backend

[Jonathan Nieder]

Roman Kagan wrote:

> Subversion serf backend in versions 1.8.5 and below has a bug that the
> function creating the descriptor of a file change -- add_file() --
> doesn't make a copy of its 3d argument when storing it on the returned

3d makes me think of 3-dimensional. ;-)  I think you mean third
(or the abbreviation 3rd).

> descriptor.  As a result, by the time this field is used (in
> transactions of file copying or renaming) it may well be released.

Please describe the symptom so this patch is easy to find when other
people run into it.

Do I remember correctly that "... released and scribbled over with a
new value, causing such-and-such assertion to fire" was what happened?

> This patch works around this bug, by storing the value to be passed as
> the 3d argument to add_file() in a local variable with the same scope as
> the file change descriptor, making sure their lifetime is the same.

Could this be reproduced with a test script to make sure we don't
reintroduce the bug again later?  (It's okay if the test only fails on
machines with the problematic svn version.)

Modulo the confusing 3-dimensional arguments in comments, the code
change looks good.

Thanks and hope that helps,
Jonathan

Re: [PATCH] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

2013/12/27 Jonathan Nieder <jrnieder@gmail.com>:
> Roman Kagan wrote:
>
>> Subversion serf backend in versions 1.8.5 and below has a bug that the
>> function creating the descriptor of a file change -- add_file() --
>> doesn't make a copy of its 3d argument when storing it on the returned
>
> 3d makes me think of 3-dimensional. ;-)  I think you mean third
> (or the abbreviation 3rd).

Indeed.

>> descriptor.  As a result, by the time this field is used (in
>> transactions of file copying or renaming) it may well be released.
>
> Please describe the symptom so this patch is easy to find when other
> people run into it.

OK

> Do I remember correctly that "... released and scribbled over with a
> new value, causing such-and-such assertion to fire" was what happened?

Exactly.

>> This patch works around this bug, by storing the value to be passed as
>> the 3d argument to add_file() in a local variable with the same scope as
>> the file change descriptor, making sure their lifetime is the same.
>
> Could this be reproduced with a test script to make sure we don't
> reintroduce the bug again later?  (It's okay if the test only fails on
> machines with the problematic svn version.)

That would need a fairly fancy setup phase, as the bug triggers only
on http(s)-accessed svn repositories.  I'll take a look if there's
something already available in the existing test scripts; writing one
from scratch for this specific case is IMO beyond the reasonable
effort.

> Modulo the confusing 3-dimensional arguments in comments, the code
> change looks good.

Thanks, I'll adjust the wording and resubmit.

Roman.

Re: [PATCH] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

2013/12/27 Roman Kagan <rkagan@mail.ru>:
> 2013/12/27 Jonathan Nieder <jrnieder@gmail.com>:
>> Could this be reproduced with a test script to make sure we don't
>> reintroduce the bug again later?  (It's okay if the test only fails on
>> machines with the problematic svn version.)
>
> That would need a fairly fancy setup phase, as the bug triggers only
> on http(s)-accessed svn repositories.  I'll take a look if there's
> something already available in the existing test scripts

Turns out the stuff is all there, and the tests doing file renames and
dcomit-ting them do exist (t9115-git-svn-dcommit-funky-renames.sh for
one).

However, the httpd setup is seriously broken; I haven't managed to get
it to run on my Fedora 20 with apache 2.4.6.  Apparently git-svn tests
(almost) never get executed against an http-based repository; even
those who don't set NO_SVN_TESTS get them run against file-based
repository and thus don't trigger the error.

Someone with better apache-foo needs to take a look into that.  Once
that is sorted out I believe the tests will start triggering the bug.

Meanwhile I assume that the patch doesn't need to include an extra testcase.

Roman.

[PATCH v2] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

Subversion serf backend in versions 1.8.5 and below has a bug that the
function creating the descriptor of a file change -- add_file() --
doesn't make a copy of its third argument when storing it on the
returned descriptor.  As a result, by the time this field is used (in
transactions of file copying or renaming) it may well be released, and
the memory reused.

One of its possible manifestations is the svn assertion triggering on an
invalid path, with a message

svn_fspath__skip_ancestor: Assertion
`svn_fspath__is_canonical(child_fspath)' failed.

This patch works around this bug, by storing the value to be passed as
the third argument to add_file() in a local variable with the same scope
as the file change descriptor, making sure their lifetime is the same.

Cc: Benjamin Pabst <benjamin.pabst85@gmail.com>
Cc: Eric Wong <normalperson@yhbt.net>
Cc: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Roman Kagan <rkagan@mail.ru>
---
changes since v1:
  - fix grammar in the patch and the log message
  - refer to the triggered error message

 perl/Git/SVN/Editor.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index b3bcd47..34e8af9 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -304,8 +304,12 @@ sub C {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
 	my $pbat = $self->ensure_path($dir, $deletions);
+	# workaround for a bug in svn serf backend (v1.8.5 and below):
+	# store third argument to ->add_file() in a local variable, to make it
+	# have the same lifetime as $fbat
+	my $upa = $self->url_path($m->{file_a});
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
-				$self->url_path($m->{file_a}), $self->{r});
+				$upa, $self->{r});
 	print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->chg_file($fbat, $m);
 	$self->close_file($fbat,undef,$self->{pool});
@@ -323,8 +327,10 @@ sub R {
 	my ($self, $m, $deletions) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
 	my $pbat = $self->ensure_path($dir, $deletions);
+	# workaround for a bug in svn serf backend, see comment in C() above
+	my $upa = $self->url_path($m->{file_a});
 	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
-				$self->url_path($m->{file_a}), $self->{r});
+				$upa, $self->{r});
 	print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
 	$self->apply_autoprops($file, $fbat);
 	$self->chg_file($fbat, $m);
-- 
1.8.4.2

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Jonathan Nieder]

Roman Kagan wrote:

> Subversion serf backend in versions 1.8.5 and below has a bug that the
> function creating the descriptor of a file change -- add_file() --
> doesn't make a copy of its third argument when storing it on the
> returned descriptor.  As a result, by the time this field is used (in
> transactions of file copying or renaming) it may well be released, and
> the memory reused.
>
> One of its possible manifestations is the svn assertion triggering on an
> invalid path, with a message
>
> svn_fspath__skip_ancestor: Assertion `svn_fspath__is_canonical(child_fspath)' failed.
[...]

Makes sense.  Perhaps also worth mentioning that this is fixed by
r1553376, but no need to reroll just for that.

> Cc: Benjamin Pabst <benjamin.pabst85@gmail.com>
> Cc: Eric Wong <normalperson@yhbt.net>
> Cc: Jonathan Nieder <jrnieder@gmail.com>

No need for these lines --- the mail header already keeps track of who
is being cc-ed.

> Signed-off-by: Roman Kagan <rkagan@mail.ru>

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Eric Wong]

Jonathan Nieder <jrnieder@gmail.com> wrote:
> Roman Kagan wrote:
> 
> > Subversion serf backend in versions 1.8.5 and below has a bug that the
> > function creating the descriptor of a file change -- add_file() --
> > doesn't make a copy of its third argument when storing it on the
> > returned descriptor.  As a result, by the time this field is used (in
> > transactions of file copying or renaming) it may well be released, and
> > the memory reused.
> >
> > One of its possible manifestations is the svn assertion triggering on an
> > invalid path, with a message
> >
> > svn_fspath__skip_ancestor: Assertion `svn_fspath__is_canonical(child_fspath)' failed.
> [...]
> 
> Makes sense.  Perhaps also worth mentioning that this is fixed by
> r1553376, but no need to reroll just for that.

Thanks all, I noted this in an addendum to the commit:

    Subversion serf backend in versions 1.8.5 and below has a bug(*) that the

    ...

    * [ew: fixed in Subversion r1553376 as noted by Jonathan Nieder]

> > Cc: Benjamin Pabst <benjamin.pabst85@gmail.com>
> > Cc: Eric Wong <normalperson@yhbt.net>
> > Cc: Jonathan Nieder <jrnieder@gmail.com>
> 
> No need for these lines --- the mail header already keeps track of who
> is being cc-ed.

I don't mind seeing it in history.  At least I've gotten accustomed to
it from the Linux kernel and tracking patch flow between dev -> stable
trees.

> > Signed-off-by: Roman Kagan <rkagan@mail.ru>
> 
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

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


The following changes since commit 7794a680e63a2a11b73cb1194653662f2769a792:

  Sync with 1.8.5.2 (2013-12-17 14:12:17 -0800)

are available in the git repository at:


  git://git.bogomips.org/git-svn.git master

for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace:

  git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +0000)

----------------------------------------------------------------
Roman Kagan (1):
      git-svn: workaround for a bug in svn serf backend

 perl/Git/SVN/Editor.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Roman Kagan wrote:
>> 
>> > Subversion serf backend in versions 1.8.5 and below has a bug that the
>> > function creating the descriptor of a file change -- add_file() --
>> > doesn't make a copy of its third argument when storing it on the
>> > returned descriptor.  As a result, by the time this field is used (in
>> > transactions of file copying or renaming) it may well be released, and
>> > the memory reused.
>> >
>> > One of its possible manifestations is the svn assertion triggering on an
>> > invalid path, with a message
>> >
>> > svn_fspath__skip_ancestor: Assertion `svn_fspath__is_canonical(child_fspath)' failed.
>> [...]
>> 
>> Makes sense.  Perhaps also worth mentioning that this is fixed by
>> r1553376, but no need to reroll just for that.
>
> Thanks all, I noted this in an addendum to the commit:
>
>     Subversion serf backend in versions 1.8.5 and below has a bug(*) that the
>
>     ...
>
>     * [ew: fixed in Subversion r1553376 as noted by Jonathan Nieder]
>
>> > Cc: Benjamin Pabst <benjamin.pabst85@gmail.com>
>> > Cc: Eric Wong <normalperson@yhbt.net>
>> > Cc: Jonathan Nieder <jrnieder@gmail.com>
>> 
>> No need for these lines --- the mail header already keeps track of who
>> is being cc-ed.
>
> I don't mind seeing it in history.  At least I've gotten accustomed to
> it from the Linux kernel and tracking patch flow between dev -> stable
> trees.
>
>> > Signed-off-by: Roman Kagan <rkagan@mail.ru>
>> 
>> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
>
>
> The following changes since commit 7794a680e63a2a11b73cb1194653662f2769a792:
>
>   Sync with 1.8.5.2 (2013-12-17 14:12:17 -0800)
>
> are available in the git repository at:
>
>
>   git://git.bogomips.org/git-svn.git master
>
> for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace:
>
>   git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +0000)
>
> ----------------------------------------------------------------
> Roman Kagan (1):
>       git-svn: workaround for a bug in svn serf backend
>
>  perl/Git/SVN/Editor.pm | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Thanks. I almost missed this pull-request, though.

Will pull.

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

2013/12/28 Junio C Hamano <gitster@pobox.com>:
> Eric Wong <normalperson@yhbt.net> writes:
>>   git://git.bogomips.org/git-svn.git master
>>
>> for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace:
>>
>>   git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +0000)
>>
>> ----------------------------------------------------------------
>> Roman Kagan (1):
>>       git-svn: workaround for a bug in svn serf backend
>>
>>  perl/Git/SVN/Editor.pm | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> Thanks. I almost missed this pull-request, though.
>
> Will pull.

Thanks!

I'd like to note that it's IMO worth including in the 'maint' branch
as it's a crasher.  Especially so since the real fix has been merged
in the subversion upstream and nominated for 1.8 branch, so the
workaround may soon lose its relevance.

Roman.

Re: [PATCH] git-svn: workaround for a bug in svn serf backend

[Thomas Rast]

Roman Kagan <rkagan@mail.ru> writes:

> +	# workaround for a bug in svn serf backend (v1.8.5 and below):
> +	# store 3d argument to ->add_file() in a local variable, to make it
> +	# have the same lifetime as $fbat
> +	my $upa = $self->url_path($m->{file_a});
>  	my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
> -				$self->url_path($m->{file_a}), $self->{r});
> +				$upa, $self->{r});

Hmm, now that you put it that way, I wonder if the patch is correct.

Let me first rephrase the problem to verify that I understand the issue:

  $fbat keeps a pointer to the $upa string, without maintaining a
  reference to it.  When $fbat is destroyed, it needs this string, so we
  must ensure that the lifetime of $upa is at least as long as that of
  $fbat.

However, does Perl make any guarantees as to the order in which local
variables are unreferenced and then destroyed?  I can't find any such
guarantee.

In the absence of such, wouldn't we have to keep $upa in an outer,
separate scope to ensure that $fbat is destroyed first?

-- 
Thomas Rast
tr@thomasrast.ch

Re: [PATCH] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

2013/12/30 Thomas Rast <tr@thomasrast.ch>:
> Roman Kagan <rkagan@mail.ru> writes:
>
>> +     # workaround for a bug in svn serf backend (v1.8.5 and below):
>> +     # store 3d argument to ->add_file() in a local variable, to make it
>> +     # have the same lifetime as $fbat
>> +     my $upa = $self->url_path($m->{file_a});
>>       my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
>> -                             $self->url_path($m->{file_a}), $self->{r});
>> +                             $upa, $self->{r});
>
> Hmm, now that you put it that way, I wonder if the patch is correct.
>
> Let me first rephrase the problem to verify that I understand the issue:
>
>   $fbat keeps a pointer to the $upa string, without maintaining a
>   reference to it.  When $fbat is destroyed, it needs this string, so we
>   must ensure that the lifetime of $upa is at least as long as that of
>   $fbat.

No.  The string is needed in subversion's close_file(), so we want to
keep it alive until close_file() returns. Surviving till the end of
the current function scope is sufficient for that.

> However, does Perl make any guarantees as to the order in which local
> variables are unreferenced and then destroyed?

We don't care about the order they are destroyed WRT each other.

Roman.

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Junio C Hamano]

Roman Kagan <rkagan@mail.ru> writes:

> 2013/12/28 Junio C Hamano <gitster@pobox.com>:
>> Eric Wong <normalperson@yhbt.net> writes:
>>>   git://git.bogomips.org/git-svn.git master
>>>
>>> for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace:
>>>
>>>   git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +0000)
>>>
>>> ----------------------------------------------------------------
>>> Roman Kagan (1):
>>>       git-svn: workaround for a bug in svn serf backend
>>>
>>>  perl/Git/SVN/Editor.pm | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> Thanks. I almost missed this pull-request, though.
>>
>> Will pull.
>
> Thanks!

That's redundant; the project should thank you for contributing, not
the other way around.

> I'd like to note that it's IMO worth including in the 'maint' branch
> as it's a crasher.  Especially so since the real fix has been merged
> in the subversion upstream and nominated for 1.8 branch, so the
> workaround may soon lose its relevance.

I do not quite get this part, though.

If they refused to fix it for real, it would make it likely that
this workaround will stay relevant for a long time, in which case it
would be worth cherry-picking to an older maintenance track.  But if
this workaround is expected to lose its relevance shortly, I see it
as one less reason to cherry-pick it to an older maintenance track.

Confused...

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

2013/12/30 Junio C Hamano <gitster@pobox.com>:
> Roman Kagan <rkagan@mail.ru> writes:
>> I'd like to note that it's IMO worth including in the 'maint' branch
>> as it's a crasher.  Especially so since the real fix has been merged
>> in the subversion upstream and nominated for 1.8 branch, so the
>> workaround may soon lose its relevance.
>
> I do not quite get this part, though.
>
> If they refused to fix it for real, it would make it likely that
> this workaround will stay relevant for a long time, in which case it
> would be worth cherry-picking to an older maintenance track.  But if
> this workaround is expected to lose its relevance shortly, I see it
> as one less reason to cherry-pick it to an older maintenance track.
>
> Confused...

I thought it was exactly the other way around.  By the time the next
feature release reaches users, chances are they'd already have
subversion with the fix.  OTOH the workaround would benefit those who
get their maintenance release of git (e.g. through their Linux distro
update) before they get their maintenance release of subversion.

Documentation/SubmittingPatches also suggests to submit bugfixes
against 'maint'.

But I might have got it wrong...

Roman.

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Andreas Stricker]

Hi Roman

>>>   git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +0000)
> Thanks!

Well thanks to you for finding and fixing this bug that really annoyed
me just before Christmas again. Your bug analysis proved my observation
that even a fresh checkout (as I suggested in my last message) didn't
fix this issue.

And thanks to the reviewers too. Awesome!

~Andy

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Roman Kagan]

2013/12/31 Roman Kagan <rkagan@mail.ru>:
> 2013/12/30 Junio C Hamano <gitster@pobox.com>:
>> Roman Kagan <rkagan@mail.ru> writes:
>>> I'd like to note that it's IMO worth including in the 'maint' branch
>>> as it's a crasher.  Especially so since the real fix has been merged
>>> in the subversion upstream and nominated for 1.8 branch, so the
>>> workaround may soon lose its relevance.
>>
>> I do not quite get this part, though.
>>
>> If they refused to fix it for real, it would make it likely that
>> this workaround will stay relevant for a long time, in which case it
>> would be worth cherry-picking to an older maintenance track.  But if
>> this workaround is expected to lose its relevance shortly, I see it
>> as one less reason to cherry-pick it to an older maintenance track.
>>
>> Confused...
>
> I thought it was exactly the other way around.  By the time the next
> feature release reaches users, chances are they'd already have
> subversion with the fix.  OTOH the workaround would benefit those who
> get their maintenance release of git (e.g. through their Linux distro
> update) before they get their maintenance release of subversion.

So this actually happened: 1.8.5.3 is out, and some distributions are
shipping it (Arch, Debian), but the workaround didn't make it there.

Could you please consider including it in 'maint', so that 1.8.5.4
brings them a working combination of git and subversion?

Roman.

Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend

[Junio C Hamano]

Roman Kagan <rkagan@mail.ru> writes:

> 2013/12/31 Roman Kagan <rkagan@mail.ru>:
>> 2013/12/30 Junio C Hamano <gitster@pobox.com>:
>>> Roman Kagan <rkagan@mail.ru> writes:
>>>> I'd like to note that it's IMO worth including in the 'maint' branch
>>>> as it's a crasher.  Especially so since the real fix has been merged
>>>> in the subversion upstream and nominated for 1.8 branch, so the
>>>> workaround may soon lose its relevance.
>>>
>>> I do not quite get this part, though.
>>>
>>> If they refused to fix it for real, it would make it likely that
>>> this workaround will stay relevant for a long time, in which case it
>>> would be worth cherry-picking to an older maintenance track.  But if
>>> this workaround is expected to lose its relevance shortly, I see it
>>> as one less reason to cherry-pick it to an older maintenance track.
>>>
>>> Confused...
>>
>> I thought it was exactly the other way around.  By the time the next
>> feature release reaches users, chances are they'd already have
>> subversion with the fix.  OTOH the workaround would benefit those who
>> get their maintenance release of git (e.g. through their Linux distro
>> update) before they get their maintenance release of subversion.
>
> So this actually happened: 1.8.5.3 is out, and some distributions are
> shipping it (Arch, Debian), but the workaround didn't make it there.

The way I read your message was that the fix on the subversion side
is already there and this patch to work it around on our end is of
no importance.

But actually you wanted to say quite the opposite.  They are slow
and it is likely that we need to work their bug around for a while.

If so, then I think it might make sense to cherry-pick it to the
maint branch, even though we usually apply only fixes to our own
bugs to the maintenance track.