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

RE: 1.5.5 next Wednesday (12/17)

From: Bert Huijben <bert_at_vmoo.com>
Date: Thu, 11 Dec 2008 01:33:11 +0100

> -----Original Message-----
> From: Mark Phippard [mailto:markphip_at_gmail.com]
> Sent: Thursday, December 11, 2008 12:13 AM
> To: kmradke_at_rockwellcollins.com
> Cc: dev_at_subversion.tigris.org
> Subject: Re: 1.5.5 next Wednesday (12/17)
>
> On Wed, Dec 10, 2008 at 6:08 PM, <kmradke_at_rockwellcollins.com> wrote:
> >
> > Anyone know if the 1.5 regression that causes a:
> >
> > svn cp WC URL
> >
> > operation to fail with a "file already exists" error
> > when performed over http when WC has modified files has
> > been fixed? I'm not seeing any closure to this thread:
> >
> > http://svn.haxx.se/users/archive-2008-10/0829.shtml
> >
> > I didn't see an issue being filed or anything in 1.5.x STATUS,
> > but I have run into this particular problem recently and it
> > worked in 1.4 (and possibly as late as 1.5.1)
>
> AFAIK, even 1.6 is in danger of shipping with that bug still in place.
> I have not seen anyone attempt to fix it.

For other developers looking at this issue:

r31692 (which was merged to 1.5.x in r31799) is the only change between
1.5.0 and 1.5.1 in neon that can explain the regression starting in 1.5.1.
It touches precisely the code Karl quotes in his mail.

Modified: trunk/subversion/libsvn_ra_neon/commit.c
URL:
http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_neon/commit.c?pa
threv=31692&r1=31691&r2=31692
============================================================================
==
--- trunk/subversion/libsvn_ra_neon/commit.c Tue Jun 10 12:59:16 2008
(r31691)
+++ trunk/subversion/libsvn_ra_neon/commit.c Tue Jun 10 13:20:28 2008
(r31692)
@@ -1036,8 +1036,8 @@ static svn_error_t * commit_add_file(con
      svn_ra_neon__resource_t *res;
      svn_error_t *err = svn_ra_neon__get_starting_props(&res,
                                                         file->cc->ras,
- file->rsrc->url,
NULL,
- workpool);
+
file->rsrc->wr_url,
+ NULL, workpool);
      if (!err)
        {
          /* If the PROPFIND succeeds the file already exists */

=====================================================================
[[
Fix issue #2939 over ra_neon: bogus existence check.

See reproduction recipe in the issue to verify this fix -- it uses
svnmucc. Scenario can't be tested using the normal 'svn' client,
since it involves doing a directory replacement within a single
transaction. svnmucc and gvn can create the scenario, however,
resulting is a revision which (until now) couldn't be svnsync'd over
http!

* subversion/libsvn_ra_neon/commit.c (commit_add_file):
    do PROPFIND existence check on the *working* url (i.e. the path
    within the transaction), not the public url (which the path in HEAD).
]]

[[
Merge r31692, 31695 from trunk:

 * r31692, r31695
   Fix issue #2939 over ra_neon/serf: bogus existence check.
   Votes:
     +1: epg, lgo, Arfrever
]]

I would guess that reverting this change will bring us back to the old
behavior, but it will break issue #2939.

Karl's diagnosis continues in mod_dav_svn, but the comment above this block
of code says:

  /* If the parent directory existed before this commit then there may be a
     file with this URL already. We need to ensure such a file does not
     exist, which we do by attempting a PROPFIND. Of course, a
     PROPFIND *should* succeed if this "add" is actually the second
     half of a "replace".

[....]

Looking at this neon change, it looks like it starts checking whether the
file exists in the transaction and stops checking whether it exists before
the commit. (I'm trying to understand what the consequences are, but the old
behavior matches what I would expect reading the comments around this code).

The serf change for the same issue is completely different than the neon
version. (I don't think r31695 needs further investigation)

        Bert
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageI
> d=982600

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=982623
Received on 2008-12-11 01:33:33 CET

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