You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2007/11/12 18:12:11 UTC
Re: svn commit: r27710 - trunk/subversion/libsvn_client
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);
> +
> + /* If we have no RA session yet, open one. */
> + if (! session)
> + {
> + sesspool = svn_pool_create(pool);
> + SVN_ERR(svn_client__open_ra_session_internal(&session, url,
> + NULL, NULL, NULL, FALSE,
> + TRUE, ctx, sesspool));
> + }
> +
> + /* If we haven't resolved for ourselves a numeric peg revision, do so. */
> + if (! SVN_IS_VALID_REVNUM(peg_revnum))
> + SVN_ERR(svn_client__get_revision_number(&peg_revnum, NULL, session,
> + peg_revision, NULL, pool));
> +
> + /* Fetch the location segments for our URL@PEG_REVNUM. */
> + SVN_ERR(svn_client__repos_location_segments(&segments, session, "",
> + peg_revnum, peg_revnum, 0,
> + ctx, pool));
> +
> + /* Translate location segments into merge sources and ranges. */
> + for (i = 0; i < segments->nelts; i++)
> + {
> + svn_location_segment_t *segment =
> + APR_ARRAY_IDX(segments, i, svn_location_segment_t *);
> + apr_array_header_t *path_ranges;
> + svn_merge_range_t *range;
> + const char *source_path;
> +
> + /* No path segment? Skip it. */
> + if (! segment->path)
> + continue;
> +
> + /* Prepend a leading slash to our path. */
> + source_path = apr_pstrcat(pool, "/", segment->path, NULL);
> +
> + /* See if we already stored ranges for this path. If not, make
> + a new list. */
> + path_ranges = apr_hash_get(mergeinfo, source_path, APR_HASH_KEY_STRING);
> + if (! path_ranges)
> + path_ranges = apr_array_make(pool, 1, sizeof(range));
> +
> + /* Build a merge range, push it onto the list of ranges, and for
> + good measure, (re)store it in the hash. */
> + range = apr_pcalloc(pool, sizeof(*range));
> + range->start = MAX(segment->range_start - 1, 0);
> + range->end = segment->range_end;
> + range->inheritable = TRUE;
> + APR_ARRAY_PUSH(path_ranges, svn_merge_range_t *) = range;
> + apr_hash_set(mergeinfo, source_path, APR_HASH_KEY_STRING, path_ranges);
> + }
> +
> + /* If we opened an RA session, ensure its closure. */
> + if (sesspool)
> + svn_pool_destroy(sesspool);
> +
> + *mergeinfo_p = mergeinfo;
> + return SVN_NO_ERROR;
> +}
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.
Re: svn commit: r27710 - trunk/subversion/libsvn_client
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 12 Nov 2007, C. Michael Pilato wrote:
> 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.
I'm attaching a patch-in-progress that implements this. Unfortunately
I managed to inject a few test failures while adding that optimization
we discussed on IRC (had a clean test run before that, bleh).
Re: svn commit: r27710 - trunk/subversion/libsvn_client
Posted by "C. Michael Pilato" <cm...@collab.net>.
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 <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand