[PATCH v2] Replace svn_path_join() in libsvn_client/mergeinfo.c

On Fri, Nov 06, 2009 at 01:57:51PM -0800, Daniel Rall wrote:
> On Fri, Nov 6, 2009 at 10:39 AM, Daniel Näslund <daniel_at_longitudo.com> wrote:

New log:

[[[
Remove use of deprecated svn_path_join().

* subversion/libsvn_client/mergeinfo.c
  (svn_client__adjust_mergeinfo_source_paths): Replace svn_path_join()
    with svn_uri_join(). An svn_mergeinfo_t key is the "base" parameter.
  (svn_client__get_wc_mergeinfo): Replace svn_path_join() with
    svn_relpath_join(). The "base" parameter is created with a relative
    path from svn_dirent_dirname().
  (svn_client_mergeinfo_get_merged): Replace svn_path_join() with
    svn_uri_join(). target_repos_root is the "base" parameter.
  
Patch by: Daniel Näslund <daniel_at_longitudo.com>
Suggested by: dlr
]]]

> > Some background: In subversion/include/svn_mergeinfo.h svn_mergeinfo_t
> > is just a typedef for a apr_hash_t. The keys are described like this:
> >
> >  * (c) @c svn_mergeinfo_t, called "mergeinfo".  A hash mapping merge
> >  *     source paths (@c const char *, starting with slashes) to
> >  *     non-empty rangelist arrays.  A @c NULL hash is used to represent
> >  *     no mergeinfo and an empty hash is used to represent empty
> >  *     mergeinfo.
> >
> > I'm assuming that means a dirent.
>
> It's a path fragment. In this case, the path is relative to the
> repository root. While you may be able to use it as a local file
> system path in some situations, it's really a URL fragment.

After some reading I agree. But, they keys in svn_mergeinfo_t is not
uri_encoded! Or atleast they don't say anything about beeing
uri-encoded and when svn_client_mergeinfo_get_merged wants a hash of
full urls, the keys are being uri-encoded before the join! And in
logs_for_mergeinfo_rangelist() the following lines says that keys are
required to be repository-absolute but does not uri-encode the string:

[[[
  /* FILTER_LOG_ENTRY_BATON_T->TARGET_MERGEINFO_CATALOG's keys are required
     to be repository-absolute. */

  if (apr_hash_count(target_mergeinfo_catalog))
    {
      apr_hash_index_t *hi;
      svn_mergeinfo_catalog_t rekeyed_catalog = apr_hash_make(scratch_pool);

      for (hi = apr_hash_first(scratch_pool, target_mergeinfo_catalog);
           hi;
           hi = apr_hash_next(hi))
        {
          const char *path = svn_apr_hash_index_key(hi);

          if (!svn_dirent_is_absolute(path))
            apr_hash_set(rekeyed_catalog,
                         svn_dirent_join("/", path, scratch_pool),
                         APR_HASH_KEY_STRING,
                         svn_apr_hash_index_val(hi));
        }
      target_mergeinfo_catalog = rekeyed_catalog;
]]]

What I'm saying is that if repository-absolute means that it's an uri
(generalization of url) then it should be uri-encoded, right?

/Daniel