RE: RE: Re: [PATCH][merge-tracking]merge_tests 1 fails after r23785

> -----Original Message-----
> From: Paul Burba [mailto:pburba@collab.net]
> Sent: Wednesday, March 14, 2007 11:43 AM
> To: Kamesh Jayachandran
> Cc: SVN Dev; Daniel Rall
> Subject: RE: Re: [PATCH][merge-tracking]merge_tests 1 fails
> after r23785
>
> > -----Original Message-----
> > From: Kamesh Jayachandran [mailto:kamesh@collab.net]
> > Sent: Wednesday, March 14, 2007 11:07 AM
> > To: Paul Burba
> > Cc: SVN Dev; Daniel Rall
> > Subject: Re: [PATCH][merge-tracking]merge_tests 1 fails after r23785
> >
> > Hi Paul,
> >
> > > I keep saying "should" because this doesn't currently work.
> > When the
> > > last merge is performed (r2:3 into /A/D/G) no textual
> merge occurs,
> > > although the mergeinfo on rho is correctly updated. I'm
> > still trying
> > > to figure out what's wrong but that's a separate issue from
> > the test,
> > > so I'm sending this along now.
> > >
> > > Paul B.
> > >
> > My reasoning for the failure is,
> > * When you did a revert 'r-3' on other_wc/A/D/G/rho. It did
> a magic of
> > getting the mergeinfo from 'other_wc' as '2-3' and removed
> 3 from it
> > and retained 2.
> > * When you do subsequent merge on other_wc/A/D/G,
> 'find_wc_merge_info'
> > adds '2-3' of 'other_wc and '2' of 'other_wc/A/D/G/rho' and gives
> > '2-3', this makes our merge a no-op on rho too.
>
> Kamesh,
>
> That analysis is correct. It's because of a bug when the child
> ('other_wc/A/D/G/rho') of a merge target has different merge
> info from the target itself ('other_wc/A/D/G').
>
> The problem is that do_single_file_merge() passes a relative path to
> get_wc_target_merge_info() which opens an svn_wc_entry_t with
> that path and then passes both the entry and the relative path to
> find_wc_merge_info().
>
> find_wc_merge_info() then changes the relative path to an
> absolute path and looks for it in the entry. It never finds
> it because of the relative/absolute mismatch. So we never
> get the child's explicit merge info and crawl up the WC to
> 'other_wc/' and inherit it's merge info leading to the no-op
> you describe.
>
> Initially I was thinking the fix was simple, just keep the
> relative wcpath the first time we call
> svn_client__parse_merge_info() so it works with the path in
> entry. Then if we didn't find any merge info we switch to
> the absolute path and open new adm_accesses and entries with
> the absolute path and everthing is fine:
>
> static svn_error_t *
> find_wc_merge_info(apr_hash_t **mergeinfo,
> svn_boolean_t *inherited,
> const svn_wc_entry_t *entry,
> const char *wcpath,
> svn_wc_adm_access_t *adm_access,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> const char *walk_path = "";
> apr_hash_t *wc_mergeinfo;
>
> - SVN_ERR(svn_path_get_absolute(&wcpath, wcpath, pool));
>
> while (TRUE)
> {
> /* Look for merge info on WCPATH. If there isn't any, walk
> towards the root of the WC until we encounter either (a) an
> unversioned directory, or (b) merge info. If we
> encounter (b),
> use that inherited merge info as our baseline. */
> SVN_ERR(svn_client__parse_merge_info(&wc_mergeinfo,
> entry, wcpath,
> adm_access, ctx, pool));
>
> + if (svn_path_is_absolute(wcpath, strlen(wcpath)))

Argh! This should have been
+ if (!svn_path_is_absolute(wcpath, strlen(wcpath)))

> + SVN_ERR(svn_path_get_absolute(&wcpath, wcpath, pool));
>
> if (apr_hash_count(wc_mergeinfo) == 0 &&
> !svn_path_is_root(wcpath, strlen(wcpath)))
> {
>
> Problem is that isn't fine. It works to the extent that it
> finds the correct merge info for 'other_wc/A/D/G/rho' in our
> example, but it breaks down badly: in do_single_file_merge()
> the merge doesn't happen due to temp files being created in
> the wrong place, many other tests start failing, svn.exe uses
> 2.5 GB of memory, etc...needless to say I'm still trying to
> figure out what is going wrong here.

Figured it all out (hopefully) it was all fairly simple in the end.

Patch attached and log below. Please take a look if you have a moment,
thanks,

Paul B.

[[[
On the merge-tracking branch: Fix merge_test-1 failure.

Fix a problem where we missed explicit merge info on a merge target's
children and erroneously inherited merge info from the target or one
its ancestors (or not at all). Also fix problem where we created
temporary files for merging of a target's children in the wrong place,
resulting in merges that never happen but should.

* subversion/libsvn_client/diff.c
  (find_wc_merge_info): Don't use absolute path when looking for merge
  info in the svn_wc_entry passed in, we can miss explicit merge info
  if we do.
  (single_file_merge_get_file): Replace merge_cmd_baton struct with a
  simple path since the former may actually point to the parent of the
  file we want to merge.
  (do_single_file_merge): Update calls to single_file_merge_get_file().
  Pass actual path to file not merge_b->target, which might point to the
  actual target's parent.

* subversion/tests/cmdline/merge_tests.py
  (textual_merges_galore): Create correct status rather than creating
  the wrong one and tweaking it. Don't check svn:mergeinfo props of
  final merge with run_and_verify_merge since that chokes on conflict
  files, check the props separately instead.
  (obey_reporter_api_semantics_while_doing_subtree_merges): We *really*
  do expect some clean merges to occur in the final merge.
  (test_list): Remove XFail from textual_merges_galore.
  
]]]

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