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
> New Revision: 17328
>
> Modified:
> branches/wc-propcaching/subversion/libsvn_wc/adm_crawler.c
> branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c
> branches/wc-propcaching/subversion/libsvn_wc/copy.c
> branches/wc-propcaching/subversion/libsvn_wc/log.c
> branches/wc-propcaching/subversion/libsvn_wc/props.c
> branches/wc-propcaching/subversion/libsvn_wc/props.h
> branches/wc-propcaching/subversion/libsvn_wc/update_editor.c
>
> Log:
> On wc-propcaching branch: Stop storing a working props file unless there
> are property modifications and stop storing a base props file unless there are
> any properties. Use the prop-mods flag in the WC entry to determine
> which file to load when reading working properties.
>
> * subversion/libsvn_wc/props.c
> (svn_wc__load_props): New function.
> (svn_wc__install_props): Make sure prop files don't exist when they
> shouldn't.
> (svn_wc__merge_props, svn_wc_prop_list): Use svn_wc__load_props instead of
> loading props manually.
> (svn_wc__merge_prop_diffs): Add TODO to revisit this later.
> (svn_wc__has_props): Check baseprops if we're in a enough recent WC
> and there are no property modifications according to the entry.
> Add TODO for dberlin.
> (svn_wc_props_modified_p): Add TODO item.
> (svn_wc_get_prop_diffs): Use svn_wc__load_props to read properties.
>
> * subversion/libsvn_wc/props.h (svn_wc__load_props): Declare.
>
> * subversion/libsvn_wc/copy.c
> (copy_file_administratively): Use svn_wc__load_props().
>
> * subversion/libsvn_wc/adm_crawler.c
> (svn_wc_transmit_prop_deltas): Return early if there are no prop mods.
>
> * subversion/libsvn_wc/log.c (log_do_committed): Remove working props file.
>
> * subversion/libsvn_wc/adm_ops.c
> (revert_admin_things): Use svn_wc__install_props() to revert the props.
> This means we will always load the base props into memory, but we will
> have to do that anyway when we have flags for the presence of individual
> properties.
> Fix misspelled comment.
>
> * subversion/libsvn_wc/update_editor.c
> (install_added_props): Use svn_wc__install_props() to do most of the work
> for regular props.
>
> 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
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!
>
> /* First, get the prop_path from the original path */
> SVN_ERR (svn_wc__prop_path (&props, path, entry->kind, FALSE, pool));
>
> Modified: branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c
> Url: http://svn.collab.net/viewcvs/svn/branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c?rev=17328&p1=branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c&p2=branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c&r1=17327&r2=17328
> ==============================================================================
> --- branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c (original)
> +++ branches/wc-propcaching/subversion/libsvn_wc/adm_ops.c Mon Nov 14 06:32:32 2005
> @@ -1349,6 +1349,7 @@
> apr_hash_t *modify_entry_atts = apr_hash_make (pool);
> svn_stringbuf_t *log_accum = svn_stringbuf_create ("", pool);
> const char *bprop, *rprop, *wprop; /* full paths */
> + apr_hash_t *baseprops;
> const char *adm_path = svn_wc_adm_access_path (adm_access);
>
> /* Build the full path of the thing we're reverting. */
> @@ -1377,27 +1378,13 @@
>
> /* Get the full list of property changes and see if any magic
> properties were changed. */
> - SVN_ERR (svn_wc_get_prop_diffs (&propchanges, NULL, fullpath,
> + SVN_ERR (svn_wc_get_prop_diffs (&propchanges, &baseprops, fullpath,
> adm_access, pool));
>
> /* Determine if any of the propchanges are the "magic" ones that
> might require changing the working file. */
> target_needs_retranslation = svn_wc__has_magic_property (propchanges);
>
> - /* There may be a base props file but no working props file, if
> - the mod was that the working file was `R'eplaced by a new
> - file with no props.
> -
> - If there is a pristing property file, copy it out as the
> - working property file, else just remove the working property
> - file. */
> - SVN_ERR (svn_wc__loggy_copy (&log_accum, NULL,
> - adm_access, svn_wc__copy_normal,
> - bprop, wprop, TRUE, pool));
> -
> - apr_hash_set (modify_entry_atts, SVN_WC__ENTRY_ATTR_PROP_MODS,
> - APR_HASH_KEY_STRING, "false");
> - /* ### TODO: Update properties flags when we have such flags. */
> }
> else if (entry->schedule == svn_wc_schedule_replace)
> {
> @@ -1413,14 +1400,14 @@
> svn_wc_props_modified_p is deliberately ignoring the
> base-props; it's "no" answer simply means that there are no
> working props. It's *still* possible that the base-props
> - exist, however, from the original replaced file. If they do,
> - then we need to restore them. */
> - SVN_ERR (svn_wc__loggy_copy (&log_accum, &tgt_modified,
> - adm_access, svn_wc__copy_normal,
> - bprop, wprop, FALSE, pool));
> + exist, however, from the original replaced file. */
> + baseprops = apr_hash_make (pool);
> + SVN_ERR (svn_wc__load_prop_file (rprop, baseprops, pool));
> }
>
> - /* ### 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! :)
> Modified: branches/wc-propcaching/subversion/libsvn_wc/props.c
> Url: http://svn.collab.net/viewcvs/svn/branches/wc-propcaching/subversion/libsvn_wc/props.c?rev=17328&p1=branches/wc-propcaching/subversion/libsvn_wc/props.c&p2=branches/wc-propcaching/subversion/libsvn_wc/props.c&r1=17327&r2=17328
> ==============================================================================
> --- branches/wc-propcaching/subversion/libsvn_wc/props.c (original)
> +++ branches/wc-propcaching/subversion/libsvn_wc/props.c Mon Nov 14 06:32:32 2005
> @@ -1721,6 +1802,8 @@
> svn_boolean_t is_empty;
> const char *prop_path;
> const svn_wc_entry_t *entry;
> + svn_boolean_t has_propcaching =
> + svn_wc__adm_wc_format (adm_access) > SVN_WC__NO_PROPCACHING_VERSION;
>
> SVN_ERR (svn_wc_entry (&entry, path, adm_access, FALSE, pool));
>
> @@ -1732,7 +1815,13 @@
> return SVN_NO_ERROR;
> }
>
> - SVN_ERR (svn_wc__prop_path (&prop_path, path, entry->kind, FALSE, pool));
> + if (has_propcaching && ! entry->prop_mods)
> + SVN_ERR (svn_wc__prop_base_path (&prop_path, path, entry->kind, FALSE, pool));
> + else
> + SVN_ERR (svn_wc__prop_path (&prop_path, path, entry->kind, FALSE, pool));
> + /* ### TODO: (dberlin) When we have property flags in the entry, we can
> + know for sure that we *have* props if any flag is set.
> + This *might* not be enough. */
> SVN_ERR (empty_props_p (&is_empty, prop_path, pool));
>
> if (is_empty)
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.
--
Ivan Zhakov
Received on Mon Nov 14 14:33:04 2005