On Mon, 19 Mar 2007, Paul Burba wrote:
...
> Attached is a patch addressing our recent IRC conversation regarding the
> need to not combine repos mergeinfo with local mergeinfo when the latter
> is modified. Per your suggestion I created a new secondary header file
> in include/private/.
...
Nice Paul! Comments in-line below.
> [[[
> On the merge-tracking branch: Don't combine repos with local merge info
> when the latter is modified.
>
> * subversion/include/private/svn_wc_private.h
> New file.
> (svn_wc__props_modified): Similar to svn_wc_props_modified_p()
> but also indicates the specific props that are modified and their
> new values.
>
> * build.conf
> (libsvn_wc): Add private\svn_wc_private.h to msvc-export list.
>
> * subversion/libsvn_client/diff.c
> Include private/svn_wc_private.h
> (get_wc_and_repos_merge_info): Use svn_wc__props_modified() to detect
> modified local merge info.
> local changes to merge info. Remove FIXME portion of comment.
I assume the the last line of the log msg for
get_wc_and_repos_merge_info() is what we intend to use here.
> * subversion/libsvn_wc/props.c
> (svn_wc__props_modified): New.
> ]]]
Content-Description: local.mergeinfo.mods.diff.txt
> Index: build.conf
> ===================================================================
> --- build.conf (revision 23893)
> +++ build.conf (working copy)
> @@ -328,7 +328,7 @@
> path = subversion/libsvn_wc
> libs = libsvn_delta libsvn_diff libsvn_subr aprutil apriconv apr
> install = lib
> -msvc-export = svn_wc.h
> +msvc-export = svn_wc.h private\svn_wc_private.h
>
> # Subversion plugin for Apache's mod_dav
> [mod_dav_svn]
> Index: subversion/include/private/svn_wc_private.h
> ===================================================================
> --- subversion/include/private/svn_wc_private.h (revision 0)
> +++ subversion/include/private/svn_wc_private.h (revision 0)
> @@ -0,0 +1,45 @@
> +/*
> + * svn_wc_private.h: Declarations for libsvn_wc APIs which are
> + * only used internally to Subversion.
> + *
> + * ====================================================================
> + * Copyright (c) 2000-2007 CollabNet. All rights reserved.
Just 2007 is sufficient.
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifndef SVN_WC_PRIVATE_H
> +#define SVN_WC_PRIVATE_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +/** Set @a *modified_p to non-zero if @a path's properties are modified
> + * with regard to the base revision, else set @a modified_p to zero.
> + * If any properties are modified set @a *which_props to hashtable
> + * (<tt>const char *name</tt> -> <tt>const svn_string_t *value</tt>)
> + * that contains only the modified properties, else @a *which_props is
> + * set to an empty hash. @a adm_access must be an access baton for
> + * @a path. @a *which_props is allocated in @a pool.
> + */
> +svn_error_t *svn_wc__props_modified(svn_boolean_t *modified_p,
> + const char *path,
> + apr_hash_t **which_props,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> +
We could drop the modified_p flag here, instead using the doc string
to indicate that *WHICH_PROPS will be set to NULL (or an empty hash)
when no props are modified.
> +#ifdef __cplusplus
> +}
> +#endif /* __cplusplus */
> +
> +#endif /* SVN_WC_PRIVATE_H */
>
> Property changes on: subversion\include\private\svn_wc_private.h
> ___________________________________________________________________
> Name: svn:eol-style
> + native
>
> Index: subversion/libsvn_client/diff.c
> ===================================================================
> --- subversion/libsvn_client/diff.c (revision 23893)
> +++ subversion/libsvn_client/diff.c (working copy)
> @@ -45,6 +45,7 @@
> #include "client.h"
> #include <assert.h>
>
> +#include "private/svn_wc_private.h"
> #include "svn_private_config.h"
>
> /* Sanity check -- ensure that we have valid revisions to look at. */
> @@ -1947,6 +1948,9 @@
>
> if (repos_mergeinfo != NULL)
> {
> + svn_boolean_t modified_props;
> + apr_hash_t *which_props;
That would do away with this "which_props" local variable.
> +
> /* If pre-existing local changes reverted some of the merge info
> on TARGET_WCPATH, this should be recorded in the WC's merge
> info as TARGET_WCPATH's complete set of merge info minus
> @@ -1958,13 +1962,18 @@
> using a separate, negative set of merge source -> revision
> range mappings. */
>
> - /* ### FIXME: Avoid combining local merge info with repos merge
> - ### info for TARGET_WCPATH when its merge info has been
> - ### locally modified. We need a variation of
> - ### libsvn_wc/props.c:svn_wc_props_modified_p() which reports
> - ### on a single property value. */
> - SVN_ERR(svn_mergeinfo_merge(target_mergeinfo, repos_mergeinfo,
> - pool));
> + /* Avoid combining local merge info with repos merge info for
> + TARGET_WCPATH when its merge info has been locally modified. */
> + SVN_ERR(svn_wc__props_modified(&modified_props, target_wcpath,
> + &which_props, adm_access, pool));
> +
> + if(!modified_props
> + || !apr_hash_get(which_props, SVN_PROP_MERGE_INFO,
> + APR_HASH_KEY_STRING))
> + {
> + SVN_ERR(svn_mergeinfo_merge(target_mergeinfo, repos_mergeinfo,
> + pool));
> + }
...and make sense here for the caller.
> }
> return SVN_NO_ERROR;
> }
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c (revision 23893)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -2549,3 +2549,53 @@
> }
> return FALSE;
> }
> +
> +svn_error_t *
> +svn_wc__props_modified(svn_boolean_t *modified_p,
> + const char *path,
> + apr_hash_t **which_props,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> +{
> + SVN_ERR(svn_wc_props_modified_p(modified_p, path, adm_access, pool));
I'd rather expect svn_wc_props_modified_p() to be calling this new
routine, rather than vice-versa. If WHICH_PROPS is NULL, we could
bail out of the old svn_wc_props_modified_p() logic earlier.
> + *which_props = apr_hash_make(pool);
> +
> + if (*modified_p)
> + {
> + apr_pool_t *subpool = svn_pool_create(pool);
> + const svn_wc_entry_t *entry;
> + const char *prop_path, *prop_base_path;
> + apr_array_header_t *local_propchanges;
> + apr_hash_t *localprops, *baseprops;
> + int i;
> +
> + SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, subpool));
> +
> + /* Get the paths of the working and base prop files. */
> + SVN_ERR(svn_wc__prop_path(&prop_path, path, entry->kind, FALSE,
> + subpool));
> + SVN_ERR(svn_wc__prop_base_path(&prop_base_path, path, entry->kind,
> + FALSE, subpool));
> +
> + /* Load the working and base props into temporary hashses. */
> + localprops = apr_hash_make(subpool);
> + baseprops = apr_hash_make(subpool);
> + SVN_ERR(svn_wc__load_prop_file(prop_path, localprops, subpool));
> + SVN_ERR(svn_wc__load_prop_file(prop_base_path, baseprops, subpool));
> +
> + /* Don't use subpool here. */
> + SVN_ERR(svn_prop_diffs(&local_propchanges, localprops,
> + baseprops, pool));
> +
> + /* Put any modified props in *WHICH_PROPS. */
> + for (i = 0; i < local_propchanges->nelts; i++)
> + {
> + svn_prop_t *propt = &APR_ARRAY_IDX(local_propchanges, i,
> + svn_prop_t);
> + apr_hash_set(*which_props, propt->name,
> + APR_HASH_KEY_STRING, propt->value);
> + }
> + svn_pool_destroy(subpool);
> + }
> + return SVN_NO_ERROR;
> +}
> \ No newline at end of file
Also, is there any overlap with svn_client__get_prop_from_wc()?
We call svn_client__get_prop_from_wc() from svn_client__parse_merge_info(),
which is in turn called from a loop inside get_wc_merge_info()
(meaning we're probably doing some redudant work here).
- Dan
- application/pgp-signature attachment: stored
Received on Mon Mar 19 18:03:45 2007