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

Re: SVN_ERR_ASSERT with reintegrate

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 27 Mar 2012 14:13:44 +0100 (BST)

Stefan Küng wrote:

> On 27.03.2012 12:06, Julian Foad wrote:
>> Stefan Küng<tortoisesvn_at_gmail.com>  wrote:
>>
>>> There's an SVN_ERR_ASSERT when reintegrating. [...]
>>>
>>> libsvn_tsvn!svn_ra_get_location_segments+0x12
>>> libsvn_tsvn!svn_client__repos_location_segments+0x9f
>>> libsvn_tsvn!svn_client_merge4+0xff8
>>> libsvn_tsvn!svn_client_merge4+0x172e
>>> libsvn_tsvn!svn_client_merge_reintegrate+0x113
>>> tortoiseproc!SVN::MergeReintegrate+0x12c
>>>
>>> in ra_loader.c, function svn_ra_get_location_segments() the
>>>     SVN_ERR_ASSERT(*path != '/');
>>> is hit.
[...]
>>>   From looking at the code I think this would happen if someone
>>> tries to merge from or to a repo root. [...]
>>
>> Yup.  In trunk there is such a check in find_reintegrate_merge:
[...]
>> First included in release 1.6.10. [...]
>
> Strange. The crash dumps are for the latest 1.7.4 release (TSVN 1.7.6).
> So the libraries are from that version as well.

OK.  In that case I suspect that it's not being caused by a merge to or from the repo root, but by some other conditions in which reintegrate tries to calculate a relative path to

I suspect the invalid rel-path to svn_client__repos_location_segments() must occur in one of these two places (quoting from 1.7.4, abbreviating):

static svn_error_t *
find_unmerged_mergeinfo(... source_catalog ... source_repos_rel_path ...)
{ ...
      for (hi = apr_hash_first(scratch_pool, source_catalog); ...)
        {
          source_path = svn__apr_hash_index_key(hi);
          source_path_rel_to_session =
            svn_relpath_skip_ancestor(source_repos_rel_path, source_path);
          ...
            /* Get the source path's natural history and convert it to
               mergeinfo.  Then merge that natural history into source
               path's explicit or inherited mergeinfo. */
            svn_client__repos_location_segments(
              ..., source_path_rel_to_session, ...));
}

static svn_error_t *
calculate_left_hand_side(... target_repos_rel_path,
                         subtrees_with_mergeinfo,
                         target_repos_root, ...)
{ ...
  /* Get the history (segments) for TARGET_ABSPATH and any of its subtrees
     with explicit mergeinfo. */
  for (hi = apr_hash_first(scratch_pool, subtrees_with_mergeinfo); ...)
    {
      absolute_path = svn__apr_hash_index_key(hi);
      svn_client__path_relative_to_root(
        &path_rel_to_root, ..., absolute_path, target_repos_root, ...)
      path_rel_to_session = svn_relpath_skip_ancestor(
                              target_repos_rel_path,path_rel_to_root);
      svn_client__repos_location_segments(... path_rel_to_session ...)
}

But I can't (yet) figure out by inspection what merge inputs would cause that.

In trunk, some time in the last few months I changed these code paths so that this can't happen.  Instead of calculating a relpath (relative to the session), instead the code now reparents the session to the desired URL and passes a literal "" as the relpath argument to svn_ra_get_location_segments().  My intention was just to make the code simpler to read, but the change may have had the effect of eliminating some latent bugs as well.  (Of course I accept that it could have introduced bugs as well, or just changed the result from an assertion failure to quietly doing something wrong.)

We ought to back-port some sort of fix or improved failure mode to 1.7.x when we figure it out.  If nothing else, we could throw an error if the relpath comes out as NULL in the two code places I quoted above.

- Julian
Received on 2012-03-27 15:14:26 CEST

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