Re: svnserve under Linux inetd hangs, burning CPU cycles, under too low TCP-sendbuffer.

[Dr. Andreas Krüger]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hello,

> He additionally reports that this is reproducible not only with
> 1.5.1, but with Debian's build of 1.6.9.  (That mail is probably in
> the moderation queue now.)

thank you Peter! (To me, this is the first indication that someone
actually noticed my attempts to tie that bug down where you folks can
hit it.)

Peter refers to my attempt to ask the Debian bug tracking system to
forward my Debian bug report to this list. Nice try on my part... As
that doesn't look like it's getting anywhere, here comes a manual copy.

The new information in a nutshell:

* Reproducible with Debian's unstable "1.6.9dfsg-1" version, as Peter
mentioned,

* In all my experiments thus far, the client always has been Ubuntu's
1.6.5dfsg-1ubuntu1.

Below comes the whole story as available in the Debian bug tracking
system. Basically, this is a summary of what I know at this point.
Turn to http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=571457 to
follow Debian developments.

So far, so good. As for me, I intent to sit back and relax until
either someone asks me for further information or else until I receive
clearance to introduce this to Subversion's bug tracking system.

Regards, and thank you all for providing fine software,

Andreas
 
==

Package: subversion
Version: 1.6.9dfsg-1
X-Debbugs-CC: dev@subversion.apache.org

Hello, subversionists,

I have a subversion repository with a single revision, consisting of a
single roughly 10 MB file with random data.

I use svnserve to serve that repository.

I use a client to access the svnserve over the network, via "svn co".

(The client happens to be Ubuntu's 1.6.5dfsg-1ubuntu1, in case that
matters.)

The server is an openvz guest. When, on the corresponding host, I set

vzctl set NNN --tcpsndbuf 415k:715k

or lower (my guess is the first number is the one that matters), the
checkout starts, there is some initial network traffic, but then
svnserve starts to eat up all CPU cycles it can get and no further
progress is made, until I kill the svnserve process manually on the
server. Just killing the client does not seem to stop the waste of CPU
cycles on the server.

When I increase the tcpsndbuf above that value, there is no problem.

However, when I check in a much larger file into the repository (I do
that directly, using file:/// - access), and try to check out again
(using svnserve), the problem reappears.

This is quite reproducibly, with svnserve as a standalone daemon as well
as svnserve running under inetd. For the latter case, I made no precise
experiments about the --tcpsndbuf numbers.

This is the summary. I have written three mails yesterday and today to
the dev@subversion.apache.org mailing list, but have not received any
answers there yet. There is lots of nitty-gritty background information
in those mails, which, hopefully, you'll be able to find here:

http://mail-archives.apache.org/mod_mbox/subversion-dev/201002.mbox/browser

Regards, and thank you for providing fine software,

Andreas

P.S.: I ask the Debian bug tracking system to forward a copy of this
mail to the SVN dev mailing list. This contains two pieces of
information not available in my previous mails to that list:

* The bug is reproducible with the latest subversion software version I
can easily install, short of compiling myself.

* The client I'm using is, and always has been, 1.6.5dfsg-1ubuntu1. I
think I misquoted it to be 1.5 in one of my earlier mails. I apologize
if this caused any confusion!

- --
Dr. Andreas Krüger, Berater, DV-RATIO NORDWEST GmbH
andreas.krue...@dv-ratio.com
GPG/PGP Fingerprint 8063 4A9B 362D 4220 A546  14C1 EA19 AADC FD44 5EB7

DV-RATIO NORDWEST GmbH
Tel: +49 (0)211 / 577 996-0
Fax: +49 (0)211 / 577 996-26
http://www.dv-ratio.com 
Sitz der Gesellschaft Habsburgerstraße 12, 40547 Düsseldorf
Registergericht Düsseldorf HRB 34330
USt-IdNr.:  DE811321837
Steuer-Nr.: 809/44031
Geschäftsführung: Günter Gerstmann
Prokura: Trudbert Vetter, Uwe Wolfram

DV-RATIO - "Kompetenz und Zuverlässigkeit seit 1980"
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkuHfvQACgkQ6hmq3P1EXrfSwQCglTixze7pSlyDpl3ZTikWprbR
ko4AnR2/sHePNzU2BMeLvrPCRMCL9hxu
=vrmm
-END PGP SIGNATURE-

Re: Update required Java version to 5.0?

[Blair Zajac]

Hyrum K. Wright wrote:

Just going through the svnj source, and I started wondering why we're not using 
generics in our own java packages.  Generics were introduced in Java 5.0, but 
turns out that we only require 1.3 in our own source.  Given that 5.0 was 
introduced in 2004, isn't it about time we relax our requirements and update 
our code to use the new features of 5.0?  The change to our package namespace 
presents a good opportunity to do so.


+1.  Seeing casts through the code is just ugly and it makes it easier for 
clients to use the library so they don't have to use casts.


Also, 1.4 has been end-of-life by Oracle(Sun) so most people should be on 1.5.

Blair

Re: Update required Java version to 5.0?

[Mark Phippard]

On Thu, Feb 25, 2010 at 6:49 PM, Hyrum K. Wright
 wrote:
> Just going through the svnj source, and I started wondering why we're not 
> using generics in our own java packages.  Generics were introduced in Java 
> 5.0, but turns out that we only require 1.3 in our own source.  Given that 
> 5.0 was introduced in 2004, isn't it about time we relax our requirements and 
> update our code to use the new features of 5.0?  The change to our package 
> namespace presents a good opportunity to do so.
>

I am pretty ambivalent on it.  It would be a convenient time to do it,
but at the same time off the top of my head I do not see where our
code is going to benefit much from it.  I do not recall us using a lot
of Collections in our code and the API signatures do not return them,
so why change from arrays to generics?  It is also going to be more
invasive for our users that have to convert their code even more.

I think the thing that would swing me in favor of this would be seeing
the actual improvements it could make to our code base.  I am just not
really seeing that.

-- 
Thanks

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

Re: Update required Java version to 5.0?

[Branko Čibej]

Hyrum K. Wright wrote:
> Just going through the svnj source, and I started wondering why we're not 
> using generics in our own java packages.  Generics were introduced in Java 
> 5.0, but turns out that we only require 1.3 in our own source.  Given that 
> 5.0 was introduced in 2004, isn't it about time we relax our requirements and 
> update our code to use the new features of 5.0?  The change to our package 
> namespace presents a good opportunity to do so.
>   

Generics in Java are just glorified syntactic sugar, but they /do/ help
the compiler to produce marginally better error messages ... so as far
as I'm concerned, let's do that.

-- Brane

Re: Anybody going to the Apache Retreat in April?

[Branko Čibej]

Hyrum K. Wright wrote:
> There's an Apache Retreat in Ireland in April:
> http://apache.eventbrite.com/
>
> I'm tempted to go, if only so I can get interaction with a large-ish number 
> of Subversion users.  Is anybody else planning on attending?
>   

Tickets booked.

-- Brane

Update required Java version to 5.0?

[Hyrum K. Wright]

Just going through the svnj source, and I started wondering why we're not using 
generics in our own java packages.  Generics were introduced in Java 5.0, but 
turns out that we only require 1.3 in our own source.  Given that 5.0 was 
introduced in 2004, isn't it about time we relax our requirements and update 
our code to use the new features of 5.0?  The change to our package namespace 
presents a good opportunity to do so.

-Hyrum

Re: subversion 1.6 hanging when used with gnome-keyring 2.28

[Ben Hall]

Yes, it's definitely unlocked. I did not ignore the dialog. I clicked 
'Allow Always'.


keyring_tool --info reports the following:

Keyring locked : no
Auto lock on idle : no
Auto lock idle timeout : 0 (in sec)

I can reproduce this by simply running 'svn ls' against a repo in a loop.

Thanks,
--
Ben

Senthil Kumaran S wrote:

Ben Hall wrote:

Hi all,

I've encountered a problem whereby svn operations to an authenticated
apache2 server are hanging indefinitely.

It happens erratically but often enough that it's normally reproducible
within 20 operations.

I'm using ubuntu 9.10 on the client side.

I've reproduced this with subversion 1.6.4, 1.6.5 and 1.6.9 in
conjunction with gnome-keyring 2.28.1 and 2.28.2.


Is your gnome-keyring unlocked? If not unlock it first. Ideally, there 
is a gnome dialog which is thrown up after unlocking, saying "Allow 
Access to keyring" where you can say allow once, always, etc. May be you 
ve ignored this dialog or there is some problem in opening this up.


Thank You.

[fsfs revprop packing] 'hotcopy' and a live sqlite revprops.db

[Daniel Shahaf]

fs_fs.c:svn_fs_fs__hotcopy() uses this code to copy revprops.db:
1597   /* Copy the packed revprop db. */
1598   if (format >= SVN_FS_FS__MIN_PACKED_REVPROP_FORMAT)
1599 {
1600   SVN_ERR(svn_io_dir_file_copy(src_subdir, dst_subdir, 
PATH_REVPROPS_DB,
1601pool));
1602 }

This post 
 
on the sqlite-users mailing list implies that an exclusive lock must be
used for copying an sqlite db, and that plain copying might result in
a corrupt database.

As far as I can see, 'hotcopy' doesn't take the steps recommended
in that (sub)thread to avoid corruption.

Could someone more familiar with sqlite comment on this?  Could
'hotcopy', as now written, potentially corrupt the revprops db
(of the hotcopy target)?

Re: Bug in svn merge --show-revs eligible ^/subversion/trunk ^/subversion/branches/1.6.x

[Stefan Sperling]

On Thu, Feb 25, 2010 at 03:52:25PM -0500, Paul Burba wrote:
> Stefan, do you recall, or can you make an educated guess as to what
> client version you used to make the 1.6.x-r40452 branch in r880530?

Must have been a trunk build from around that timeframe.

> Did you remember how you made the branch?  URL-to-URL copy I'm
> assuming?

I've always been doing URL-to-URL copies to create branches, yes.

Stefan

Re: Bug in svn merge --show-revs eligible ^/subversion/trunk ^/subversion/branches/1.6.x

[Paul Burba]

On Thu, Feb 25, 2010 at 1:23 PM, Paul Burba  wrote:
> On Thu, Feb 25, 2010 at 10:42 AM, Kamesh Jayachandran  
> wrote:
>
>> svn mergeinfo --show-revs eligible
>> https://svn.apache.org/repos/asf/subversion/trunk
>> https://svn.apache.org/repos/asf/subversion/branches/1.6.x
>>
>> Above command lists 'r876233' while that has already been merged!
>>
>> With regards
>> Kamesh Jayachandran
>
> As Bert pointed out, the mergeinfo on the 1.6.x branch got pretty well
> hosed in r891009 (not 3891009 :-)
>
> C:\SVN\src-branch-1.6.x>svn log -r891009
> https://svn.apache.org/repos/asf/subversion/branches/1.6.x
> 
> r891009 | hwright | 2009-12-15 16:45:45 -0500 (Tue, 15 Dec 2009) | 15 lines
>
> Manually merge the 1.6.x-r40452 branch back to 1.6.x:
>
>  * r880525, r880526
>   Fix 'svn resolve --accept=theirs-conflict' for targets which live inside
>   a subdirectory of the current working directory.
>   Justification:
>     'svn resolve --accept=theirs-conflict' cannot be used reliably.
>   Notes:
>     A backport branch is at ^/subversion/branches/1.6.x-r40452.
>     1.6.x has an additional bug which does not occur on trunk, requiring
>     additional fixes exclusive to 1.6.x. See the log message of r880532
>     (which shows what the bug is), and see r880533 (which fixes the bug).
>   Votes:
>     +1: stsp, pburba, cmpilato
>
> 
>
>
> Hyrum did this "manual merge" because the reintegrate merge of
> 1.6.x-r40452 wouldn't work:
>
> Dec 16 09:03:22         hwright: pong (from yesterday)
> Dec 16 09:04:03        pburba: I was having a difficulty
> reintegrating a branch to 1.6.x, and ended up doing it manually
> Dec 16 09:04:18         hwright: Which one?
> Dec 16 09:04:52         nm, I see the log
> Dec 16 09:04:53        The most recent merge to the branch 
> (1.6.x-r40xxx)
> Dec 16 09:05:19        I don't know if it was a user error or a
> mergeinfo migration error or something else
> Dec 16 09:05:34        but since I eventually got the merge done, I
> don't care *too* much now :P
> Dec 16 09:06:12         But *I* care :-)
> Dec 16 09:06:20        heh.  I'm glad
> Dec 16 09:09:48         hwright: Works with 1.6.x for me, you were
> using a trunk client?
> Dec 16 09:09:58        nope
> Dec 16 09:10:21         Do you recall exactly what went wrong?
> Dec 16 09:10:43        the error I was getting was something along
> the lines of " doesn't exist"
> Dec 16 09:11:06        with the mystical URL being one of the
> webdav urls with revision and path in it
> Dec 16 09:11:29        my guess is that it couldn't find the branch
> in a specific revision, and that the problem was relating to
> mergeinfo...or something
> Dec 16 09:18:45         I voted on that branch Monday and had
> reintegrated it without problem then too.  H.  You using ra_neon
> or ra_serf?
> Dec 16 09:20:02        neon
> Dec 16 09:20:13        (that's the ra method du jure for me)
> Dec 16 09:24:20         Ah ha, I see the error with neon!
> Dec 16 09:24:36         svn:
> '/repos/asf/!svn/bc/875961/subversion/branches/1.6.x' path not found
> Dec 16 09:24:48        exactly
> Dec 16 09:25:06        In all honesty, I didn't even think to try 
> serf
> Dec 16 09:25:26         I wouldn't have either.  It's usually the
> other way around.
> Dec 16 09:25:58         Honestly I was taking a complete shot in the
> dark on the ra layer...got my lucky guess allotment for the month
> Dec 16 09:27:42 *       pburba notes the x86-macosx-gnu shared buildbot
> tested 1@891009 with ra_neon without issue
> Dec 16 09:28:25        sure, the code that was merged was good
> Dec 16 09:28:35        it was just *doing* the merge that was 
> troublesome
> Dec 16 09:29:17         I meant that the test suite isn't picking up
> on this problem in the merge tests
> Dec 16 09:30:08 *       hwright listens to the cacophony of children crying
> in the other room
> Dec 16 09:30:22         Would have been nice for a simple
> reproduction in the test suite to fail is all
> Dec 16 09:30:29        sure
> Dec 16 09:30:45         anyway, looking into it right now
>
> The merge didn't work due to an intersection of several problems:
>
> Discussion of busted merge:
> http://subversion.tigris.org/issues/show_bug.cgi?id=3547
> Issue #3547: http://svn.haxx.se/dev/archive-2009-12/0338.shtml
> Self-referential mergeinfo mystery:
> http://svn.haxx.se/dev/archive-2009-12/0337.shtml
>
> Anyhow, why he did a manual merge are not important as what caused the
> breakage is either fixed or nominated for backport (see * r892050,
> 892085 in STATUS - which only needs one more vote).
>
> The manual merge itself isn't a problem either, rather the problem is
> that when stsp created the branch in r880530, all the mergeinfo
> changed:
>
> C:\SVN>svn diff --depth empty
> https://svn.apache.org/repos/asf/subversion/branches/1.6.x-r40...@880530
> -r880529:880530
>
> Proper

Re: two questions (and a proposed patch) regarding svn:ignore

[Peter Samuelson]

Sorry to revive a thread from 3 weeks ago, but I had a thought.
These things do not happen to me every day, you know!

[Stefan Sperling]
> When adding a recursive directory tree, we only heed the global ignores
> list. Recall that the svn:ignore property is not inherited.
> Subversion is adding the directory to version control, and immediately
> proceeds to read the global ignores list, and the (obviously non-existent)
> svn:ignore property of the new directory
> (see subversion/libsvn_client/add.c:add_dir_recursive()).

This makes me wonder if 'svn add' / 'svn mkdir' should explicitly copy
svn:ignore to the child directory - by default, or with a flag or
config option.  I believe this honors the spirit of what svn:ignore is
supposed to accomplish.

Pro: It is what a lot of people would expect.
Con: It is what a lot of people wouldn't expect.

I think a lot of users would welcome this functionality, though.  They
do complain that svn:ignore isn't inherited.  I hate to suggest yet
another config option, as relying on config options to settle design
conflicts is a sure path to madness, but it may actually be reasonable
here, given the potential utility.

(Come to think of it, I bet a way to specify "all new directories" in
auto-props would be welcome too.)
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: Anybody going to the Apache Retreat in April?

[Justin Erenkrantz]

On Thu, Feb 25, 2010 at 10:44 AM, Greg Stein  wrote:
> Pondering, but no plan yet...

I'll probably be there, but not confirmed just yet.  -- justin

Re: Anybody going to the Apache Retreat in April?

[Greg Stein]

Pondering, but no plan yet...

On Feb 25, 2010 8:32 AM, "Hyrum K. Wright" 
wrote:

There's an Apache Retreat in Ireland in April:
http://apache.eventbrite.com/

I'm tempted to go, if only so I can get interaction with a large-ish number
of Subversion users.  Is anybody else planning on attending?

-Hyrum

Re: Bug in svn merge --show-revs eligible ^/subversion/trunk ^/subversion/branches/1.6.x

[Paul Burba]

On Thu, Feb 25, 2010 at 10:42 AM, Kamesh Jayachandran  wrote:

> svn mergeinfo --show-revs eligible
> https://svn.apache.org/repos/asf/subversion/trunk
> https://svn.apache.org/repos/asf/subversion/branches/1.6.x
>
> Above command lists 'r876233' while that has already been merged!
>
> With regards
> Kamesh Jayachandran

As Bert pointed out, the mergeinfo on the 1.6.x branch got pretty well
hosed in r891009 (not 3891009 :-)

C:\SVN\src-branch-1.6.x>svn log -r891009
https://svn.apache.org/repos/asf/subversion/branches/1.6.x

r891009 | hwright | 2009-12-15 16:45:45 -0500 (Tue, 15 Dec 2009) | 15 lines

Manually merge the 1.6.x-r40452 branch back to 1.6.x:

 * r880525, r880526
   Fix 'svn resolve --accept=theirs-conflict' for targets which live inside
   a subdirectory of the current working directory.
   Justification:
 'svn resolve --accept=theirs-conflict' cannot be used reliably.
   Notes:
 A backport branch is at ^/subversion/branches/1.6.x-r40452.
 1.6.x has an additional bug which does not occur on trunk, requiring
 additional fixes exclusive to 1.6.x. See the log message of r880532
 (which shows what the bug is), and see r880533 (which fixes the bug).
   Votes:
 +1: stsp, pburba, cmpilato




Hyrum did this "manual merge" because the reintegrate merge of
1.6.x-r40452 wouldn't work:

Dec 16 09:03:22 hwright: pong (from yesterday)
Dec 16 09:04:03pburba: I was having a difficulty
reintegrating a branch to 1.6.x, and ended up doing it manually
Dec 16 09:04:18 hwright: Which one?
Dec 16 09:04:52 nm, I see the log
Dec 16 09:04:53The most recent merge to the branch 
(1.6.x-r40xxx)
Dec 16 09:05:19I don't know if it was a user error or a
mergeinfo migration error or something else
Dec 16 09:05:34but since I eventually got the merge done, I
don't care *too* much now :P
Dec 16 09:06:12 But *I* care :-)
Dec 16 09:06:20heh.  I'm glad
Dec 16 09:09:48 hwright: Works with 1.6.x for me, you were
using a trunk client?
Dec 16 09:09:58nope
Dec 16 09:10:21 Do you recall exactly what went wrong?
Dec 16 09:10:43the error I was getting was something along
the lines of " doesn't exist"
Dec 16 09:11:06with the mystical URL being one of the
webdav urls with revision and path in it
Dec 16 09:11:29my guess is that it couldn't find the branch
in a specific revision, and that the problem was relating to
mergeinfo...or something
Dec 16 09:18:45 I voted on that branch Monday and had
reintegrated it without problem then too.  H.  You using ra_neon
or ra_serf?
Dec 16 09:20:02neon
Dec 16 09:20:13(that's the ra method du jure for me)
Dec 16 09:24:20 Ah ha, I see the error with neon!
Dec 16 09:24:36 svn:
'/repos/asf/!svn/bc/875961/subversion/branches/1.6.x' path not found
Dec 16 09:24:48exactly
Dec 16 09:25:06In all honesty, I didn't even think to try serf
Dec 16 09:25:26 I wouldn't have either.  It's usually the
other way around.
Dec 16 09:25:58 Honestly I was taking a complete shot in the
dark on the ra layer...got my lucky guess allotment for the month
Dec 16 09:27:42 *   pburba notes the x86-macosx-gnu shared buildbot
tested 1@891009 with ra_neon without issue
Dec 16 09:28:25sure, the code that was merged was good
Dec 16 09:28:35it was just *doing* the merge that was 
troublesome
Dec 16 09:29:17 I meant that the test suite isn't picking up
on this problem in the merge tests
Dec 16 09:30:08 *   hwright listens to the cacophony of children crying
in the other room
Dec 16 09:30:22 Would have been nice for a simple
reproduction in the test suite to fail is all
Dec 16 09:30:29sure
Dec 16 09:30:45 anyway, looking into it right now

The merge didn't work due to an intersection of several problems:

Discussion of busted merge:
http://subversion.tigris.org/issues/show_bug.cgi?id=3547
Issue #3547: http://svn.haxx.se/dev/archive-2009-12/0338.shtml
Self-referential mergeinfo mystery:
http://svn.haxx.se/dev/archive-2009-12/0337.shtml

Anyhow, why he did a manual merge are not important as what caused the
breakage is either fixed or nominated for backport (see * r892050,
892085 in STATUS - which only needs one more vote).

The manual merge itself isn't a problem either, rather the problem is
that when stsp created the branch in r880530, all the mergeinfo
changed:

C:\SVN>svn diff --depth empty
https://svn.apache.org/repos/asf/subversion/branches/1.6.x-r40...@880530
-r880529:880530

Property changes on: .
___
Modified: svn:mergeinfo
   Reverse-merged /subversion/branches/in-memory-cache:r869829-871452
   Reverse-merged /subversion/branches/1.6.x-r37622:r877699-877993
   R

Re: svn commit: r916286 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h mirror.c mod_dav_svn.c

[Julian Foad]

On Thu, 2010-02-25 at 20:45 +0530, Kamesh Jayachandran wrote:
> On 02/25/2010 08:29 PM, Julian Foad wrote:
> > kame...@apache.org wrote:
[...]
> >> -if (strcmp(uri.path, root_dir) == 0) {
> >> +if (uri.path)
> >> +canonicalized_uri = svn_dirent_canonicalize(uri.path, r->pool);
> >>  
> > Oops, you called "dirent_canonicalize" on a URI.  
> 
> Is there any uri canonicalize function?.

svn_uri_canonicalize() if it's a URI (in which non-URI characters must
be escaped as '%XX').

> FWIW here uri.path is the PATH portion of the URL, i.e something like 
> /svn/blah/blah

svn_relpath_canonicalize() if it's a "relpath" (see 
for definitions).

It's definitely wrong to use a "dirent" function on the path portion of
a URL.

[...]

> >> Modified: subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
> >> URL: 
> >> http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c?rev=916286&r1=916285&r2=916286&view=diff
> >> ==
> >> --- subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c (original)
> >> +++ subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c Thu Feb 25 
> >> 13:40:22 2010
> >> @@ -169,7 +169,8 @@
> >> /* NOTE: dir==NULL creates the default per-dir config */
> >> dir_conf_t *conf = apr_pcalloc(p, sizeof(*conf));
> >>
> >> -  conf->root_dir = dir;
> >> +  if (dir)
> >> +conf->root_dir = svn_dirent_canonicalize(dir, p);
> >>  
> > And is this "root_dir" meant to be a disk path or a URI?  I'm not sure,
> > myself.
> >
> 
> May be the disk path if the context is driven by  /some/path/to/repo>, though I never tried that way.
> 
> For the  configuration root_dir is '/svn'(after 
> canonicaliztion.

We need to know what type of path it is and use the correct
canonicalization function(s).  Maybe it requires two different functions
depending on ... something.

- Julian

Bug#571457: svnserve hangs, burning CPU cycles, under low TCP sendbuffer.

[Dr. Andreas Krüger]

Package: subversion
Version: 1.6.9dfsg-1
X-Debbugs-CC: dev@subversion.apache.org

Hello, Subversionists,

I have a subversion repository with a single revision, consisting of a
single roughly 10 MB file with random data.

I use svnserve to serve that repository.

I use a client to access the svnserve over the network, via "svn co".

(The client happens to be Ubuntu's 1.6.5dfsg-1ubuntu1, in case that
matters.)

The server is an openvz guest. When, on the corresponding host, I set

vzctl set NNN --tcpsndbuf 415k:715k

or lower (my guess is the first number is the one that matters), the
checkout starts, there is some initial network traffic, but then
svnadmin starts to eat up all CPU cycles it can get and no further
progress is made, until I kill the svnadmin process manually on the
server. Just killing the client does not seem to stop the waste of CPU
cycles on the server.

When I increase the tcpsndbuf above that value, there is no problem.

However, when I check in a much larger file into the repository (I do
that directly, using file:/// - access), and try to check out again
(using svnserve), the problem reappears.

This is quite reproducibly, with svnadmin as a standalone daemon as well
as svnadmin running under inetd. For the latter case, I made no precise
experiments about the --tcpsndbuf numbers.

This is the summary. I have written three mails yesterday and today to
the dev@subversion.apache.org mailing list, but have not received any
answers there yet. There is lots of nitty-gritty background information
in those mails, which, hopefully, you'll be able to find here:

http://mail-archives.apache.org/mod_mbox/subversion-dev/201002.mbox/browser

Regards, and thank you for providing fine software,

Andreas

P.S.: I ask the Debian bug tracking system to forward a copy of this
mail to the SVN dev mailing list. This contains two pieces of
information not available in my previous mails to that list:

* The bug is reproducible with the latest subversion software version I
can easily install, short of compiling myself.
* The client I'm using is, and always has been, 1.6.5dfsg-1ubuntu1. I
think I misquoted it to be 1.5 in one of my earlier mails. I apologize
if this caused any confusion!

-- 
Dr. Andreas Krüger, Berater, DV-RATIO NORDWEST GmbH
andreas.krue...@dv-ratio.com
GPG/PGP Fingerprint 8063 4A9B 362D 4220 A546  14C1 EA19 AADC FD44 5EB7

DV-RATIO NORDWEST GmbH
Tel: +49 (0)211 / 577 996-0
Fax: +49 (0)211 / 577 996-26
http://www.dv-ratio.com 
Sitz der Gesellschaft Habsburgerstraße 12, 40547 Düsseldorf
Registergericht Düsseldorf HRB 34330
USt-IdNr.:  DE811321837
Steuer-Nr.: 809/44031
Geschäftsführung: Günter Gerstmann
Prokura: Trudbert Vetter, Uwe Wolfram

DV-RATIO - "Kompetenz und Zuverlässigkeit seit 1980"




signature.asc
Description: OpenPGP digital signature

Re: svnserve under Linux inetd hangs, burning CPU cycles, under too low TCP-sendbuffer.

[Peter Samuelson]

[Dr. Andreas Krüger]
> I run svnserve under Linux under inetd, with low memory allowances
> for TCP send buffers.
> 
> Checkout on the client side causes svnserve to burn CPU cycles,
> apparently endlessly without making any progress.

He additionally reports that this is reproducible not only with 1.5.1,
but with Debian's build of 1.6.9.  (That mail is probably in the
moderation queue now.)
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Anybody going to the Apache Retreat in April?

[Hyrum K. Wright]

There's an Apache Retreat in Ireland in April:
http://apache.eventbrite.com/

I'm tempted to go, if only so I can get interaction with a large-ish number of 
Subversion users.  Is anybody else planning on attending?

-Hyrum

RE: Bug in svn merge --show-revs eligible ^/subversion/trunk ^/subversion/branches/1.6.x

[Bert Huijben]

> -Original Message-
> From: Kamesh Jayachandran [mailto:kam...@collab.net]
> Sent: donderdag 25 februari 2010 16:42
> To: dev@subversion.apache.org
> Subject: Bug in svn merge --show-revs eligible ^/subversion/trunk
> ^/subversion/branches/1.6.x
> 
> svn mergeinfo --show-revs eligible
> https://svn.apache.org/repos/asf/subversion/trunk
> https://svn.apache.org/repos/asf/subversion/branches/1.6.x
> 
> Above command lists 'r876233' while that has already been merged!

This revision was removed from the mergeinfo in r891009.

$ svn diff . -c 3891009

Property changes on: .
___
Modified: svn:mergeinfo
   Reverse-merged /subversion/branches/in-memory-cache:r869829-871452
   Reverse-merged /subversion/branches/1.6.x-r37622:r877699-877993
   Reverse-merged
/subversion/trunk:r875965,875968,876004,876012,876017,876019,876022,876024,8
76041-876042,876048,876051,876059,876083,876091,876097,876101,876109,876123-
876125,876129,876132,876138,876160,876167,876180,876185,876205,876223-876225
,876230,876233,876252,876256,876283,876287,876312,876326-876327,876330,87636
6,876372,876374,876376,876383,876386,876442,876456-876457,876462-876464,8764
67,876469,876480,876486,876495-876497,876516-876518,876524,876526,876583,876
601,876614,876628,876633,876641,876659,876687,876689,876705,876715,876726,87
6760,876763,876794,876804,876815-876816,876821,876825,876837,876840-876841,8
76843,876849,876857-876858,876873,876890,876897,876905,876908,876925,876931,
876934,876948-876949,876953,876987,876993,877011,877028-877029,877038,877119
,877127,877146,877157,877191,877195,877211,877230,877234,877237,877243,87724
9,877259,877261,877304,877319,877407,877437,877441-877442,877453,877459,8774
72,877544,877553,877565,877568,877573,877593,877601,877612,877667,877681,877
692,877696,877701,877720,877730,877784,877793,877797,877809,877815,877819,87
7821,877842,877848,877853,877867,877869,877873,877901,877909,877916,877931,8
77942,877953,877964,877968,877970,877981-877982,878005,878013,878015,878020,
878046,878053,878062,878074,878080,878089,878091,878093,878095,878127,878129
,878131,878142,878173-878176,878240,878242,878255,878272,878279,878296-87829
7,878303,878335,878338,878353,878364,878367-878368,878385,878462,878484,8784
91,878498,878532,878595,878646,878659,878673,878682-878683,878690-878691,878
693,878723,878760-878761,878873,878875,878877,878879,878905,878915,878924-87
8925,878946,878949,878955,878960,878970,878981,879001,879033,879056,879074,8
79076,879081-879082,879105,879126,879148,879170,879198-879199,879201,879271,
879293,879357,879375-879376,879403,879631,879635-879636,879709-879711,879747
,879954,879961,880082,880095,880105,880162,880226
   Reverse-merged /subversion/branches/issue-3334-dirs:r875156-875867
   Reverse-merged /subversion/branches/tree-conflicts:r869500-873154
   Reverse-merged /subversion/branches/file-externals:r871779-873302
   Reverse-merged /subversion/branches/1.6.x-r38572:r878661-878666
   Reverse-merged /subversion/branches/1.6.x-r36178:r877876-877884
   Reverse-merged /subversion/branches/1.6.x-r37627:r877704-877928
   Reverse-merged
/subversion/branches/merge-skips-obstructions:r874525-874615
   Reverse-merged
/subversion/branches/dont-save-plaintext-passwords-by-default:r870728-871118
   Reverse-merged
/subversion/branches/issue-3067-deleted-subtrees:r873375-874084
   Reverse-merged /subversion/branches/tc-resolve:r874191-874239
   Reverse-merged /subversion/branches/1.6.x-r39887:r880024-880066
   Reverse-merged /subversion/branches/1.6.x-r37988:r878063-878897
   Reverse-merged /subversion/branches/1.6.x-r38799:r878880-878917
   Reverse-merged /subversion/branches/svnserve-logging:r869828-870893
   Reverse-merged
/subversion/branches/reintegrate-improvements:r873853-874164
   Reverse-merged /subversion/branches/issue-2843-dev:r871432-874179
   Reverse-merged /subversion/branches/1.5.x-r30215:r870312
   Reverse-merged /subversion/branches/log-g-performance:r870941-871032
   Reverse-merged /subversion/branches/1.6.x-r38000:r879083-879767
   Reverse-merged /subversion/branches/diff-callbacks3:r870059-870761
   Reverse-merged /subversion/branches/1.6.x-r38222:r878299-878891
   Reverse-merged /subversion/branches/bdb-reverse-deltas:r872050-872529
   Reverse-merged /subversion/branches/double-delete:r870511-872970
   Reverse-merged /subversion/branches/gnome-keyring:r870558-871410
   Reverse-merged /subversion/branches/fsfs-pack:r873717-874575
   Reverse-merged /subversion/branches/1.6.x-issue3443:r878597-879480
   Reverse-merged /subversion/branches/1.6.x-r37953:r879809-880071
   Reverse-merged
/subversion/branches/svn-mergeinfo-enhancements:r870119-870195,870197-870288
   Reverse-merged /subversion/branches/fs-rep-sharing:r870149-873803
   Reverse-merged /subversion/branches/1.6.x-r39557:r879668-879907
   Reverse-merged /subversion/branches/1.6.x-r38927:r879002-879176
   Reverse-merged /subversion/branches/tc-issue-3334:r874697-874773
   R

Bug in svn merge --show-revs eligible ^/subversion/trunk ^/subversion/branches/1.6.x

[Kamesh Jayachandran]

svn mergeinfo --show-revs eligible 
https://svn.apache.org/repos/asf/subversion/trunk 
https://svn.apache.org/repos/asf/subversion/branches/1.6.x


Above command lists 'r876233' while that has already been merged!

With regards
Kamesh Jayachandran

Re: svn commit: r916286 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h mirror.c mod_dav_svn.c

[Julian Foad]

kame...@apache.org wrote:
> Author: kameshj
> Date: Thu Feb 25 13:40:22 2010
> New Revision: 916286

Hi Kamesh. Some review comments below ...

> URL: http://svn.apache.org/viewvc?rev=916286&view=rev
> Log:
> With the below apache configuration(See the trailing slash at the end of 
> '/svn/').
> 
> 
>   DAV svn
>   SVNParentPath /repositories
> #See the trailing slash on the master URI also can cause the confusion.
>   SVNMasterURI http://master/svn/ 
>   SVNAdvertiseV2Protocol Off
> 
> 
> 
> We get the following error on the client side.
> 
> svn: Commit failed (details follow):
> svn: MKACTIVITY of 
> '/svn/demujin/!svn/act/4b6d547c-018d-4e02-9d3f-2b283076cc06': Could not read 
> status line: connection was closed by server (http://localhost)
> 
> 
> On the server(proxy) it is an assertion error on the following code block 
> from subversion/mod_dav_svn/mirror.c:proxy_request_fixup
> 
>assert((uri_segment[0] == '\0')
>|| (uri_segment[0] == '/'));
> 
> For the above configuration we get the uri_segment with the value 
> 'reponame/some/path/inside/the/repo'.
> 
> We fix this by canonicalizing the 'root_dir'(The one in Location) and 
> 'uri.path'(Path portion of Master URI).
> * subversion/mod_dav_svn/dav_svn.h
> (dav_svn__get_root_dir): Document that root_dir is in canonicalized form.
> * subversion/mod_dav_svn/mod_dav_svn.c
> (create_dir_config): Canonicalize the root_dir.
> * subversion/mod_dav_svn/mirror.c
> (dav_svn__location_in_filter, dav_svn__location_body_filter):
> As root_dir is in canonical form canonicalize the uri.path too to avoid
> spurious errors.
> (dav_svn__location_header_filter): As root_dir is canonical we need to
>  explicitly introduce the path seperator.
> 
> Suggested by: julianfoad
> 
> Modified:
> subversion/trunk/subversion/mod_dav_svn/dav_svn.h
> subversion/trunk/subversion/mod_dav_svn/mirror.c
> subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
> 
> Modified: subversion/trunk/subversion/mod_dav_svn/dav_svn.h
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/dav_svn.h?rev=916286&r1=916285&r2=916286&view=diff
> ==
> --- subversion/trunk/subversion/mod_dav_svn/dav_svn.h (original)
> +++ subversion/trunk/subversion/mod_dav_svn/dav_svn.h Thu Feb 25 13:40:22 2010
> @@ -352,7 +352,7 @@
>  /* Return the activities db */
>  const char * dav_svn__get_activities_db(request_rec *r);
>  
> -/* Return the root directory */
> +/* Return the root directory in canonicalized form */
>  const char * dav_svn__get_root_dir(request_rec *r);
>  
> 
> 
> Modified: subversion/trunk/subversion/mod_dav_svn/mirror.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/mirror.c?rev=916286&r1=916285&r2=916286&view=diff
> ==
> --- subversion/trunk/subversion/mod_dav_svn/mirror.c (original)
> +++ subversion/trunk/subversion/mod_dav_svn/mirror.c Thu Feb 25 13:40:22 2010
> @@ -128,7 +128,7 @@
>  locate_ctx_t *ctx = f->ctx;
>  apr_status_t rv;
>  apr_bucket *bkt;
> -const char *master_uri, *root_dir;
> +const char *master_uri, *root_dir, *canonicalized_uri;
>  apr_uri_t uri;
>  
>  /* Don't filter if we're in a subrequest or we aren't setup to
> @@ -143,7 +143,11 @@
> (that is, if our root path matches that of the master server). */
>  apr_uri_parse(r->pool, master_uri, &uri);
>  root_dir = dav_svn__get_root_dir(r);
> -if (strcmp(uri.path, root_dir) == 0) {
> +if (uri.path)
> +canonicalized_uri = svn_dirent_canonicalize(uri.path, r->pool);

Oops, you called "dirent_canonicalize" on a URI.

> +else
> +canonicalized_uri = uri.path;
> +if (strcmp(canonicalized_uri, root_dir) == 0) {
>  ap_remove_input_filter(f);
>  return ap_get_brigade(f->next, bb, mode, block, readbytes);
>  }
> @@ -156,7 +160,7 @@
>  
>  if (!f->ctx) {
>  ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
> -ctx->remotepath = uri.path;
> +ctx->remotepath = canonicalized_uri;
>  ctx->remotepath_len = strlen(ctx->remotepath);
>  ctx->localpath = root_dir;
>  ctx->localpath_len = strlen(ctx->localpath);
> @@ -226,7 +230,7 @@
>  start_foo += strlen(master_uri);
>  new_uri = ap_construct_url(r->pool,
> apr_pstrcat(r->pool,
> -   dav_svn__get_root_dir(r),
> +   dav_svn__get_root_dir(r), "/",
> start_foo, NULL),
> r);
>  apr_table_set(r->headers_out, "Location", new_uri);
> @@ -240,7 +244,7 @@
>  request_rec *r = f->r;
>  locate_ctx_t *ctx = f->ctx;
>  apr_bucket *bkt;
> -const char *master_uri, *root_dir;
> +const char *mas

Re: [PATCH] Fix the commit failure over the write-through-proxy if apache is configured as ''

[Kamesh Jayachandran]

On 02/15/2010 07:44 PM, Julian Foad wrote:

Kamesh Jayachandran wrote:
   

With the below apache configuration(See the trailing slash at the end of
'/svn/').


DAV svn
SVNParentPath /repositories
SVNMasterURI http://master/svn/
SVNAdvertiseV2Protocol Off

 

[...]
   

Attached patch fixes it.

This change came via fix for issue 3275. I believe this assertion is
just to clean the uri for double slash and not anything functional to
that issue, I may be wrong.

Thoughts?
 

The logic in your patch looks correct IF we accept that
dav_svn__get_root_dir() can return a non-canonical path. But if that's
the case, then we should document it as such:

[[[
Index: subversion/mod_dav_svn/dav_svn.h
===
--- subversion/mod_dav_svn/dav_svn.h(revision 910187)
+++ subversion/mod_dav_svn/dav_svn.h(working copy)
@@ -352,7 +352,7 @@
  /* Return the activities db */
  const char * dav_svn__get_activities_db(request_rec *r);

-/* Return the root directory */
+/* Return the root directory (not necessarily as a canonical path) */
  const char * dav_svn__get_root_dir(request_rec *r);


]]]

or if it's meant to be canonical, then we should fix that function
instead.

- Julian
   


Changed 'dav_svn__get_root_dir' to return canonical path.


Committed this in r916286.

With regards
Kamesh Jayachandran

Re: bug report: ra_serf gets PROPFIND failure on certain non-ASCII paths

[Julian Foad]

Lieven Govaerts wrote:
> [[[
> ra_serf: Fix support for international characters in paths.
> 
> Found by: cmpilato
> 
> * subversion/libsvn_ra_serf/property.c
>   (end_propfind): Replace call to svn_uri_canonicalize with a call to
>svn_ra_serf__uri_to_internal.
> * subversion/libsvn_ra_serf/merge.c
>   (end_merge): Here too.
> * subversion/libsvn_ra_serf/ra_serf.h
>   (svn_ra_serf__uri_to_internal): New function declaration.
> * subversion/libsvn_ra_serf/util.c
>   (svn_ra_serf__uri_to_internal): New function definition.
> ]]]


> [[[ 
> Index: subversion/libsvn_ra_serf/merge.c
> ===
> --- subversion/libsvn_ra_serf/merge.c   (revision 911289)
> +++ subversion/libsvn_ra_serf/merge.c   (working copy)
> @@ -364,7 +364,7 @@
>info->prop_val = apr_pstrmemdup(info->pool, info->prop_val,
>info->prop_val_len);
>if (strcmp(info->prop_name, "href") == 0)
> -info->prop_val = svn_uri_canonicalize(info->prop_val,
> info->pool);
> +info->prop_val = svn_ra_serf__uri_to_internal(info->prop_val,
> info->pool);

As I mentioned on IRC, you canonicalize here when the URI is being put
in to the hash. But when it gets read out from the hash (about 70 lines
higher up) you also need to canonicalize the URL that it is compared
with.  Here:

[line 299]
href = apr_hash_get(info->props, "href", APR_HASH_KEY_STRING);
if (! svn_uri_is_ancestor(ctx->merge_url, href))

... ctx->merge_url comes from session->repos_url, and that comes
from ... somewhere higher up.

You could canonicalize it right here, but really we need to fix the
global svn_uri_canonicalize() to do this kind of canonicalization
throughout Subversion, not just in ra_serf. Currently,
svn_uri_canonicalize() simply isn't producing a canonical URI.

- Julian


>/* Set our property. */
>apr_hash_set(info->props, info->prop_name, APR_HASH_KEY_STRING,
> Index: subversion/libsvn_ra_serf/util.c
> ===
> --- subversion/libsvn_ra_serf/util.c(revision 911289)
> +++ subversion/libsvn_ra_serf/util.c(working copy)
> @@ -1772,3 +1772,20 @@
>  
>return SVN_NO_ERROR;
>  }
> +
> +const char *
> +svn_ra_serf__uri_to_internal(const char *uri_in, apr_pool_t *pool)
> +{
> +  const char *target;
> +
> +  /* Convert to URI, unescaping all internatonal characters. */
> +  target = svn_path_uri_decode(uri_in, pool);
> +
> +  /* Now escape the international characters again. */
> +  target = svn_path_uri_from_iri(target, pool);
> +
> +  /* Strip any trailing '/' and collapse other redundant elements. */
> +  target = svn_uri_canonicalize(target, pool);
> +
> +  return target;
> +}
> Index: subversion/libsvn_ra_serf/property.c
> ===
> --- subversion/libsvn_ra_serf/property.c(revision 911289)
> +++ subversion/libsvn_ra_serf/property.c(working copy)
> @@ -29,6 +29,7 @@
>  #include "svn_xml.h"
>  #include "svn_props.h"
>  #include "svn_dirent_uri.h"
> +#include "svn_path.h"
>  
>  #include "private/svn_dav_protocol.h"
>  #include "svn_private_config.h"
> @@ -332,7 +333,7 @@
>  {
>if (strcmp(ctx->depth, "1") == 0)
>  {
> -  ctx->current_path = svn_uri_canonicalize(info->val,
> ctx->pool);
> +  ctx->current_path =
> svn_ra_serf__uri_to_internal(info->val, ctx->pool);
>  }
>else
>  {
> Index: subversion/libsvn_ra_serf/ra_serf.h
> ===
> --- subversion/libsvn_ra_serf/ra_serf.h (revision 911289)
> +++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
> @@ -1521,6 +1521,13 @@
>  svn_error_t *
>  svn_ra_serf__error_on_status(int status_code, const char *path);
>  
> +/**
> + * Handle an external uri so that it can be compared with other
> uri's.
> + * (canonicalize + re-encode international characters).
> + */
> +const char *
> +svn_ra_serf__uri_to_internal(const char *uri_in, apr_pool_t *pool);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> ]]]
> 

Re: svnserve under Linux inetd hangs, burning CPU cycles, under too low TCP-sendbuffer.

[Dr. Andreas Krüger]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hello, all,

in short:

* The bug is reproducible with svnserve 1.6.
* The bigger the files get, the more TCP sendbuffer svnserve seems to
need.

Minute detail:

Installing

> Package: subversion Version: 1.6.4dfsg-1~bpo50+1

and

> Package: libsvn1 Version: 1.6.4dfsg-1~bpo50+1

from Debian Lenny backports.

Creating a repository /var/local/svn-bug-repository, by doing (in that
directory, owned by the svn user, as the svn user)

> svnadmin create .

In /tmp , I generate a 10 MB junk file and check it in:

> svn co file:///var/local/svn-bug-repository ws cd ws perl -we
> 'srand(42); for($i=0;$i<1000;$i++) {print pack
C,int(rand(255))}' > random-binary-file.bin
> svn ci -m 'Just some arbitrary content.' .

This creates the first revision in that particular repository.

I also added user and password to
/var/local/svn-bug-repository/conf/password, and started svnserve, via

> svnserve --daemon --listen-port  --foreground --root
/var/local/svn-bug-repository

Thus far, all commands on the server machine, with the svn user's id.

Now, on the home machine (which still has an svn 1.5 client):

> svn co --username ... --password ... svn://server:/ ws

And, again, on the server, the svnserve hangs and consumes CPU.

I tried to find the limit of what would and wouldn't work.

vzctl set NNN --tcpsndbuf 600k:900k  # works
vzctl set NNN --tcpsndbuf 400k:700k  # hangs
vzctl set NNN --tcpsndbuf 500k:800k  # works
vzctl set NNN --tcpsndbuf 450k:750k  # hangs
vzctl set NNN --tcpsndbuf 475k:775k  # hangs
vzctl set NNN --tcpsndbuf 485k:785k  # works
vzctl set NNN --tcpsndbuf 480k:780k  # hangs
vzctl set NNN --tcpsndbuf 482k:782k  # hangs
vzctl set NNN --tcpsndbuf 485k:785k  # works (just checking)
vzctl set NNN --tcpsndbuf 483k:783k  # hangs
vzctl set NNN --tcpsndbuf 484k:784k  # hangs
vzctl set NNN --tcpsndbuf 485k:785k  # works (just checking one more time)

On the particular machine, the CPU load of svnserve tends to stay
around 20% as long as all goes well. This happens to be an openvpn
network connection. I see quite a bit of openvpn load. When things
turn sour, the CPU load of svnserve rises to 99% and that of openvpn
drops to invisibility. So this does not look like outgoing network
traffic.

Does the limit depend on the material?

I produce, and check in (locally on the server machine, with file:///
- - repository URL), the 10x bigger random file generated by

perl -we 'srand(42); for($i=0;$i<1;$i++) {print pack
C,int(rand(255))}' > even-bigger-random-binary-file.bin

Now it hangs with 485k:785k, and also with 500k:800k, and also with
600k:900k, and also with 900k:1200k, at which point I give up.

Apparently, bigger files do need bigger TCP send buffers.

Regards, and thank you for providing fine software,

Andreas
- --
Dr. Andreas Krüger, Berater, DV-RATIO NORDWEST GmbH
andreas.krue...@dv-ratio.com
GPG/PGP Fingerprint 8063 4A9B 362D 4220 A546  14C1 EA19 AADC FD44 5EB7

DV-RATIO NORDWEST GmbH
Tel: +49 (0)211 / 577 996-0
Fax: +49 (0)211 / 577 996-26
http://www.dv-ratio.com 
Sitz der Gesellschaft Habsburgerstraße 12, 40547 Düsseldorf
Registergericht Düsseldorf HRB 34330
USt-IdNr.:  DE811321837
Steuer-Nr.: 809/44031
Geschäftsführung: Günter Gerstmann
Prokura: Trudbert Vetter, Uwe Wolfram

DV-RATIO - "Kompetenz und Zuverlässigkeit seit 1980"
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkuGZdoACgkQ6hmq3P1EXreM9ACfedlS7PuGukQMZCkcWK53+6OQ
nQ8AnRzNWMfOVfyZUkVLgFfAuV46+XjI
=DE9K
-END PGP SIGNATURE-

Re: bug report: ra_serf gets PROPFIND failure on certain non-ASCII paths

[Lieven Govaerts]

On Thu, Feb 25, 2010 at 12:11 AM, Bert Huijben  wrote:
>
>
>> -Original Message-
>> From: lieven.govae...@gmail.com [mailto:lieven.govae...@gmail.com] On
>> Behalf Of Lieven Govaerts
>> Sent: woensdag 24 februari 2010 23:39
>> To: C. Michael Pilato
>> Cc: Subversion Development
>> Subject: Re: bug report: ra_serf gets PROPFIND failure on certain non-
>> ASCII paths
>
>
>> These paths are canonicalized before being used, but
>> svn_uri_canonicalize doesn't touch the encoded characters. Maybe it
>> should just convert those letters to lowercase? I don't really see a
>> better fix.
>
> svn_uri_canonicalize is still used for more than urls/uris, so it doesn't 
> handle any encoding itself.
>
> But besides that, I also think this is not the right way to fix this. For 
> several characters in uris it is optional if they are escaped or not. We need 
> a better fix than just fixing the casing of the escaped characters...
>
> Unescaping the paths would be an option that resolves it in the generic case, 
> or unescape followed by a specific standard escaping. (This last method is 
> used in some class libraries to avoid similar issues, but to provide a useful 
> uri anyway)

Attached patch fixes the problem. Better suggestions are welcome, I'll
commit tonight.

Lieven

[[[
ra_serf: Fix support for international characters in paths.

Found by: cmpilato

* subversion/libsvn_ra_serf/property.c
  (end_propfind): Replace call to svn_uri_canonicalize with a call to
   svn_ra_serf__uri_to_internal.
* subversion/libsvn_ra_serf/merge.c
  (end_merge): Here too.
* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__uri_to_internal): New function declaration.
* subversion/libsvn_ra_serf/util.c
  (svn_ra_serf__uri_to_internal): New function definition.
]]]
Index: subversion/libsvn_ra_serf/merge.c
===
--- subversion/libsvn_ra_serf/merge.c   (revision 911289)
+++ subversion/libsvn_ra_serf/merge.c   (working copy)
@@ -364,7 +364,7 @@
   info->prop_val = apr_pstrmemdup(info->pool, info->prop_val,
   info->prop_val_len);
   if (strcmp(info->prop_name, "href") == 0)
-info->prop_val = svn_uri_canonicalize(info->prop_val, info->pool);
+info->prop_val = svn_ra_serf__uri_to_internal(info->prop_val, 
info->pool);
 
   /* Set our property. */
   apr_hash_set(info->props, info->prop_name, APR_HASH_KEY_STRING,
Index: subversion/libsvn_ra_serf/util.c
===
--- subversion/libsvn_ra_serf/util.c(revision 911289)
+++ subversion/libsvn_ra_serf/util.c(working copy)
@@ -1772,3 +1772,20 @@
 
   return SVN_NO_ERROR;
 }
+
+const char *
+svn_ra_serf__uri_to_internal(const char *uri_in, apr_pool_t *pool)
+{
+  const char *target;
+
+  /* Convert to URI, unescaping all internatonal characters. */
+  target = svn_path_uri_decode(uri_in, pool);
+
+  /* Now escape the international characters again. */
+  target = svn_path_uri_from_iri(target, pool);
+
+  /* Strip any trailing '/' and collapse other redundant elements. */
+  target = svn_uri_canonicalize(target, pool);
+
+  return target;
+}
Index: subversion/libsvn_ra_serf/property.c
===
--- subversion/libsvn_ra_serf/property.c(revision 911289)
+++ subversion/libsvn_ra_serf/property.c(working copy)
@@ -29,6 +29,7 @@
 #include "svn_xml.h"
 #include "svn_props.h"
 #include "svn_dirent_uri.h"
+#include "svn_path.h"
 
 #include "private/svn_dav_protocol.h"
 #include "svn_private_config.h"
@@ -332,7 +333,7 @@
 {
   if (strcmp(ctx->depth, "1") == 0)
 {
-  ctx->current_path = svn_uri_canonicalize(info->val, ctx->pool);
+  ctx->current_path = svn_ra_serf__uri_to_internal(info->val, 
ctx->pool);
 }
   else
 {
Index: subversion/libsvn_ra_serf/ra_serf.h
===
--- subversion/libsvn_ra_serf/ra_serf.h (revision 911289)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -1521,6 +1521,13 @@
 svn_error_t *
 svn_ra_serf__error_on_status(int status_code, const char *path);
 
+/**
+ * Handle an external uri so that it can be compared with other uri's.
+ * (canonicalize + re-encode international characters).
+ */
+const char *
+svn_ra_serf__uri_to_internal(const char *uri_in, apr_pool_t *pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */