RE: Case-only renames on Windows (issue #3702)

[Bolstridge, Andrew]

> -Original Message-
> From: Branko Čibej [mailto:br...@xbc.nu] On Behalf Of Branko Cibej
> Sent: 24 March 2011 15:32
> To: dev@subversion.apache.org
> Subject: Re: Case-only renames on Windows (issue #3702)
> 
[snip]
> In short, things aren't /that/ tricky as far as renames go. They're a lot 
> trickier
> during update and commit, if the editor hits a local file that has the wrong
> case on-disk, but is otherwise versioned (I've known brain-dead tools to
> case-only rename the files they'd worked on).
> 
> -- Brane
> 

Now you've reminded me of something, those brain-dead tools that rename files 
you have under svn, and when you come to commit them, they get ignored because 
the system think's they're new files. TortoiseSVN has some special-case 
handling for these, try to commit one of them and Tortoise will pop up a 
messagebox asking if you want to rename the case and retry the operation (and 
gives you a menu option to rename files directly).

http://tortoisesvn.net/docs/release/TortoiseSVN_en/tsvn-dug-rename.html

Of course, the best approach would just to make the entire system 
case-insensitive and cater to the lowest common denominator (after all, the 
http transport should be case-insensitive too, I get to the same place typing 
in www.apache.com or www.APACHE.com, but not when it comes to the svn repo url).

Re: Case-only renames on Windows (issue #3702)

[Branko Čibej]

On 25.03.2011 11:09, Bolstridge, Andrew wrote:
> (after all, the http transport should be case-insensitive too, I get
> to the same place typing in www.apache.com or www.APACHE.com, but not
> when it comes to the svn repo url).

Oops. Host names are are case-insensitive by definition. Paths in URLs
are not.

-- Brane

RE: Case-only renames on Windows (issue #3702)

[Bert Huijben]

> -Original Message-
> From: Branko Čibej [mailto:br...@xbc.nu] On Behalf Of Branko Cibej
> Sent: vrijdag 25 maart 2011 11:50
> To: Bolstridge, Andrew
> Cc: dev@subversion.apache.org
> Subject: Re: Case-only renames on Windows (issue #3702)
> 
> On 25.03.2011 11:09, Bolstridge, Andrew wrote:
> > (after all, the http transport should be case-insensitive too, I get
> > to the same place typing in www.apache.com or www.APACHE.com, but
> not
> > when it comes to the svn repo url).
> 
> Oops. Host names are are case-insensitive by definition. Paths in URLs
> are not.

Since Subversion 1.6.0 we normalize all schema and hostnames in urls to lower 
case. (Urls stored in working copies are automatically converted on upgrading 
to 1.6.x).

The rest of the url is kept as-is.

In Subversion 1.7 we extended this normalization a bit further to always use 
the same character escaping, so http://host/path and http://host/p%61th are the 
same url in 1.7, but not in 1.6. 

Only the first format is canonical, so you get an error if you pass the second 
form to an internal API. Clients can use svn_uri_canonicalize() to get the 
canonical format of a user provided url.
(Older clients are expected to use the now deprecated svn_path_canonicalize())

Bert

Re: Case-only renames on Windows (issue #3702)

[John Beranek]

On 25/03/11 10:09, Bolstridge, Andrew wrote:
> 
>> -Original Message- From: Branko Čibej [mailto:br...@xbc.nu]
>> On Behalf Of Branko Cibej Sent: 24 March 2011 15:32 To:
>> dev@subversion.apache.org Subject: Re: Case-only renames on Windows
>> (issue #3702)
>> 
> [snip]
>> In short, things aren't /that/ tricky as far as renames go. They're
>> a lot trickier during update and commit, if the editor hits a local
>> file that has the wrong case on-disk, but is otherwise versioned
>> (I've known brain-dead tools to case-only rename the files they'd
>> worked on).
>> 
>> -- Brane
>> 
> 
>  Of course, the best approach would just to make the entire system
> case-insensitive and cater to the lowest common denominator (after
> all, the http transport should be case-insensitive too, I get to the
> same place typing in www.apache.com or www.APACHE.com, but not when
> it comes to the svn repo url).

You may need to explain why people can't put copies of Linux 2.4 kernels
in the system then:

http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.4.37.y.git;a=blob;f=net/ipv4/netfilter/ipt_tos.c
http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.4.37.y.git;a=blob;f=net/ipv4/netfilter/ipt_TOS.c

Cheers,

John.

-- 
John Beranek To generalise is to be an idiot.
http://redux.org.uk/ -- William Blake



smime.p7s
Description: S/MIME Cryptographic Signature

Re: Case-only renames on Windows (issue #3702)

[Philip Martin]

Johan Corveleyn  writes:

> C:\Temp\test>dir /B
> ToDo
>
> C:\Temp\test>move todo todo
>
> C:\Temp\test>dir /B
> todo

So "svn mv todo todo" will be a case-only rename, I guess that's OK if
it is what users expect.

What if the file is already lower-case "todo"?  Subversion is happy to
do

  svn rm todo
  svn cp todo@base todo

to generate a replace on commit.  Should "svn mv todo todo" generate a
replace when the file is already "todo"?

-- 
Philip

Re: Case-only renames on Windows (issue #3702)

[Johan Corveleyn]

On Fri, Mar 25, 2011 at 1:43 PM, Philip Martin
 wrote:
> Johan Corveleyn  writes:
>
>> C:\Temp\test>dir /B
>> ToDo
>>
>> C:\Temp\test>move todo todo
>>
>> C:\Temp\test>dir /B
>> todo
>
> So "svn mv todo todo" will be a case-only rename, I guess that's OK if
> it is what users expect.
>
> What if the file is already lower-case "todo"?  Subversion is happy to
> do
>
>  svn rm todo
>  svn cp todo@base todo
>
> to generate a replace on commit.  Should "svn mv todo todo" generate a
> replace when the file is already "todo"?

Good question. I'm not sure.

There are two other options that I can think of (let's call your
suggestion option 1)):

2) svn could just ignore the no-op move, and do nothing (no warning,
no error, nothing).

3) svn could error out, like it does now, with "Cannot move path
'XXX/todo' into itself"


I think 2) would be more "the Windows way", so if we are aiming for
platform-fidelity here, that would be my choice.

Again, looking at the native "move" command on Windows, we can see
that "move todo todo" acts as if it has done a move, but really didn't
do anything, not even "touch" the file or the parent dir. OTOH, if we
do a real case-only rename, "move todo TODO", the timestamp of the
parent dir is updated (i.e., it really has changed). See below.

[[[
C:\Temp\test>echo test > todo

C:\Temp\test>dir
 Volume in drive C has no label.
 Volume Serial Number is 98BA-E619

 Directory of C:\Temp\test

25/03/2011  13:57  .
25/03/2011  13:57  ..
25/03/2011  13:57 7 todo
   1 File(s)  7 bytes
   2 Dir(s)   2.945.830.912 bytes free

C:\Temp\test>time /t
13:59

C:\Temp\test>move todo todo
1 file(s) moved.

C:\Temp\test>dir
 Volume in drive C has no label.
 Volume Serial Number is 98BA-E619

 Directory of C:\Temp\test

25/03/2011  13:57  .
25/03/2011  13:57  ..
25/03/2011  13:57 7 todo
   1 File(s)  7 bytes
   2 Dir(s)   2.945.384.448 bytes free

C:\Temp\test>move todo TODO
1 file(s) moved.

C:\Temp\test>dir
 Volume in drive C has no label.
 Volume Serial Number is 98BA-E619

 Directory of C:\Temp\test

25/03/2011  14:00  .
25/03/2011  14:00  ..
25/03/2011  13:57 7 TODO
   1 File(s)  7 bytes
   2 Dir(s)   2.945.363.968 bytes free
]]]


Cheers,
-- 
Johan

Re: Case-only renames on Windows (issue #3702)

[Johan Corveleyn]

On Sun, Mar 20, 2011 at 9:32 PM, Johan Corveleyn  wrote:
> Some thoughts:
>
> - There is only a problem if the dst_path gets case-normalized to one
> of the source paths. Otherwise, the case-normalization really does
> need to happen.

Hm, this thought may be incorrect (or at least "unexpected" for
windows users). In fact, if we look at how the native "move" behaves,
it never case-normalizes the target path:

[[[
C:\Temp>dir /B test
TODO

C:\Temp>echo anothertest > bla

C:\Temp>move bla test\toDO
Overwrite C:\Temp\test\toDO? (Yes/No/All): y
1 file(s) moved.

C:\Temp>dir /B test
toDO

C:\Temp>type test\todo
anothertest
]]]

So it seems that, if we want "svn mv" to behave more like "move" on
Windows, the target path should never be normalized to the on-disk
casing. Just use it as is ...

Cheers,
-- 
Johan

Windows folks: can someone test this?

[C. Michael Pilato]

Attached is a patch that I hope will fix svnrdump_tests.py 33 on Windows.
But in my total Linuxness, I am unable to verify.  Help?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
{{{
Fix svnrdump_tests.py 33 failures on Windows.

* subversion/tests/cmdline/svntest/main.py
  (run_svnrdump): Use binary mode when running 'svnrdump'.
}}}

Index: subversion/tests/cmdline/svntest/main.py
===
--- subversion/tests/cmdline/svntest/main.py(revision 1084938)
+++ subversion/tests/cmdline/svntest/main.py(working copy)
@@ -599,12 +599,12 @@
 
 def run_svnrdump(stdin_input, *varargs):
   """Run svnrdump with VARARGS, returns exit code as int; stdout, stderr as
-  list of lines (including line terminators)."""
+  list of lines (including line terminators).  Use binary mode for output."""
   if stdin_input:
-return run_command_stdin(svnrdump_binary, 0, 1, 0, stdin_input,
+return run_command_stdin(svnrdump_binary, 0, 1, 1, stdin_input,
  *(_with_auth(_with_config_dir(varargs
   else:
-return run_command(svnrdump_binary, 1, 0,
+return run_command(svnrdump_binary, 1, 1,
*(_with_auth(_with_config_dir(varargs
 
 def run_svnsync(*varargs):


signature.asc
Description: OpenPGP digital signature

Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties

[neil.winton]

Hi,

There appears to be a problem with the new svnrdump command handling revisions 
where multiple simultaneous property changes occur on a single file or 
directory. I couldn't see any mention of this in the user or dev lists 
elsewhere.

To recreate to bug, do the following:

svnadmin create test-original
svnadmin load test-original < original.dump
svnrdump dump file://$PWD/test-original > new.dump
svnadmin create test-new
ln -s /bin/true test-new/hooks/pre-revprop-change
svnrdump load file://$PWD/test-new < new.dump

The result of the load (also using svnadmin instead of svnrdump) is:

* Loaded revision 0.
svnrdump: E140001: Unrecognised record type in stream

The problem appears to be that multiple property changes are not consolidated 
into a single set of changes associated with a node, but instead appear as 
"orphaned" Prop-delta sections. So in the original dump you see:

Node-path:
Node-kind: dir
Node-action: change
Prop-content-length: 42
Content-length: 42

K 3
foo
V 3
bar
K 3
bar
V 3
baz
PROPS-END

But in the output from svnrdump this appears as follows:

Node-path:
Node-kind: dir
Node-action: change
Prop-delta: true
Prop-content-length: 26
Content-length: 26

K 3
foo
V 3
bar
PROPS-END


Prop-delta: true
Prop-content-length: 26
Content-length: 26

K 3
bar
V 3
baz
PROPS-END

Sorry, but I'm not quite up to providing a fix as yet, but I hope this helps 
someone to figure it out.

Regards,
Neil

Neil Winton
BT Innovate and Design, Tools Platform
Tel: +44 (0)1473 651976, E-Mail: neil.win...@bt.com
This email contains information from British Telecommunications plc which may 
be confidential or privileged. The information is intended to be for the use of 
the individuals or the entities named above. If you are not the intended 
recipient be aware that any disclosure, copying, distribution or use of the 
contents of this information is prohibited. If you have received this 
electronic message in error, please notify us by telephone or email (to the 
numbers or address above) immediately.

Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties

[Hyrum K Wright]

On Fri, Mar 25, 2011 at 9:50 AM,   wrote:
> Hi,
>
>
>
> There appears to be a problem with the new svnrdump command handling
> revisions where multiple simultaneous property changes occur on a single
> file or directory. I couldn’t see any mention of this in the user or dev
> lists elsewhere.
>
>
>
> To recreate to bug, do the following:
>
>
>
> svnadmin create test-original
>
> svnadmin load test-original < original.dump
>
> svnrdump dump file://$PWD/test-original > new.dump
>
> svnadmin create test-new
>
> ln -s /bin/true test-new/hooks/pre-revprop-change
>
> svnrdump load file://$PWD/test-new < new.dump
>
>
>
> The result of the load (also using svnadmin instead of svnrdump) is:
>
>
>
> * Loaded revision 0.
>
> svnrdump: E140001: Unrecognised record type in stream
>
>
>
> The problem appears to be that multiple property changes are not
> consolidated into a single set of changes associated with a node, but
> instead appear as “orphaned” Prop-delta sections. So in the original dump
> you see:
>
>
>
> Node-path:
>
> Node-kind: dir
>
> Node-action: change
>
> Prop-content-length: 42
>
> Content-length: 42
>
>
>
> K 3
>
> foo
>
> V 3
>
> bar
>
> K 3
>
> bar
>
> V 3
>
> baz
>
> PROPS-END
>
>
>
> But in the output from svnrdump this appears as follows:
>
>
>
> Node-path:
>
> Node-kind: dir
>
> Node-action: change
>
> Prop-delta: true
>
> Prop-content-length: 26
>
> Content-length: 26
>
>
>
> K 3
>
> foo
>
> V 3
>
> bar
>
> PROPS-END
>
>
>
>
>
> Prop-delta: true
>
> Prop-content-length: 26
>
> Content-length: 26
>
>
>
> K 3
>
> bar
>
> V 3
>
> baz
>
> PROPS-END
>
>
>
> Sorry, but I’m not quite up to providing a fix as yet, but I hope this helps
> someone to figure it out.

Neil,
Thanks for the report.  In attempting to reproduce, I wasn't even able
to get as far as comparing the dumpfiles, as svnrdump asserted
somewhere in the process.  I opened issue 3844[1] to track this bug,
and committed an XFailing test in r1085428.  I also marked the issue
as important for a 1.7.0 release.

-Hyrum

[1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844

Performance benchmarks

[Mark Phippard]

Hi,

I have been working on a framework for writing tests to record
performance.  I have something good enough to share:

https://ctf.open.collab.net/sf/projects/csvn

It is pretty easy to add new tests if you have ideas on more tests you
think we should add.  I think I have pretty good coverage of the major
functions.  The wiki on the site I linked to above has details on how
I have constructed the current tests.  I am going to put out a call to
users for feedback and try to get more people to run the tests and
record results.

I am not claiming these are anything definitive or even that we will
use them to help us make the release decision, but I think it is a
start on coming up with some reproducible tests that people can run
easily.  If after people look at and run the tests they think they are
useful or can be tweaked to be useful, then great.  If not, then at
least I got to write some code for a change :)

The tests are written in Java because that is what I know and it gives
me good cross platform coverage.  However, the Java just drives the
command line so all you need to do is have the svn command line in
your PATH and that is what it uses for all the work.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties

[C. Michael Pilato]

On 03/25/2011 11:31 AM, Hyrum K Wright wrote:
> Neil,
> Thanks for the report.  In attempting to reproduce, I wasn't even able
> to get as far as comparing the dumpfiles, as svnrdump asserted
> somewhere in the process.  I opened issue 3844[1] to track this bug,
> and committed an XFailing test in r1085428.  I also marked the issue
> as important for a 1.7.0 release.
> 
> -Hyrum
> 
> [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844

See r1085523.

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



signature.asc
Description: OpenPGP digital signature

Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties

[Hyrum K Wright]

On Fri, Mar 25, 2011 at 2:11 PM, C. Michael Pilato  wrote:
> On 03/25/2011 11:31 AM, Hyrum K Wright wrote:
>> Neil,
>> Thanks for the report.  In attempting to reproduce, I wasn't even able
>> to get as far as comparing the dumpfiles, as svnrdump asserted
>> somewhere in the process.  I opened issue 3844[1] to track this bug,
>> and committed an XFailing test in r1085428.  I also marked the issue
>> as important for a 1.7.0 release.
>>
>> -Hyrum
>>
>> [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844
>
> See r1085523.

Thanks, Mike.

Neil, when you get a chance, could you test a r1085523 and see if it
fixes your problem?

Cheers,
-Hyrum

Re: svn commit: r1081487 - /subversion/trunk/subversion/tests/svn_test_main.c

[Daniel Shahaf]

Log message might want to say why you did that... ie what problem is
being fixed.

hwri...@apache.org wrote on Mon, Mar 14, 2011 at 17:30:33 -:
> Author: hwright
> Date: Mon Mar 14 17:30:33 2011
> New Revision: 1081487
> 
> URL: http://svn.apache.org/viewvc?rev=1081487&view=rev
> Log:
> * subversion/tests/svn_test_main.c
>   (do_test_num): Flush stdout between individual tests.
> 
> Modified:
> subversion/trunk/subversion/tests/svn_test_main.c
> 
> Modified: subversion/trunk/subversion/tests/svn_test_main.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/svn_test_main.c?rev=1081487&r1=1081486&r2=1081487&view=diff
> ==
> --- subversion/trunk/subversion/tests/svn_test_main.c (original)
> +++ subversion/trunk/subversion/tests/svn_test_main.c Mon Mar 14 17:30:33 2011
> @@ -339,6 +339,8 @@ do_test_num(const char *progname,
>if (desc->msg == NULL)
>  printf("WARNING: New-style test descriptor is missing a docstring.\n");
>  
> +  fflush(stdout);
> +
>skip_cleanup = test_failed;
>  
>return test_failed;
> 
> 

svn_wc_diff_callbacks4_t and abspaths

[Daniel Shahaf]

>From doc string of svn_wc_diff_callbacks4_t:

"Note that @a path is a relative path, not just the basename of the path."


>From merge_file_added() (which implements svn_wc_diff_callbacks4_t.file_added):

SVN_ERR_ASSERT(svn_dirent_is_absolute(mine_abspath));


Given that MINE_ABSPATH is the parameter called PATH in the doc string,
I think the contradiction is obvious...

Re: svn commit: r1081097 - in /subversion/trunk/subversion: include/private/svn_cache.h libsvn_fs_fs/caching.c libsvn_fs_fs/tree.c libsvn_subr/cache-inprocess.c tests/libsvn_subr/cache-test.c

[Daniel Shahaf]

stef...@apache.org wrote on Sun, Mar 13, 2011 at 12:40:50 -:
> Author: stefan2
> Date: Sun Mar 13 12:40:49 2011
> New Revision: 1081097
> 
> URL: http://svn.apache.org/viewvc?rev=1081097&view=rev
> Log:
> Prepare for the introduction of a generic cache statistics API:
> make cache-inprocess caches identifiable.
> 
> * subversion/include/private/svn_cache.h
>   (svn_cache__create_inprocess): add id parameter
> * subversion/libsvn_subr/cache-inprocess.c
>   (inprocess_cache_t): add id member
>   (svn_cache__create_inprocess): store id parameter
> * subversion/libsvn_fs_fs/caching.c
>   (init_callbacks): new function; factored out from the init function
>   (svn_fs_fs__initialize_caches): provide IDs / prefixes to all cache types,
>call new init sub-function
> * subversion/libsvn_fs_fs/tree.c
>   (make_txn_root): adapt to internal API change
> * subversion/tests/libsvn_subr/cache-test.c
>   (test_inprocess_cache_basic): dito
> 
> Modified:
> subversion/trunk/subversion/include/private/svn_cache.h
> subversion/trunk/subversion/libsvn_fs_fs/caching.c
> subversion/trunk/subversion/libsvn_fs_fs/tree.c
> subversion/trunk/subversion/libsvn_subr/cache-inprocess.c
> subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
> 
> Modified: subversion/trunk/subversion/include/private/svn_cache.h
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_cache.h?rev=1081097&r1=1081096&r2=1081097&view=diff
> ==
> --- subversion/trunk/subversion/include/private/svn_cache.h (original)
> +++ subversion/trunk/subversion/include/private/svn_cache.h Sun Mar 13 
> 12:40:49 2011
> @@ -136,6 +136,7 @@ svn_cache__create_inprocess(svn_cache__t
>  apr_int64_t pages,
>  apr_int64_t items_per_page,
>  svn_boolean_t thread_safe,
> +const char *id,

Need to document the ID parameter please...

>  apr_pool_t *pool);
>  /**
>   * Creates a new cache in @a *cache_p, communicating to a memcached
> 
> Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/caching.c?rev=1081097&r1=1081096&r2=1081097&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Sun Mar 13 12:40:49 
> 2011
> @@ -64,6 +64,24 @@ warn_on_cache_errors(svn_error_t *err,
>return SVN_NO_ERROR;
>  }
>  
> +static svn_error_t *
> +init_callbacks(svn_cache__t *cache,
> +   svn_fs_t *fs,
> +   svn_boolean_t no_handler,
> +   apr_pool_t *pool)
> +{
> +  if (cache != NULL)
> +{
> +  if (! no_handler)
> +SVN_ERR(svn_cache__set_error_handler(cache,
> + warn_on_cache_errors,
> + fs,
> + pool));
> +
> +}
> +
> +  return SVN_NO_ERROR;
> +}
>  
>  svn_error_t *
>  svn_fs_fs__initialize_caches(svn_fs_t *fs,
> @@ -101,11 +119,12 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
>svn_fs_fs__serialize_id,
>svn_fs_fs__deserialize_id,
>sizeof(svn_revnum_t),
> -  1, 100, FALSE, fs->pool));
> -  if (! no_handler)
> -  SVN_ERR(svn_cache__set_error_handler(ffd->rev_root_id_cache,
> -  warn_on_cache_errors, fs, pool));
> +  1, 100, FALSE,
> +  apr_pstrcat(pool, prefix, "RRI",
> +  (char *)NULL),
> +  fs->pool));
>  
> +  SVN_ERR(init_callbacks(ffd->rev_root_id_cache, fs, no_handler, pool));
>  
>/* Rough estimate: revision DAG nodes have size around 320 bytes, so
> * let's put 16 on a page. */
> @@ -123,11 +142,12 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
>  svn_fs_fs__dag_serialize,
>  svn_fs_fs__dag_deserialize,
>  APR_HASH_KEY_STRING,
> -1024, 16, FALSE, fs->pool));
> -  if (! no_handler)
> -SVN_ERR(svn_cache__set_error_handler(ffd->rev_node_cache,
> - warn_on_cache_errors, fs, pool));
> +1024, 16, FALSE,
> +apr_pstrcat(pool, prefix, "DAG",
> +(char *)NULL),
> +  

[PATCH] #3840: Re: Error with tree conflict data in database after update from 1.6

[Daniel Shahaf]

I've added my thoughts to the issue [1]; essentially, I've observed that
svn 1.6 stores tree_conflict_data="(foo bar)" where 1.7 stores
tree_conflict_data="foo bar", and this patch makes 'svn upgrade'
adjust accordingly.

I haven't tested upgrading from interim 1.7-dev formats, and I don't
recall offhand at which point we made the transition to lose the extra
level of ().

Reviews welcome, otherwise I'll commit it in a few days.

[[[
Fix issue #3840, "empty conflict description in db after update from 1.6".

* subversion/libsvn_wc/entries.c
  (write_entry): Strip one level of skel-wrapping () from TREE_CONFLICT_DATA
when upgrading entries.
]]]

[[[
Index: subversion/libsvn_wc/entries.c
===
--- subversion/libsvn_wc/entries.c  (revision 1085641)
+++ subversion/libsvn_wc/entries.c  (working copy)
@@ -1824,8 +1824,20 @@ write_entry(struct write_baton **entry_node,
 
   if (entry->tree_conflict_data)
 {
+  /* Issue #3840: 1.6 uses one pair of () more than we do, so
+ strip it.  Thus, "()" becomes NULL and "((skel1) (skel2))"
+ becomes "(skel1) (skel2)". */
+  svn_skel_t *skel;
+
   actual_node = MAYBE_ALLOC(actual_node, scratch_pool);
-  actual_node->tree_conflict_data = entry->tree_conflict_data;
+  skel = svn_skel__parse(entry->tree_conflict_data,
+ strlen(entry->tree_conflict_data),
+ scratch_pool);
+  if (skel->children)
+actual_node->tree_conflict_data = svn_skel__unparse(skel->children,
+result_pool)->data;
+  else
+actual_node->tree_conflict_data = NULL;
 }
 
   if (entry->file_external_path != NULL)
]]]

[1] http://subversion.tigris.org/issues/show_bug.cgi?id=3840

Alan Wood wrote on Mon, Mar 21, 2011 at 23:52:20 +1300:
> 
> On 21 Mar 2011 at 10:48, Stefan Sperling wrote:
> 
> > On Mon, Mar 21, 2011 at 12:33:34PM +1300, Alan Wood wrote:
> > >  There should not have been any conflict data of any sort in this 
> > > working copy as I am the only developer and I have only every had 
> > > one working copy.
> > 
> > That's not a valid assumption.
> > It is possible to inflict tree conflicts upon yourself, with only
> > one working copy: http://subversion.apache.org/faq.html#self-tree-conflict
> 
> Thanks for the link. I had read this a while ago, but forgotton all about it.
> 
> > > I hope someone may be able to plug this little hole in the upgrade 
> > > system.
> > 
> > Looks like the upgrade code shouldn't insert empty conflict description
> > skels into the DB. Can you file an issue that points to this mailing list
> > thread, so that this doesn't fall through the cracks? Thanks.
> 
> Filed issue# 3840
> http://subversion.tigris.org/issues/show_bug.cgi?id=3840
> 
> Sorry for the confusion with the version labels.
> 
> > 
> > And thanks for testing!
> No trouble, would like to have more time for it.
> 
> Alan
> 
> 
> 
> Alan Wood
> 
> Napier
> New Zealand
> Phone +64 6 835 4505
>