[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Tree-conflicts: do_entry_deletion segfault

From: Neels Hofmeyr <neels_at_elego.de>
Date: Tue, 09 Sep 2008 01:59:41 +0200

Julian Foad wrote:
> On Fri, 2008-09-05 at 04:25 +0200, Neels Hofmeyr wrote:
>> Quoting:
>>
>> * subversion/libsvn_wc/update_editor.c
>> (bump_dir_info): Update a doc-string to allow for a directory to have tree
>> conflicts.
>> (entry_has_local_mods, check_tree_conflict): New functions.
>> (do_entry_deletion): Have the parent's admin access baton passed in by the
>> caller. Check for tree conflicts.
>> ### Broken when parent_adm_access is NULL.
>>
>>
>> I've already posted a fix for this one almost two weeks ago. It wasn't being
>> heard much.
>
> That's my fault, Neels. I read your message then, and the earlier ones,
> and started to reply, but lost my way. Sorry.
>
>> So I'm posting an update, using today's tree-conflicts branch, compacting my
>> explanations. You may refer to the following mail for more (confusing) detail:
>
> Excellent. Thanks.
>
>> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=142154
>> Date: Sat, 23 Aug 2008 22:43:11 +0200
>> From: Neels Hofmeyr <neels_at_elego.de>
>> Subject: Re: Tree-conflicts branch - log message / review
>>
>>
>>
>> Two problems coincide:
>>
>> 1. Tree conflict detection is skipped when examining the path that was
>> specifically given as target. E.g. `svn update A/D/G' means that while G's
>> contents are checked for tree conflicts, G itself will not be checked.
>>
>> 2. Tree conflict detection segfaults if run in that situation given in point
>> 1 above.
>>
>>
>> This patch fixes both, plus it tweaks two "unrelated" cmdline test to ignore
>> a 'C' marker caused by the fix. The fix itself consists of three parts:
>>
>> i) A change made in the tree-conflicts branch is reverted to trunk:
>> do_entry_deletion() does not need a PARENT_ADM_ACCESS parameter. It was
>> being used to indicate the specific case mentioned in point 1 above, so as
>> to be able to skip tree-conflicts detection in that case, oddly enough. Stsp
>> has pointed out that this change must have been committed by accident.
>
> Heh. It was me who added that. It wasn't by accident. I was trying to
> solve the problem of when the target is the root of the WC and therefore
> there is no parent WC directory. However, I didn't properly solve that
> problem, and I forgot to mention it in the log message, so we might very
> well call it an accident.

lol, sorry and thanks :)

But, about when the target is the working copy root: that got me thinking.
There is no parent directory to report conflicts in! And what will
adm_retrieve return in such a case?

This is the exact case:
1. Directory X has been issued on the svn commandline (e.g. `svn update X')
2. This directory X is a tree-conflict victim, and
3. X also happens to be the root of the local working copy.

(It doesn't make sense to think of the repository root being a tree
conflicts victim. Here, though, X has been checked out explicitly, and is
the working copy root, as one would check out `trunk')

There has been word that tree conflicts should be reported at the actual
victim nodes, not in their respective parent directories. That would
sufficiently prevent this problem altogether.

But for now, I tried to create conflicts in a working copy's root dir. It
seems that at least update is waterproof, in a way. I've attached scripts
that run the cases listed here, numbers corresponding:

1) Try to update the working copy root to a deleted revision (no conflicts):
   "svn: Target path does not exist"

2) Prop conflicts are handled as they should.

3) Run `svn rm .' in the working copy root and try to commit:
   States that the commit succeeded but warns about ugly problems.
   Status complains about a "missing" file removed by svn itself.

4) First run `svn rm .' in the working copy root, then try to update it to a
   deleted revision: same as 1): "svn: Target path does not exist".
   Also, `svn rm .' fails to lock a subdir, whatever that means.

5) First run `svn rm .' in the working copy root, then try to update it to a
   modified revision: same as 3): commit succeeded but warns about ugly
   problems. Also, the update "restores" a file removed by svn itself.

It is pretty obvious that these cases are not considered valid. At least no
user out there will run into the problem I am poking for here, because it
doesn't even run that far, for other reasons.

Have I missed a case? Otherwise everything is fair enough, at least from the
tree-conflicts point of view ... switch and merge remain to be checked, but
I expect the same stuff happening.

> I made just one tweak: There was a point where you wrote "if
> (svn_io_check_path(...) != SVN_NO_ERROR ..." which would leak the error
> object if it threw an error. Instead, we have to call the function on
> its own, as "SVN_ERR(svn_io_check_path(...));". Any error that it might
> return is one that we don't know how to deal with, so it is OK to just
> return the error to our caller.

Ah yes, true. (Hasn't this been said before somewhere, too?)
I checked and yours is better. I had assumed that the SVN_ERR indicates a
missing directory, but duh.

> Committed in r32932.
Thanks!

~Neels

-- 
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696  mobile: +49 177 2345869  fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194






+ which svn
/arch/elego/svn/prefix/tc-prefix/bin/svn
+ which svnadmin
/arch/elego/svn/prefix/tc-prefix/bin/svnadmin
+ svn --version
svn, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository access (RA) modules are available:

* ra_neon : Module for accessing a repository via WebDAV protocol using Neon.
  - handles 'http' scheme
  - handles 'https' scheme
* ra_svn : Module for accessing a repository using the svn network protocol.
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme

+ svnadmin --version
svnadmin, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository back-end (FS) modules are available:

* fs_fs : Module for working with a plain file (FSFS) repository.

+ name=root_conflict1
+ name_D1=root_conflict1_D1
+ rm -rf repos/root_conflict1
+ rm -rf wc/root_conflict1
+ rm -rf wc/root_conflict1_D1
+ cd repos
+ svnadmin create root_conflict1
+ echo -e '[general]\nanon-access = write\n'
+ cd ..
+ pwd
/arch/elego/svn/test
+ svnserve -d -r /arch/elego/svn/test/repos
+ cd wc
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict1
Checked out revision 0.
+ cd root_conflict1
+ pwd
/arch/elego/svn/test/wc/root_conflict1
+ echo -e 'x\ny\nz\n'
+ mkdir -p D1/D2
+ echo y
+ svn add x D1
A x
A D1
A D1/y
A D1/D2
+ svn ci -m message
Adding D1
Adding D1/D2
Adding D1/y
Adding x
Transmitting file data ..
Committed revision 1.
+ svn rm D1
D D1/D2
D D1/y
D D1
+ svn diff
Index: D1/y
===================================================================
--- D1/y (revision 0)
+++ D1/y (working copy)
@@ -1 +0,0 @@
-y
+ svn ci -m m
Deleting D1

Committed revision 2.
+ svn diff -r 1
Index: D1/y
===================================================================
--- D1/y (revision 1)
+++ D1/y (working copy)
@@ -1 +0,0 @@
-y
+ cd ..
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict1/D1_at_1 root_conflict1_D1
A root_conflict1_D1/D2
A root_conflict1_D1/y
Checked out revision 1.
+ cd root_conflict1_D1
+ pwd
/arch/elego/svn/test/wc/root_conflict1_D1
+ svn up
subversion/libsvn_repos/reporter.c:1161: (apr_err=160005)
svn: Target path does not exist
+ svn status
+ killall svnserve
+ set +x

+ which svn
/arch/elego/svn/prefix/tc-prefix/bin/svn
+ which svnadmin
/arch/elego/svn/prefix/tc-prefix/bin/svnadmin
+ svn --version
svn, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository access (RA) modules are available:

* ra_neon : Module for accessing a repository via WebDAV protocol using Neon.
  - handles 'http' scheme
  - handles 'https' scheme
* ra_svn : Module for accessing a repository using the svn network protocol.
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme

+ svnadmin --version
svnadmin, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository back-end (FS) modules are available:

* fs_fs : Module for working with a plain file (FSFS) repository.

+ name=root_conflict2
+ name_D1=root_conflict2_D1
+ rm -rf repos/root_conflict2
+ rm -rf wc/root_conflict2
+ rm -rf wc/root_conflict2_D1
+ cd repos
+ svnadmin create root_conflict2
+ echo -e '[general]\nanon-access = write\n'
+ cd ..
+ pwd
/arch/elego/svn/test
+ svnserve -d -r /arch/elego/svn/test/repos
+ cd wc
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict2
Checked out revision 0.
+ cd root_conflict2
+ pwd
/arch/elego/svn/test/wc/root_conflict2
+ echo -e 'x\ny\nz\n'
+ mkdir -p D1/D2
+ echo y
+ svn add x D1
A x
A D1
A D1/y
A D1/D2
+ svn propset test:prop testval D1
property 'test:prop' set on 'D1'
+ svn ci -m message
Adding D1
Adding D1/D2
Adding D1/y
Adding x
Transmitting file data ..
Committed revision 1.
+ svn propset test:prop proptest D1
property 'test:prop' set on 'D1'
+ svn diff

Property changes on: D1
___________________________________________________________________
Modified: test:prop
   - testval
   + proptest

+ svn ci -m m
Sending D1

Committed revision 2.
+ svn diff -r 1

Property changes on: D1
___________________________________________________________________
Modified: test:prop
   - testval
   + proptest

+ cd ..
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict2/D1_at_1 root_conflict2_D1
A root_conflict2_D1/D2
A root_conflict2_D1/y
 U root_conflict2_D1
Checked out revision 1.
+ cd root_conflict2_D1
+ pwd
/arch/elego/svn/test/wc/root_conflict2_D1
+ svn propset test:prop valdemar .
property 'test:prop' set on '.'
+ yes p
+ svn up
Conflict for property 'test:prop' discovered on ''.
Select: (p) postpone, (df) diff-full, (e) edit,
        (s) show all options: C .
Updated to revision 2.
+ svn status
 C .
? dir_conflicts.prej
+ killall svnserve
+ set +x

+ which svn
/arch/elego/svn/prefix/tc-prefix/bin/svn
+ which svnadmin
/arch/elego/svn/prefix/tc-prefix/bin/svnadmin
+ svn --version
svn, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository access (RA) modules are available:

* ra_neon : Module for accessing a repository via WebDAV protocol using Neon.
  - handles 'http' scheme
  - handles 'https' scheme
* ra_svn : Module for accessing a repository using the svn network protocol.
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme

+ svnadmin --version
svnadmin, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository back-end (FS) modules are available:

* fs_fs : Module for working with a plain file (FSFS) repository.

+ name=root_conflict3
+ name_D1=root_conflict3_D1
+ rm -rf repos/root_conflict3
+ rm -rf wc/root_conflict3
+ rm -rf wc/root_conflict3_D1
+ cd repos
+ svnadmin create root_conflict3
+ echo -e '[general]\nanon-access = write\n'
+ cd ..
+ pwd
/arch/elego/svn/test
+ svnserve -d -r /arch/elego/svn/test/repos
+ cd wc
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict3
Checked out revision 0.
+ cd root_conflict3
+ pwd
/arch/elego/svn/test/wc/root_conflict3
+ echo -e 'x\ny\nz\n'
+ mkdir D1
+ echo y
+ svn add x D1
A x
A D1
A D1/y
+ svn ci -m message
Adding D1
Adding D1/y
Adding x
Transmitting file data ..
Committed revision 1.
+ cd ..
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict3/D1 root_conflict3_D1
A root_conflict3_D1/y
Checked out revision 1.
+ cd root_conflict3_D1
+ pwd
/arch/elego/svn/test/wc/root_conflict3_D1
+ svn rm .
D .
+ svn status
D .
! y
+ svn ci -m message
Deleting .
subversion/libsvn_client/commit.c:926: (apr_err=200000)
svn: Commit succeeded, but other errors follow:
subversion/libsvn_client/commit.c:944: (apr_err=155005)
svn: Error bumping revisions post-commit (details follow):
subversion/libsvn_wc/lock.c:997: (apr_err=155005)
svn: Directory '/arch/elego/svn/test/wc/.svn' containing working copy admin area is missing
+ killall svnserve
+ set +x

+ which svn
/arch/elego/svn/prefix/tc-prefix/bin/svn
+ which svnadmin
/arch/elego/svn/prefix/tc-prefix/bin/svnadmin
+ svn --version
svn, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository access (RA) modules are available:

* ra_neon : Module for accessing a repository via WebDAV protocol using Neon.
  - handles 'http' scheme
  - handles 'https' scheme
* ra_svn : Module for accessing a repository using the svn network protocol.
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme

+ svnadmin --version
svnadmin, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository back-end (FS) modules are available:

* fs_fs : Module for working with a plain file (FSFS) repository.

+ name=root_conflict3
+ name_D1=root_conflict3_D1
+ rm -rf repos/root_conflict3
+ rm -rf wc/root_conflict3
+ rm -rf wc/root_conflict3_D1
+ cd repos
+ svnadmin create root_conflict3
+ echo -e '[general]\nanon-access = write\n'
+ cd ..
+ pwd
/arch/elego/svn/test
+ svnserve -d -r /arch/elego/svn/test/repos
+ cd wc
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict3
Checked out revision 0.
+ cd root_conflict3
+ pwd
/arch/elego/svn/test/wc/root_conflict3
+ echo -e 'x\ny\nz\n'
+ mkdir -p D1/D2
+ echo y
+ svn add x D1
A x
A D1
A D1/y
A D1/D2
+ svn ci -m message
Adding D1
Adding D1/D2
Adding D1/y
Adding x
Transmitting file data ..
Committed revision 1.
+ svn rm D1
D D1/D2
D D1/y
D D1
+ svn diff
Index: D1/y
===================================================================
--- D1/y (revision 0)
+++ D1/y (working copy)
@@ -1 +0,0 @@
-y
+ svn ci -m m
Deleting D1

Committed revision 2.
+ svn diff -r 1
Index: D1/y
===================================================================
--- D1/y (revision 1)
+++ D1/y (working copy)
@@ -1 +0,0 @@
-y
+ cd ..
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict3/D1_at_1 root_conflict3_D1
A root_conflict3_D1/D2
A root_conflict3_D1/y
Checked out revision 1.
+ cd root_conflict3_D1
+ pwd
/arch/elego/svn/test/wc/root_conflict3_D1
+ svn rm .
D .
subversion/libsvn_wc/lock.c:1002: (apr_err=155005)
svn: Unable to lock 'D2'
+ svn up
subversion/libsvn_repos/reporter.c:1161: (apr_err=160005)
svn: Target path does not exist
+ svn status
D .
+ killall svnserve
+ set +x

+ which svn
/arch/elego/svn/prefix/tc-prefix/bin/svn
+ which svnadmin
/arch/elego/svn/prefix/tc-prefix/bin/svnadmin
+ svn --version
svn, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository access (RA) modules are available:

* ra_neon : Module for accessing a repository via WebDAV protocol using Neon.
  - handles 'http' scheme
  - handles 'https' scheme
* ra_svn : Module for accessing a repository using the svn network protocol.
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme

+ svnadmin --version
svnadmin, version 1.6.0 (dev build)
   compiled Sep 4 2008, 01:24:53

Copyright (C) 2000-2008 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository back-end (FS) modules are available:

* fs_fs : Module for working with a plain file (FSFS) repository.

+ name=root_conflict3
+ name_D1=root_conflict3_D1
+ rm -rf repos/root_conflict3
+ rm -rf wc/root_conflict3
+ rm -rf wc/root_conflict3_D1
+ cd repos
+ svnadmin create root_conflict3
+ echo -e '[general]\nanon-access = write\n'
+ cd ..
+ pwd
/arch/elego/svn/test
+ svnserve -d -r /arch/elego/svn/test/repos
+ cd wc
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict3
Checked out revision 0.
+ cd root_conflict3
+ pwd
/arch/elego/svn/test/wc/root_conflict3
+ echo -e 'x\ny\nz\n'
+ mkdir D1
+ echo y
+ svn add x D1
A x
A D1
A D1/y
+ svn ci -m message
Adding D1
Adding D1/y
Adding x
Transmitting file data ..
Committed revision 1.
+ echo z
+ svn diff
Index: D1/y
===================================================================
--- D1/y (revision 1)
+++ D1/y (working copy)
@@ -1 +1 @@
-y
+z
+ svn ci -m m
Sending D1/y
Transmitting file data .
Committed revision 2.
+ svn diff -r 1
Index: D1/y
===================================================================
--- D1/y (revision 1)
+++ D1/y (working copy)
@@ -1 +1 @@
-y
+z
+ cd ..
+ pwd
/arch/elego/svn/test/wc
+ svn co svn://localhost/root_conflict3/D1_at_1 root_conflict3_D1
A root_conflict3_D1/y
Checked out revision 1.
+ cd root_conflict3_D1
+ pwd
/arch/elego/svn/test/wc/root_conflict3_D1
+ svn rm .
D .
+ svn up
Restored 'y'
U y
Updated to revision 2.
+ svn status
D .
+ svn ci -m message
Deleting .
subversion/libsvn_client/commit.c:926: (apr_err=200000)
svn: Commit succeeded, but other errors follow:
subversion/libsvn_client/commit.c:944: (apr_err=155005)
svn: Error bumping revisions post-commit (details follow):
subversion/libsvn_wc/lock.c:997: (apr_err=155005)
svn: Directory '/arch/elego/svn/test/wc/.svn' containing working copy admin area is missing
+ killall svnserve
+ set +x

Received on 2008-09-09 02:00:27 CEST

This is an archived mail posted to the Subversion Dev mailing list.