On Mon, 2008-10-06 at 15:56 -0700, David Glasser wrote:
> This logic makes sense to me when svn_wc__merge_props gets a non-NULL
> server_baseprops argument (ie, the props actually represent what the
> *server* thinks the old revision from the diff had) but not when that
> argument is NULL and that argument just gets replaced by the wc BASE.
David,
Thanks for examining this.
[[[
/* Given ADM_ACCESS/PATH and an array of PROPCHANGES based on
SERVER_BASEPROPS, merge the changes into the working copy.
Append all necessary log entries to ENTRY_ACCUM.
If BASE_PROPS or WORKING_PROPS is NULL, use the props from the
working copy.
If SERVER_BASEPROPS is NULL then use base props as PROPCHANGES
base.
[...]
*/
svn_error_t *svn_wc__merge_props(svn_wc_notify_state_t *state,
svn_wc_adm_access_t *adm_access,
const char *path,
apr_hash_t *server_baseprops,
apr_hash_t *base_props,
apr_hash_t *working_props,
const apr_array_header_t *propchanges,
[...]
]]]
I read the doc-string of svn_wc__merge_props() as meaning that it would
apply the changes defined by 'propchanges' relative to
('server_baseprops', which defaults to 'base_props' for convenience if
not given).
> Have you checked to make sure that all the relevant codepaths pass a
> good server_baseprops?
I assumed that callers would pay attention to this, and supply the
arguments as required, and I did not check them.
Checking now...
This is called from one place in the "merge" code path, which supplies
server_baseprops explicitly. It is called from two places in the
"update" code path, which both pass server_baseprops as NULL. This
appears to be correct, as during an update the base is known to be the
server's starting point.
Does this make sense or did you understand something different about
when server_baseprops is passed as NULL?
- Julian
> --dave
>
> 2008/10/6 Julian Foad <julianfoad_at_btopenworld.com>:
> > On Thu, 2008-09-25 at 19:19 +0100, Julian Foad wrote:
> >> This patch fixes merge_tests.py 106.
> >
> > This version adjusts merge_tests.py 41 to expect success in its second
> > prop-change merge, so no failure is introduced.
> >
> > I will commit this soon if no further comments.
> >
> > It would be good if a mergeinfo expert could take a look at the
> > stripped-down mergeinfo-merging function here,
> > apply_single_mergeinfo_prop_change().
> >
> > - Julian
> >
> >
> >> Problem:
> >>
> >> Merging a prop-change into the WC, the existing code regarded it as a
> >> conflict if the property had any local modification in the WC, even if
> >> this mod was exactly what was needed to make the working copy of the
> >> prop value be the same as the old value of the incoming change. For
> >> example, if you tried to merge two successive changes to the same
> >> property into an initially clean and up-to-date WC, you would get a
> >> conflict if you didn't commit in between the two halves. This breaks the
> >> merge-tracking merges, and merge_tests.py 106 is an XFail test for it.
> >>
> >> Solution:
> >>
> >> I noticed that the logic paid great attention to the "base" version in
> >> the WC, in ways that are rather obscure to me, whereas I would expect it
> >> could safely ignore the "base" value altogether and just merge into the
> >> working value. The attached patch rewrites that bit of logic using an
> >> extremely simple condition: "if WORKING is the same as OLD then it's a
> >> simple update, else call the resolver".
> >>
> >> Help needed:
> >>
> >> The only outstanding problem I know of is that merge_tests.py 41 appears
> >> to have bogus expectations relating to whether to such a merge. I tried
> >> briefly to fix it but have put it aside for now. If anyone can help with
> >> this, that would be much appreciated.
> >>
> >> Theory:
> >>
> >> I believe we always want to perform merges into the "working" state of
> >> the WC, without much or any regard to the "base" state. (Note that
> >> updating the BASE during an "update" or "switch" is certainly needed, it
> >> just doesn't impact the merge into WORKING.) Indeed, I believe the
> >> current merge tracking implementation depends on this theory because it
> >> splits merges into arbitrary sub-ranges. Is there any doubt? I keep
> >> seeing old bits of code and tests that say explicitly that they expect a
> >> conflict if there is any local mod. I intend to keep removing/fixing
> >> such erroneous claims, unless someone points out a contrary theory.
> >>
> >> Thanks.
> >> - Julian
> >>
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> > For additional commands, e-mail: dev-help_at_subversion.tigris.org
> >
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-07 12:57:23 CEST