Re: svn commit: r27710 - trunk/subversion/libsvn_client

Daniel Rall wrote:
> On Thu, 08 Nov 2007, cmpilato@tigris.org wrote:
> ...
>> New client library helper function, svn_client__get_implicit_mergeinfo().
>>
>> * subversion/libsvn_client/mergeinfo.h,
>> * subversion/libsvn_client/mergeinfo.c
>> (svn_client__get_implicit_mergeinfo): New function.
>>
>> Modified:
>> trunk/subversion/libsvn_client/mergeinfo.c
>> trunk/subversion/libsvn_client/mergeinfo.h
>>
>> Modified: trunk/subversion/libsvn_client/mergeinfo.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/mergeinfo.c?pathrev=27710&r1=27709&r2=27710
>> ==============================================================================
>> --- trunk/subversion/libsvn_client/mergeinfo.c (original)
>> +++ trunk/subversion/libsvn_client/mergeinfo.c Thu Nov 8 07:25:48 2007
>> @@ -400,6 +400,129 @@
>> return SVN_NO_ERROR;
>> }
>>
>> +
>> +svn_error_t *
>> +svn_client__get_implicit_mergeinfo(apr_hash_t **mergeinfo_p,
>> + const char *path_or_url,
>> + const svn_opt_revision_t *peg_revision,
>> + svn_ra_session_t *ra_session,
>> + svn_wc_adm_access_t *adm_access,
>> + svn_client_ctx_t *ctx,
>> + apr_pool_t *pool)
>> +{
>> + apr_array_header_t *segments;
>> + svn_revnum_t peg_revnum = SVN_INVALID_REVNUM;
>> + svn_ra_session_t *session = ra_session;
>> + const char *url;
>> + apr_hash_t *mergeinfo = apr_hash_make(pool);
>> + apr_pool_t *sesspool = NULL; /* only used for an RA session we open */
>> + int i;
>> +
>> + /* If PATH_OR_URL is a local path (not a URL), we need to transform
>> + it into a URL, open an RA session for it, and resolve the peg
>> + revision. Note that if the local item is scheduled for addition
>> + as a copy of something else, we'll use its copyfrom data to query
>> + its history. */
>> + if (! svn_path_is_url(path_or_url))
>> + {
>> + const svn_wc_entry_t *entry;
>> +
>> + ra_session = NULL;
>> + if (adm_access)
>> + {
>> + SVN_ERR(svn_wc_entry(&entry, path_or_url, adm_access, FALSE, pool));
>> + }
>> + 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(&entry, path_or_url, adm_access, FALSE, pool));
>> + SVN_ERR(svn_wc_adm_close(adm_access));
>> + }
>> +
>> + if (entry->copyfrom_url
>> + && peg_revision->kind == svn_opt_revision_working)
>> + {
>> + url = entry->copyfrom_url;
>> + peg_revnum = entry->copyfrom_rev;
>> + }
>> + else if (entry->url)
>> + {
>> + url = entry->url;
>> + }
>> + else
>> + {
>> + return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
>> + _("'%s' has no URL"),
>> + svn_path_local_style(path_or_url, pool));
>> + }
>> + }
>> + else
>> + {
>> + url = path_or_url;
>> + }
>
> I feel like I've written the above code ten times. I can't put my
> finger on exactly what it should look like, but I get the feeling we
> should be using a shared routine for this...
>
> How about something like:
>
> /* Set *URL to the repository URL of PATH_OR_URL. ADM_ACCESS may be
> NULL, if PATH_OR_URL is a URL. Use POOL for all allocations. */
> svn_error_t *
> svn_client__derive_url(const char **url,
> const char *path_or_url,
> svn_wc_adm_access_t *adm_access,
> apr_pool_t *pool);

I'm all for it. Even had a patch for it at one point (that got lost in a
code scramble). I think that we need a flag to say how to deal with
schedule-add-with-history items (do we want the target URL or the source
copyfrom URL?), but other than that, +1 on the idea of such a helper function.

> The remaining part of this function definitely has some overlap with
> code I've seen or written in libsvn_client's copy.c, merge.c, and/or
> mergeinfo.c.

Sorta kinda. I'm busy right now working through those issues.

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand