You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Justin Erenkrantz <ju...@erenkrantz.com> on 2006/03/07 01:46:43 UTC

[PATCH] libsvn_wc losing properties?

A weird corner case that I seem to be hitting with ra_serf is that
when I do an update any directory props (like svn:ignore) get lost
each time.  ra_serf doesn't have any ability to know what properties
have changed from one revision to the next (mainly because the server
can't tell us unless we were to change mod_dav_svn - which would break
compat) - so ra_serf calls change_file_prop or change_dir_prop with
the 'current' list and lets libsvn_wc sort the property situation out.
 Which it does for files, but for dirs libsvn_wc goes and deletes all
properties for a directory instead.  Oops.

I've confirmed that libsvn_wc is indeed issuing the rm command from
props.c:416 which produces in the WC log:

<rm
   name=".svn/dir-props"/>

The below patch fixes it for me, but am I missing something?

Also, cosmetically, because libsvn_wc is doing the reduction itself
(instead of being able to do it at the RA layer), svn always reports
that properties are changed even when they aren't.  Ideally, we could
make libsvn_wc smarter when it doesn't have any core propchanges (the
base props always change - i.e. the checked-in href wcprop) - such as
the checks around props.c:568.  (So, before ra_dav has been lying to
everyone as it silently changes the properties of the directories...)

In case you're interested, the reproduction case I'm using here is
'svn update' from 18738 to HEAD (18747 now) using ra_serf from trunk -
after the update, we lose all of the dir-props in any touched
directory (i.e. svn:ignore in svn/www).

Thanks.  -- justin

* subversion/libsvn_wc/props.c
  (svn_wc__install_props): Only delete our props if we now have no items.

Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c        (revision 18738)
+++ subversion/libsvn_wc/props.c        (working copy)
@@ -410,9 +410,9 @@
       SVN_ERR(svn_wc__loggy_set_readonly(log_accum, adm_access,
                                          real_props, pool));
     }
-  else
+  else if (!tmp_entry.has_props)
     {
-      /* No property modifications, remove the file instead. */
+      /* No property modifications and no more props, remove the file. */
       SVN_ERR(svn_wc__loggy_remove(log_accum, adm_access, real_props, pool));
     }

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


Re: [PATCH] libsvn_wc losing properties?

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Justin Erenkrantz writes:
 > On 3/6/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
 > > This sounds like a regression introduced by wc-propcaching.  Your
 > > solution isn't correct, because one of the main points of
 > > wc-propcaching was to not have a working props file when there are no
 > > property *changes*:-) I'll have a look tonight if possible.
 > 
 > If you build with ra_serf with the latest Serf trunk and revert 18749
 > (which reduces our exposure to this bug), you should be able to
 > reproduce this with:
 > 
 > % svn co -r18738 http://svn.collab.net/repos/svn/trunk/
 > % svn up trunk
 > 
 > You'll see any changed dir (such as www) loses it svn:ignore properties.
 > 

Just to follow up, I wasn't able to reproduce this (even with Justin's
help via IRC). Removing the working props when there are no property
modifications relative to the base props is a feature:-)

Regards,
//Peter

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

Re: [PATCH] libsvn_wc losing properties?

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 3/6/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> This sounds like a regression introduced by wc-propcaching.  Your
> solution isn't correct, because one of the main points of
> wc-propcaching was to not have a working props file when there are no
> property *changes*:-) I'll have a look tonight if possible.

If you build with ra_serf with the latest Serf trunk and revert 18749
(which reduces our exposure to this bug), you should be able to
reproduce this with:

% svn co -r18738 http://svn.collab.net/repos/svn/trunk/
% svn up trunk

You'll see any changed dir (such as www) loses it svn:ignore properties.

HTH.  -- justin

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


Re: [PATCH] libsvn_wc losing properties?

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 3/6/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> This sounds like a regression introduced by wc-propcaching.  Your
> solution isn't correct, because one of the main points of
> wc-propcaching was to not have a working props file when there are no
> property *changes*:-) I'll have a look tonight if possible.

As a follow-up, in r18749, I reduced ra_serf's exposure to this bug to
just the root of the WC as  mod_dav_svn does actually tell us if the
properties changed - which is almost good enough except it doesn't
handle the root directory of our WC.  HTH.  -- justin

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


Re: [PATCH] libsvn_wc losing properties?

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Justin Erenkrantz writes:
 > Index: subversion/libsvn_wc/props.c
 > ===================================================================
 > --- subversion/libsvn_wc/props.c        (revision 18738)
 > +++ subversion/libsvn_wc/props.c        (working copy)
 > @@ -410,9 +410,9 @@
 >        SVN_ERR(svn_wc__loggy_set_readonly(log_accum, adm_access,
 >                                           real_props, pool));
 >      }
 > -  else
 > +  else if (!tmp_entry.has_props)
 >      {
 > -      /* No property modifications, remove the file instead. */
 > +      /* No property modifications and no more props, remove the file. */
 >        SVN_ERR(svn_wc__loggy_remove(log_accum, adm_access, real_props, pool));
 >      }

This sounds like a regression introduced by wc-propcaching.  Your
solution isn't correct, because one of the main points of
wc-propcaching was to not have a working props file when there are no
property *changes*:-) I'll have a look tonight if possible.

Thanks for finding this, Justin,
//Peter

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