You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2008/04/01 17:37:54 UTC

Re: 1.5 Neon bug in copy-on-update

Does the naive tweak help?

Index: subversion/libsvn_ra_neon/fetch.c
===================================================================
--- subversion/libsvn_ra_neon/fetch.c	(revision 30139)
+++ subversion/libsvn_ra_neon/fetch.c	(working copy)
@@ -1307,6 +1307,7 @@
            || child == ELEM_add_directory
            || child == ELEM_absent_file
            || child == ELEM_add_file
+          || child == ELEM_remove_prop
            || child == ELEM_set_prop
            || child == ELEM_SVN_prop
            || child == ELEM_checked_in)
@@ -1330,6 +1331,7 @@
        if (child == ELEM_checked_in
            || child == ELEM_txdelta
            || child == ELEM_set_prop
+          || child == ELEM_remove_prop
            || child == ELEM_SVN_prop)
          return child;
        else

David Glasser wrote:
> Here's today's episode of "Subversion WebDAV RA tries to be too smart
> for its own good instead of just serializing RA API calls like
> ra_svn"!
> 
> Before 1.5, server never sent add-with-history to clients, so it would
> never make sense for a "remove prop" XML element to be inside an "add
> file" element.
> 
> And in fact, libsvn_ra_neon/fetch.c(validate_element) tries to
> validate this, throwing a corrupted XML error if (among many other
> things) an ELEM_remove_prop is inside an ELEM_add_file.  This breaks
> the update.  (Similar issues presumably include remove-prop inside
> add-directory, and delete-entry inside add-directory, but perhaps many
> more combinations as well.)
> 
> To reproduce, do something like:
> 
>  $ svn ps foo bar a
>  $ svn ci
>  Revision 10.
>  $ svn cp a b
>  $ svn pd foo b
>  $ svn ci
>  Revision 11.
>  $ svn up -r10
>  $ svn up -r11
> 
> I have no idea if serf has this bug; I've just verified that ra_svn
> and ra_local don't.
> 
> I don't really know enough about Neon to feel comfortable trying to
> fix this, but it's a serious 1.5-blocker of a bug.
> 
> --dave
> 


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


Re: 1.5 Neon bug in copy-on-update

Posted by "C. Michael Pilato" <cm...@collab.net>.
Any chance you'd be willing to bang out an update_tests.py addition that 
tests the cases that concern you?

David Glasser wrote:
> I'm guessing it would, though as I mentioned there's probably more
> necessary (eg, for delete_entry on added directories).  I'm not
> familiar with the details of the DAV report in question.
> 
> --dave
> 
> On Tue, Apr 1, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> Does the naive tweak help?
>>
>>  Index: subversion/libsvn_ra_neon/fetch.c
>>  ===================================================================
>>  --- subversion/libsvn_ra_neon/fetch.c   (revision 30139)
>>  +++ subversion/libsvn_ra_neon/fetch.c   (working copy)
>>  @@ -1307,6 +1307,7 @@
>>             || child == ELEM_add_directory
>>             || child == ELEM_absent_file
>>             || child == ELEM_add_file
>>  +          || child == ELEM_remove_prop
>>             || child == ELEM_set_prop
>>             || child == ELEM_SVN_prop
>>             || child == ELEM_checked_in)
>>  @@ -1330,6 +1331,7 @@
>>         if (child == ELEM_checked_in
>>             || child == ELEM_txdelta
>>             || child == ELEM_set_prop
>>  +          || child == ELEM_remove_prop
>>             || child == ELEM_SVN_prop)
>>           return child;
>>         else
>>
>>
>>
>>  David Glasser wrote:
>>  > Here's today's episode of "Subversion WebDAV RA tries to be too smart
>>  > for its own good instead of just serializing RA API calls like
>>  > ra_svn"!
>>  >
>>  > Before 1.5, server never sent add-with-history to clients, so it would
>>  > never make sense for a "remove prop" XML element to be inside an "add
>>  > file" element.
>>  >
>>  > And in fact, libsvn_ra_neon/fetch.c(validate_element) tries to
>>  > validate this, throwing a corrupted XML error if (among many other
>>  > things) an ELEM_remove_prop is inside an ELEM_add_file.  This breaks
>>  > the update.  (Similar issues presumably include remove-prop inside
>>  > add-directory, and delete-entry inside add-directory, but perhaps many
>>  > more combinations as well.)
>>  >
>>  > To reproduce, do something like:
>>  >
>>  >  $ svn ps foo bar a
>>  >  $ svn ci
>>  >  Revision 10.
>>  >  $ svn cp a b
>>  >  $ svn pd foo b
>>  >  $ svn ci
>>  >  Revision 11.
>>  >  $ svn up -r10
>>  >  $ svn up -r11
>>  >
>>  > I have no idea if serf has this bug; I've just verified that ra_svn
>>  > and ra_local don't.
>>  >
>>  > I don't really know enough about Neon to feel comfortable trying to
>>  > fix this, but it's a serious 1.5-blocker of a bug.
>>  >
>>  > --dave
>>  >
>>
>>
>>  --
>>  C. Michael Pilato <cm...@collab.net>
>>  CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>>
>>
> 
> 
> 


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


Re: 1.5 Neon bug in copy-on-update

Posted by Eric Gillespie <ep...@pretzelnet.org>.
BTW, if someone is getting motivated to fix neon deficiencies as
compared to ra-local/svn, have a look at these, too:

certain property names cause non-wf XML responses
  http://subversion.tigris.org/issues/show_bug.cgi?id=1971
"svn mkdir URL" gives poor error message when directory exists
  http://subversion.tigris.org/issues/show_bug.cgi?id=2295
Can't replace branch with neon/serf
  http://subversion.tigris.org/issues/show_bug.cgi?id=2939
mod-dav-svn runs pre-revprop-change twice
  http://subversion.tigris.org/issues/show_bug.cgi?id=3085
mod-dav-svn ignores pre-revprop-change failure on revprop delete
  http://subversion.tigris.org/issues/show_bug.cgi?id=3086

Eric Gillespie <ep...@pretzelnet.org> writes:

> > On Tue, Apr 1, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
> > > Does the naive tweak help?
> > >
> > >  Index: subversion/libsvn_ra_neon/fetch.c
> > >  ===================================================================
> > >  --- subversion/libsvn_ra_neon/fetch.c   (revision 30139)
> > >  +++ subversion/libsvn_ra_neon/fetch.c   (working copy)
> > >  @@ -1307,6 +1307,7 @@
> > >             || child == ELEM_add_directory
> > >             || child == ELEM_absent_file
> > >             || child == ELEM_add_file
> > >  +          || child == ELEM_remove_prop
> 
> What would really help is including some real information in the
> "invalid xml" error messages.  In this case, something like
> "unexpected %s element in report response" maybe?  There's no
> avoiding a user-hostile message in this class of failure, but at
> least we can help the developer the question eventually gets to...

-- 
Eric Gillespie <*> epg@pretzelnet.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5 Neon bug in copy-on-update

Posted by Eric Gillespie <ep...@pretzelnet.org>.
> On Tue, Apr 1, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
> > Does the naive tweak help?
> >
> >  Index: subversion/libsvn_ra_neon/fetch.c
> >  ===================================================================
> >  --- subversion/libsvn_ra_neon/fetch.c   (revision 30139)
> >  +++ subversion/libsvn_ra_neon/fetch.c   (working copy)
> >  @@ -1307,6 +1307,7 @@
> >             || child == ELEM_add_directory
> >             || child == ELEM_absent_file
> >             || child == ELEM_add_file
> >  +          || child == ELEM_remove_prop

What would really help is including some real information in the
"invalid xml" error messages.  In this case, something like
"unexpected %s element in report response" maybe?  There's no
avoiding a user-hostile message in this class of failure, but at
least we can help the developer the question eventually gets to...

-- 
Eric Gillespie <*> epg@pretzelnet.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: 1.5 Neon bug in copy-on-update

Posted by David Glasser <gl...@davidglasser.net>.
I'm guessing it would, though as I mentioned there's probably more
necessary (eg, for delete_entry on added directories).  I'm not
familiar with the details of the DAV report in question.

--dave

On Tue, Apr 1, 2008 at 10:37 AM, C. Michael Pilato <cm...@collab.net> wrote:
> Does the naive tweak help?
>
>  Index: subversion/libsvn_ra_neon/fetch.c
>  ===================================================================
>  --- subversion/libsvn_ra_neon/fetch.c   (revision 30139)
>  +++ subversion/libsvn_ra_neon/fetch.c   (working copy)
>  @@ -1307,6 +1307,7 @@
>             || child == ELEM_add_directory
>             || child == ELEM_absent_file
>             || child == ELEM_add_file
>  +          || child == ELEM_remove_prop
>             || child == ELEM_set_prop
>             || child == ELEM_SVN_prop
>             || child == ELEM_checked_in)
>  @@ -1330,6 +1331,7 @@
>         if (child == ELEM_checked_in
>             || child == ELEM_txdelta
>             || child == ELEM_set_prop
>  +          || child == ELEM_remove_prop
>             || child == ELEM_SVN_prop)
>           return child;
>         else
>
>
>
>  David Glasser wrote:
>  > Here's today's episode of "Subversion WebDAV RA tries to be too smart
>  > for its own good instead of just serializing RA API calls like
>  > ra_svn"!
>  >
>  > Before 1.5, server never sent add-with-history to clients, so it would
>  > never make sense for a "remove prop" XML element to be inside an "add
>  > file" element.
>  >
>  > And in fact, libsvn_ra_neon/fetch.c(validate_element) tries to
>  > validate this, throwing a corrupted XML error if (among many other
>  > things) an ELEM_remove_prop is inside an ELEM_add_file.  This breaks
>  > the update.  (Similar issues presumably include remove-prop inside
>  > add-directory, and delete-entry inside add-directory, but perhaps many
>  > more combinations as well.)
>  >
>  > To reproduce, do something like:
>  >
>  >  $ svn ps foo bar a
>  >  $ svn ci
>  >  Revision 10.
>  >  $ svn cp a b
>  >  $ svn pd foo b
>  >  $ svn ci
>  >  Revision 11.
>  >  $ svn up -r10
>  >  $ svn up -r11
>  >
>  > I have no idea if serf has this bug; I've just verified that ra_svn
>  > and ra_local don't.
>  >
>  > I don't really know enough about Neon to feel comfortable trying to
>  > fix this, but it's a serious 1.5-blocker of a bug.
>  >
>  > --dave
>  >
>
>
>  --
>  C. Michael Pilato <cm...@collab.net>
>  CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>
>



-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org