On Mon, 02 Apr 2007, Peter Lundblad wrote:
> dlr@tigris.org writes:
...
> > --- trunk/subversion/include/svn_client.h (original)
> > +++ trunk/subversion/include/svn_client.h Fri Mar 30 19:41:09 2007
...
> > +/**
> > + * Retrieve the merge info for @a path_or_url in @a *mergeinfo,
> > + * storing a mapping of repository-relative paths to @c
> > + * apr_array_header_t *'s of @c svn_merge_range_t *'s, or @c NULL if
> > + * there is no merge info.
> > + *
>
> Document @a pool usage.
Done in r24342.
> > + * @a path_or_url is a WC path or repository URL. @a revision is the
> > + * revision at which to get @a path's merge info.
>
> ^ path_or_url
Done.
> > + *
> > + * @since New in 1.5.
> > + */
> > +svn_error_t *
> > +svn_client_get_mergeinfo(apr_hash_t **mergeinfo,
> > + const char *path_or_url,
> > + svn_opt_revision_t *revision,
> > + svn_client_ctx_t *ctx,
> > + apr_pool_t *pool);
>
> Is there a reason this doesn't take a peg revision?
REVISION seems implicitly peg to me. I'm fine with adding an explicit
PEG_REVISION, if people think it's necessary. I couldn't think of a
meaningful use case where the operative revision would differ from the
peg revision, but adding a peg revision would be more consistent with
our other APIs.
...
> > --- trunk/subversion/libsvn_client/merge.c (original)
> > +++ trunk/subversion/libsvn_client/merge.c Fri Mar 30 19:41:09 2007
> > @@ -2371,3 +2371,59 @@
> > target_wcpath, recurse, ignore_ancestry, force,
> > dry_run, NULL, ctx, pool);
> > }
> > +
> > +svn_error_t *
> > +svn_client_get_mergeinfo(apr_hash_t **mergeinfo,
> > + const char *path_or_url,
> > + svn_opt_revision_t *revision,
> > + svn_client_ctx_t *ctx,
> > + apr_pool_t *pool)
> > +{
> > + svn_revnum_t rev;
> > + svn_wc_adm_access_t *adm_access;
> > + const svn_wc_entry_t *entry;
> > + svn_ra_session_t *ra_session;
> > + const char *url;
> > +
> > + if (svn_path_is_url(path_or_url))
> > + {
> > + url = path_or_url;
> > + }
> > + else
> > + {
> > + SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, path_or_url, FALSE,
> > + 0, ctx->cancel_func, ctx->cancel_baton,
> > + pool));
> > + SVN_ERR(svn_wc__entry_versioned(&entry, path_or_url, adm_access, FALSE,
> > + pool));
> > + if (entry->url == NULL)
> > + return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
> > + _("'%s' has no URL"),
> > + svn_path_local_style(path_or_url, pool));
> > + }
> > +
> > + SVN_ERR(svn_client__open_ra_session_internal(&ra_session, entry->url,
> > + NULL, NULL, NULL, FALSE,
> > + TRUE, ctx, pool));
>
> Seems a bit unnecesary to open an RA session if this just retrieves the
> merge info for BASE or WORKING.
If the merge info for BASE is inherited, it will need to be retrieved
from the repository.
If the merge info for BASE or WORKING stored locally in the WC, the
opening an RA session is indeed unnecessary. I will refactor
get_wc_or_repos_merge_info() to open the RA session on demand, if one
was not provided.
> > + SVN_ERR(svn_client__get_revision_number(&rev, ra_session, revision,
> > + "", pool));
> > +
> > + if (svn_path_is_url(path_or_url))
> > + {
> > + char *repos_rel_path;
> > + SVN_ERR(svn_client__path_relative_to_root(&repos_rel_path, url, NULL,
> > + ra_session, NULL, pool));
> > + SVN_ERR(svn_client__get_repos_merge_info(ra_session, mergeinfo,
> > + repos_rel_path, rev, pool));
> > + }
> > + else
> > + {
> > + svn_boolean_t inherited;
> > + SVN_ERR(get_wc_or_repos_merge_info(mergeinfo, &entry, &inherited,
> > + ra_session, path_or_url, adm_access,
> > + ctx, pool));
>
> Hmmm, so if a WC path is specified, the revision is ignored... That's a bit
> unintuitive to me and not documented.
I've documented the behavior. I agree that it's a bit unintuitive,
but found the convenience of the API a compelling enough trade-off.
Do you have an alternate suggestion?
- application/pgp-signature attachment: stored
Received on Mon Apr 2 21:44:42 2007