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

Re: svn commit: r17328 - branches/wc-propcaching/subversion/libsvn_wc

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-11-15 10:32:07 CET

On Mon, 14 Nov 2005, Ivan Zhakov wrote:

> Peter,
I have some comments on your commit. Please check it inline. I hope it
helps you.

On 11/14/05, lundblad@tigris.org <lundblad@tigris.org> wrote:
> Author: lundblad
> Date: Mon Nov 14 06:32:32 2005
> Modified: branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c
> Url: http://svn.collab.net/viewcvs/svn/branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c?rev=17328&p1=branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c&p2=branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c&r1=17327&r2=17328
> ==============================================================================
> --- branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c (original)
> +++ branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c Mon Nov 14 06:32:32 2005
> @@ -42,6 +42,7 @@
> #include "props.h"
> #include "translate.h"
> #include "entries.h"
> +#include "lock.h"
>
> #include "svn_private_config.h"
>
> @@ -896,6 +897,11 @@
>
> /* Get the right access baton for the job. */
> SVN_ERR (svn_wc_adm_probe_retrieve (&adm_access, adm_access, path, pool));
> +
> + /* For an enough recent WC, we can have a really easy out. */
> + if (svn_wc__adm_wc_format (adm_access) > SVN_WC__NO_PROPCACHING_VERSION
> + && ! entry->prop_mods)
> + return SVN_NO_ERROR;
>You leave variable *tempfile uninitialized. But it only begin of

Good catch! Fixed in r17348.

>problems, if you fix this you have problems in log_do_commited when
>commiting clearing of props, because tempfile doesn't exists and
>log_do_commited thinks that there is no props changes!

But in this case, there were no property modifications, so is that a real
problem? The function now fullfills its API.

...
> - /* ### TODO: Update props flags when we have them. */
> + if (modified_p || entry->schedule == svn_wc_schedule_replace)
> + SVN_ERR (svn_wc__install_props (&log_accum, adm_access, name, baseprops,
> + baseprops, FALSE, pool));
>I like this change! :)

Thanks:-) It is in line with your envisioned property abstraction.

...
>For present time svn_wc__has_props() used only in one place, in
>assemble_status() mostyle for optimize call to svn_wc__is_special(). I
>consider we should reconsider later necessity of this function.

It actually has a semantic difference as well. The property status is set
to none if there are no properties at all, and normal if there or no
propmods. See TODO on the branch for how I thihnk we should handle this.

Thanks for the review!
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 15 10:34:12 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.