Author: pburba Date: Wed May 1 22:27:31 2013 New Revision: 1478220 URL: http://svn.apache.org/r1478220 Log: Fix (hopefully for the last time) issue #4355 'svn_client_log5 broken with multiple revisions which span a rename' and refactor the aforementioned API, which was a bit of an unwieldy beast.
* subversion/include/private/svn_client_private.h (svn_client__resolve_rev_and_url): "New", but actually just the old libsvn_client/ra.c:resolve_rev_and_url(). * subversion/libsvn_client/log.c (assert.h): New include. (resolve_log_targets, find_youngest_and_oldest_revs, resolve_wc_opt_revs, resolve_wc_head_revs, resolve_wc_date_revs, rev_range_t, convert_opt_rev_array_to_rev_range_array, compare_rev_to_segment, run_ra_get_log): New. (svn_client_log5): Refactor this into the new helpers above. Beyond the simple refactoring, this function now uses svn_client__repos_location_segments() to get the history spanning the requested log revisions and then uses this history to handle issue #4355. Credit cmpilato and rhhijben with this idea, see http://svn.haxx.se/dev/archive-2013-04/0474.shtml * subversion/libsvn_client/ra.c (resolve_rev_and_url): Convert from static to svn_client private function, renaming from this to... (svn_client__resolve_rev_and_url): ...this. * subversion/tests/cmdline/authz_tests.py (authz_log_and_tracing_test): Tweak expected error to account for new svn_client_log5 implementation. * subversion/tests/cmdline/log_tests.py (log_multiple_revs_spanning_rename): Remove XFail decorator. Tweak comments re failure status. Add a required 'svn update' for testing log with WC target. Modified: subversion/trunk/subversion/include/private/svn_client_private.h subversion/trunk/subversion/libsvn_client/log.c subversion/trunk/subversion/libsvn_client/ra.c subversion/trunk/subversion/tests/cmdline/authz_tests.py subversion/trunk/subversion/tests/cmdline/log_tests.py Modified: subversion/trunk/subversion/include/private/svn_client_private.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1478220&r1=1478219&r2=1478220&view=diff ============================================================================== --- subversion/trunk/subversion/include/private/svn_client_private.h (original) +++ subversion/trunk/subversion/include/private/svn_client_private.h Wed May 1 22:27:31 2013 @@ -147,6 +147,37 @@ svn_client__ra_session_from_path2(svn_ra svn_client_ctx_t *ctx, apr_pool_t *pool); +/* Given PATH_OR_URL, which contains either a working copy path or an + absolute URL, a peg revision PEG_REVISION, and a desired revision + REVISION, find the path at which that object exists in REVISION, + following copy history if necessary. If REVISION is younger than + PEG_REVISION, then check that PATH_OR_URL is the same node in both + PEG_REVISION and REVISION, and return @c + SVN_ERR_CLIENT_UNRELATED_RESOURCES if it is not the same node. + + If PEG_REVISION->kind is 'unspecified', the peg revision is 'head' + for a URL or 'working' for a WC path. If REVISION->kind is + 'unspecified', the operative revision is the peg revision. + + Store the actual location of the object in *RESOLVED_LOC_P. + + RA_SESSION should be an open RA session pointing at the URL of + PATH_OR_URL, or NULL, in which case this function will open its own + temporary session. + + Use authentication baton cached in CTX to authenticate against the + repository. + + Use POOL for all allocations. */ +svn_error_t * +svn_client__resolve_rev_and_url(svn_client__pathrev_t **resolved_loc_p, + svn_ra_session_t *ra_session, + const char *path_or_url, + const svn_opt_revision_t *peg_revision, + const svn_opt_revision_t *revision, + svn_client_ctx_t *ctx, + apr_pool_t *pool); + /** Return @c SVN_ERR_ILLEGAL_TARGET if TARGETS contains a mixture of * URLs and paths; otherwise return SVN_NO_ERROR. * Modified: subversion/trunk/subversion/libsvn_client/log.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/log.c?rev=1478220&r1=1478219&r2=1478220&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_client/log.c (original) +++ subversion/trunk/subversion/libsvn_client/log.c Wed May 1 22:27:31 2013 @@ -42,6 +42,7 @@ #include "svn_private_config.h" #include "private/svn_wc_private.h" +#include <assert.h> /*** Getting misc. information ***/ @@ -260,57 +261,336 @@ limit_receiver(void *baton, svn_log_entr return rb->receiver(rb->baton, log_entry, pool); } - -/*** Public Interface. ***/ +/* Resolve the URLs or WC path in TARGETS as per the svn_client_log5 API. + The limitations on TARGETS specified by svn_client_log5 are enforced here. + So TARGETS can only contain a single WC path or a URL and zero or more + relative paths -- anything else will raise an error. -svn_error_t * -svn_client_log5(const apr_array_header_t *targets, - const svn_opt_revision_t *peg_revision, - const apr_array_header_t *revision_ranges, - int limit, - svn_boolean_t discover_changed_paths, - svn_boolean_t strict_node_history, - svn_boolean_t include_merged_revisions, - const apr_array_header_t *revprops, - svn_log_entry_receiver_t real_receiver, - void *real_receiver_baton, - svn_client_ctx_t *ctx, - apr_pool_t *pool) + PEG_REVISION, TARGETS, and CTX are as per svn_client_log5. + + If TARGETS contains a single WC path then set *RA_TARGET to the absolute + path of that single path if PEG_REVISION is dependent on the working copy + (e.g. PREV). Otherwise set *RA_TARGET to the corresponding URL for the + single WC path. Set *RELATIVE_TARGETS to an array with a single + element "". + + If TARGETS contains only a single URL, then set *RA_TARGET to a copy of + that URL and *RELATIVE_TARGETS to an array with a single element "". + + If TARGETS contains a single URL and one or more relative paths, then + set *RA_TARGET to a copy of that URL and *CONDENSED_PATHS to a copy of + each relative path after the URL. + + If *PEG_REVISION is svn_opt_revision_unspecified, then *PEG_REVISION is + set to svn_opt_revision_head for URLs or svn_opt_revision_working for a + WC path. + + *RA_TARGET and *RELATIVE_TARGETS are allocated in RESULT_POOL. */ +static svn_error_t * +resolve_log_targets(apr_array_header_t **relative_targets, + const char **ra_target, + svn_opt_revision_t *peg_revision, + const apr_array_header_t *targets, + svn_client_ctx_t *ctx, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) { - svn_ra_session_t *ra_session; - const char *url_or_path; - svn_boolean_t has_log_revprops; - apr_array_header_t *condensed_targets; - svn_opt_revision_t session_opt_rev; - const char *ra_target; - pre_15_receiver_baton_t rb = {0}; - apr_pool_t *iterpool; int i; - svn_opt_revision_t peg_rev; - svn_boolean_t url_targets = FALSE; + svn_boolean_t url_targets; + + /* Per svn_client_log5, TARGETS contains either a URL followed by zero or + more relative paths, or one working copy path. */ + const char *url_or_path = APR_ARRAY_IDX(targets, 0, const char *); + + /* svn_client_log5 requires at least one target. */ + if (targets->nelts == 0) + return svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL, + _("No valid target found")); - if (revision_ranges->nelts == 0) + /* Initialize the output array. */ + *relative_targets = apr_array_make(result_pool, 1, sizeof(const char *)); + + if (svn_path_is_url(url_or_path)) { - return svn_error_create - (SVN_ERR_CLIENT_BAD_REVISION, NULL, - _("Missing required revision specification")); + /* An unspecified PEG_REVISION for a URL path defaults + to svn_opt_revision_head. */ + if (peg_revision->kind == svn_opt_revision_unspecified) + (*peg_revision).kind = svn_opt_revision_head; + + /* The logic here is this: If we get passed one argument, we assume + it is the full URL to a file/dir we want log info for. If we get + a URL plus some paths, then we assume that the URL is the base, + and that the paths passed are relative to it. */ + if (targets->nelts > 1) + { + /* We have some paths, let's use them. Start after the URL. */ + for (i = 1; i < targets->nelts; i++) + { + const char *target; + + target = APR_ARRAY_IDX(targets, i, const char *); + + if (svn_path_is_url(target) || svn_dirent_is_absolute(target)) + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, + _("'%s' is not a relative path"), + target); + + APR_ARRAY_PUSH(*relative_targets, const char *) = + apr_pstrdup(result_pool, target); + } + } + else + { + /* If we have a single URL, then the session will be rooted at + it, so just send an empty string for the paths we are + interested in. */ + APR_ARRAY_PUSH(*relative_targets, const char *) = ""; + } + + /* Remember that our targets are URLs. */ + url_targets = TRUE; } + else /* WC path target. */ + { + const char *target; + const char *target_abspath; - /* Make a copy of PEG_REVISION, we may need to change it to a - default value. */ - peg_rev = *peg_revision; + url_targets = FALSE; + if (targets->nelts > 1) + return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, + _("When specifying working copy paths, only " + "one target may be given")); - /* Use the passed URL, if there is one. */ - url_or_path = APR_ARRAY_IDX(targets, 0, const char *); - session_opt_rev.kind = svn_opt_revision_unspecified; + /* An unspecified PEG_REVISION for a working copy path defaults + to svn_opt_revision_working. */ + if (peg_revision->kind == svn_opt_revision_unspecified) + (*peg_revision).kind = svn_opt_revision_working; - for (i = 0; i < revision_ranges->nelts; i++) + /* Get URLs for each target */ + target = APR_ARRAY_IDX(targets, 0, const char *); + + SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, scratch_pool)); + SVN_ERR(svn_wc__node_get_url(&url_or_path, ctx->wc_ctx, target_abspath, + scratch_pool, scratch_pool)); + APR_ARRAY_PUSH(*relative_targets, const char *) = ""; + } + + /* If this is a revision type that requires access to the working copy, + * we use our initial target path to figure out where to root the RA + * session, otherwise we use our URL. */ + if (SVN_CLIENT__REVKIND_NEEDS_WC(peg_revision->kind)) + { + if (url_targets) + return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL, + _("PREV, BASE, or COMMITTED revision " + "keywords are invalid for URL")); + + else + SVN_ERR(svn_dirent_get_absolute( + ra_target, APR_ARRAY_IDX(targets, 0, const char *), result_pool)); + } + else + { + *ra_target = apr_pstrdup(result_pool, url_or_path); + } + + return SVN_NO_ERROR; +} + +/* Keep track of oldest and youngest opt revs found. + + If REV is younger than *YOUNGEST_REV, or *YOUNGEST_REV is + svn_opt_revision_unspecified, then set *YOUNGEST_REV equal to REV. + + If REV is older than *OLDEST_REV, or *OLDEST_REV is + svn_opt_revision_unspecified, then set *OLDEST_REV equal to REV. */ +static void +find_youngest_and_oldest_revs(svn_revnum_t *youngest_rev, + svn_revnum_t *oldest_rev, + svn_revnum_t rev) +{ + /* Is REV younger than YOUNGEST_REV? */ + if (! SVN_IS_VALID_REVNUM(*youngest_rev) + || rev > *youngest_rev) + *youngest_rev = rev; + + if (! SVN_IS_VALID_REVNUM(*oldest_rev) + || rev < *oldest_rev) + *oldest_rev = rev; + + return; +} + +/* If OPT_REV is of a revision kind that is dependent on the WC then query + WC to determine the equivalent svn_revnum_t and set *OPT_REV to a + svn_opt_revision_number kind reflecting that revnum. */ +static svn_error_t * +resolve_wc_opt_revs(svn_opt_revision_t *opt_rev, + const char *abspath, + svn_client_ctx_t *ctx, + apr_pool_t *scratch_pool) +{ + if (SVN_CLIENT__REVKIND_NEEDS_WC(opt_rev->kind)) + { + svn_revnum_t range_start_as_revnum;// = 0; + + SVN_ERR(svn_client__get_revision_number(&range_start_as_revnum, + NULL, ctx->wc_ctx, abspath, + NULL,/* No session needed */ + opt_rev, scratch_pool)); + opt_rev->kind = svn_opt_revision_number; + opt_rev->value.number = range_start_as_revnum; + } + + return SVN_NO_ERROR; +} + +/* Resolve the head revision. + + If OPT_REV is not of svn_opt_revision_head kind, then do nothing. + + Otherwise if *YOUNGEST_REV is valid, then set set *OPT_REV to a + svn_opt_revision_number kind reflecting the value of *YOUNGEST_REV. + Else contact the repository to determine the head svn_revnum_t and + set *OPT_REV to a svn_opt_revision_number kind reflecting that revnum. + + RA_SESSION, RESOLVED_LOC_P, URL_OR_ABSPATH, and PEG_REV are all as per the + parameters of the same name in convert_opt_rev_array_to_rev_range_array(). +*/ +static svn_error_t * +resolve_wc_head_revs(svn_opt_revision_t *opt_rev, + svn_ra_session_t **ra_session, + svn_client__pathrev_t **resolved_loc_p, + svn_revnum_t *head_rev, + const char *url_or_abspath, + const svn_opt_revision_t *peg_rev, + svn_client_ctx_t *ctx, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + if (opt_rev->kind == svn_opt_revision_head) + { + svn_opt_revision_t head_opt_rev; + + head_opt_rev.kind = svn_opt_revision_head; + + if (*ra_session == NULL) + SVN_ERR(svn_client__ra_session_from_path2(ra_session, + resolved_loc_p, + url_or_abspath, NULL, + peg_rev, &head_opt_rev, + ctx, result_pool)); + + /* Find HEAD rev, but only do it once! */ + if (!SVN_IS_VALID_REVNUM(*head_rev)) + SVN_ERR(svn_ra_get_latest_revnum(*ra_session, head_rev, + scratch_pool)); + + opt_rev->kind = svn_opt_revision_number; + opt_rev->value.number = *head_rev; + } + + return SVN_NO_ERROR; +} + +/* If OPT_REV is of svn_opt_revision_date kind, then contact the repository + to determine the equivalent svn_revnum_t and set *OPT_REV to a + svn_opt_revision_number kind reflecting that revnum. + + RA_SESSION, RESOLVED_LOC_P, URL_OR_ABSPATH, and PEG_REV are all as per the + parameters of the same name in convert_opt_rev_array_to_rev_range_array(). +*/ +static svn_error_t * +resolve_wc_date_revs(svn_opt_revision_t *opt_rev, + svn_ra_session_t **ra_session, + svn_client__pathrev_t **resolved_loc_p, + const char *url_or_abspath, + const svn_opt_revision_t *peg_rev, + svn_client_ctx_t *ctx, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + if (opt_rev->kind == svn_opt_revision_date) + { + svn_revnum_t date_as_revnum; + + if (*ra_session == NULL) + SVN_ERR(svn_client__ra_session_from_path2(ra_session, + resolved_loc_p, + url_or_abspath, NULL, + peg_rev, opt_rev, + ctx, result_pool)); + + SVN_ERR(svn_ra_get_dated_revision(*ra_session, &date_as_revnum, + opt_rev->value.date, scratch_pool)); + opt_rev->kind = svn_opt_revision_number; + opt_rev->value.number = date_as_revnum; + } + + return SVN_NO_ERROR; +} + +typedef struct rev_range_t +{ + svn_revnum_t range_start; + svn_revnum_t range_end; +} rev_range_t; + +/* Convert array of svn_opt_revision_t ranges to an array of svn_revnum_t + ranges. + + Given a log target URL_OR_ABSPATH@PEG_REV and an array of + svn_opt_revision_range_t's OPT_REV_RANGES, resolve the opt revs in + OPT_REV_RANGES to svn_revnum_t's and return these in *REVISION_RANGES, an + array of rev_range_t *. + + Set *YOUNGEST_REV and *OLDEST_REV to the youngest and oldest revisions + found in *REVISION_RANGES. + + If the repository needs to be contacted to resolve svn_opt_revision_date or + svn_opt_revision_head revisions, then the session used to do this is + returned in RA_SESSION and the resolved location used to open the session + is returned in *RESOLVED_LOC_P (see parameter of the same name in + svn_client__ra_session_from_path2). If the repository is not contacted, + then set *RA_SESSION and *RESOLVED_LOC_P to NULL. */ +static svn_error_t* +convert_opt_rev_array_to_rev_range_array( + apr_array_header_t **revision_ranges, + svn_ra_session_t **ra_session, + svn_client__pathrev_t **resolved_loc_p, + svn_revnum_t *youngest_rev, + svn_revnum_t *oldest_rev, + const char *url_or_abspath, + const apr_array_header_t *opt_rev_ranges, + const svn_opt_revision_t *peg_rev, + svn_client_ctx_t *ctx, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + int i; + + /* Initialize the input/output parameters. */ + *youngest_rev = *oldest_rev = SVN_INVALID_REVNUM; + *ra_session = NULL; + *resolved_loc_p = NULL; + + /* Convert OPT_REV_RANGES to an array of rev_range_t and find the youngest + and oldest revision range that spans all of OPT_REV_RANGES. */ + *revision_ranges = apr_array_make(result_pool, opt_rev_ranges->nelts, + sizeof(rev_range_t *)); + + for (i = 0; i < opt_rev_ranges->nelts; i++) { svn_opt_revision_range_t *range; + rev_range_t *rev_range; + svn_boolean_t start_same_as_end = FALSE; - range = APR_ARRAY_IDX(revision_ranges, i, svn_opt_revision_range_t *); + range = APR_ARRAY_IDX(opt_rev_ranges, i, svn_opt_revision_range_t *); + /* Right now RANGE can be any valid pair of svn_opt_revision_t's. We + will now convert all RANGEs in place to the corresponding + svn_opt_revision_number kind. */ if ((range->start.kind != svn_opt_revision_unspecified) && (range->end.kind == svn_opt_revision_unspecified)) { @@ -329,15 +609,15 @@ svn_client_log5(const apr_array_header_t /* Default to any specified peg revision. Otherwise, if the * first target is a URL, then we default to HEAD:0. Lastly, * the default is BASE:0 since WC@HEAD may not exist. */ - if (peg_rev.kind == svn_opt_revision_unspecified) + if (peg_rev->kind == svn_opt_revision_unspecified) { - if (svn_path_is_url(url_or_path)) + if (svn_path_is_url(url_or_abspath)) range->start.kind = svn_opt_revision_head; else range->start.kind = svn_opt_revision_base; } else - range->start = peg_rev; + range->start = *peg_rev; if (range->end.kind == svn_opt_revision_unspecified) { @@ -354,163 +634,128 @@ svn_client_log5(const apr_array_header_t _("Missing required revision specification")); } - /* Determine the revision to open the RA session to. */ - if (session_opt_rev.kind == svn_opt_revision_unspecified) - { - if (range->start.kind == svn_opt_revision_number && - range->end.kind == svn_opt_revision_number) - { - session_opt_rev = - (range->start.value.number > range->end.value.number ? - range->start : range->end); - } - else if (range->start.kind == svn_opt_revision_head || - range->end.kind == svn_opt_revision_head) - { - session_opt_rev.kind = svn_opt_revision_head; - } - else if (range->start.kind == svn_opt_revision_date && - range->end.kind == svn_opt_revision_date) - { - session_opt_rev = - (range->start.value.date > range->end.value.date ? - range->start : range->end); - } - } - } - - /* Use the passed URL, if there is one. */ - if (svn_path_is_url(url_or_path)) - { - /* Initialize this array, since we'll be building it below */ - condensed_targets = apr_array_make(pool, 1, sizeof(const char *)); - - /* The logic here is this: If we get passed one argument, we assume - it is the full URL to a file/dir we want log info for. If we get - a URL plus some paths, then we assume that the URL is the base, - and that the paths passed are relative to it. */ - if (targets->nelts > 1) - { - /* We have some paths, let's use them. Start after the URL. */ - for (i = 1; i < targets->nelts; i++) - { - const char *target; - - target = APR_ARRAY_IDX(targets, i, const char *); - - if (svn_path_is_url(target) || svn_dirent_is_absolute(target)) - return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, - _("'%s' is not a relative path"), - target); - - APR_ARRAY_PUSH(condensed_targets, const char *) = target; - } - } + if (memcmp(&(range->start), &(range->end), + sizeof(svn_opt_revision_t)) == 0) + start_same_as_end = TRUE; + + /* If the caller asked for COMMITTED, PREVIOUS, BASE, or WORKING then + resolve those to svn_opt_revision_number opt revs. */ + SVN_ERR(resolve_wc_opt_revs(&(range->start), url_or_abspath, ctx, + scratch_pool)); + if (start_same_as_end) + range->end = range->start; else - { - /* If we have a single URL, then the session will be rooted at - it, so just send an empty string for the paths we are - interested in. */ - APR_ARRAY_PUSH(condensed_targets, const char *) = ""; - } + SVN_ERR(resolve_wc_opt_revs(&(range->end), url_or_abspath, ctx, + scratch_pool)); - /* Remember that our targets are URLs. */ - url_targets = TRUE; + /* If the caller asked about the head revision we need to resolve that + to a revision number and possibly open a RA session. */ + SVN_ERR(resolve_wc_head_revs(&(range->start), ra_session, + resolved_loc_p, youngest_rev, + url_or_abspath, peg_rev, ctx, + result_pool, scratch_pool)); + if (start_same_as_end) + range->end = range->start; + else + SVN_ERR(resolve_wc_head_revs(&(range->end), ra_session, + resolved_loc_p, youngest_rev, + url_or_abspath, peg_rev, ctx, + result_pool, scratch_pool)); + + /* If the caller gave us dates we need to resolve them to revision + numbers and possibly open a RA session. */ + SVN_ERR(resolve_wc_date_revs(&(range->start), ra_session, + resolved_loc_p, url_or_abspath, peg_rev, + ctx, result_pool, scratch_pool)); + if (start_same_as_end) + range->end = range->start; + else + SVN_ERR(resolve_wc_date_revs(&(range->end), ra_session, + resolved_loc_p, url_or_abspath, peg_rev, + ctx, result_pool, scratch_pool)); + + /* Possibly update the oldest and youngest revisions requested. */ + find_youngest_and_oldest_revs(youngest_rev, + oldest_rev, + range->start.value.number); + find_youngest_and_oldest_revs(youngest_rev, + oldest_rev, + range->end.value.number); + rev_range = apr_palloc(result_pool, sizeof(*rev_range)); + rev_range->range_start = range->start.value.number; + rev_range->range_end = range->end.value.number; + APR_ARRAY_PUSH(*revision_ranges, rev_range_t *) = rev_range; } - else - { - apr_array_header_t *target_urls; - apr_array_header_t *real_targets; - /* See FIXME about multiple wc targets, below. */ - if (targets->nelts > 1) - return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, - _("When specifying working copy paths, only " - "one target may be given")); + return SVN_NO_ERROR; +} - /* An unspecified PEG_REVISION for a working copy path defaults - to svn_opt_revision_working. */ - if (peg_rev.kind == svn_opt_revision_unspecified) - peg_rev.kind = svn_opt_revision_working; +static int +compare_rev_to_segment(const void *key_p, + const void *element_p) +{ + svn_revnum_t rev = + * (svn_revnum_t *)key_p; + const svn_location_segment_t *segment = + *((const svn_location_segment_t * const *) element_p); + + if (rev < segment->range_start) + return -1; + else if (rev > segment->range_end) + return 1; + else + return 0; +} - /* Get URLs for each target */ - target_urls = apr_array_make(pool, 1, sizeof(const char *)); - real_targets = apr_array_make(pool, 1, sizeof(const char *)); - iterpool = svn_pool_create(pool); - for (i = 0; i < targets->nelts; i++) - { - const char *url; - const char *target = APR_ARRAY_IDX(targets, i, const char *); - const char *target_abspath; - - svn_pool_clear(iterpool); - SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, iterpool)); - SVN_ERR(svn_wc__node_get_url(&url, ctx->wc_ctx, target_abspath, - pool, iterpool)); - - if (! url) - return svn_error_createf - (SVN_ERR_ENTRY_MISSING_URL, NULL, - _("Entry '%s' has no URL"), - svn_dirent_local_style(target, pool)); - - APR_ARRAY_PUSH(target_urls, const char *) = url; - APR_ARRAY_PUSH(real_targets, const char *) = target; - } - - /* if we have no valid target_urls, just exit. */ - if (target_urls->nelts == 0) - return SVN_NO_ERROR; - - /* Find the base URL and condensed targets relative to it. */ - SVN_ERR(svn_uri_condense_targets(&url_or_path, &condensed_targets, - target_urls, TRUE, pool, iterpool)); - - if (condensed_targets->nelts == 0) - APR_ARRAY_PUSH(condensed_targets, const char *) = ""; - - /* 'targets' now becomes 'real_targets', which has bogus, - unversioned things removed from it. */ - targets = real_targets; - svn_pool_destroy(iterpool); - } - - - { - svn_client__pathrev_t *actual_loc; - - /* If this is a revision type that requires access to the working copy, - * we use our initial target path to figure out where to root the RA - * session, otherwise we use our URL. */ - if (SVN_CLIENT__REVKIND_NEEDS_WC(peg_rev.kind)) - { - if (url_targets) - SVN_ERR(svn_uri_condense_targets(&ra_target, NULL, targets, - TRUE, pool, pool)); - else - SVN_ERR(svn_dirent_condense_targets(&ra_target, NULL, targets, - TRUE, pool, pool)); - } - else - ra_target = url_or_path; - - SVN_ERR(svn_client__ra_session_from_path2(&ra_session, &actual_loc, - ra_target, NULL, - &peg_rev, &session_opt_rev, - ctx, pool)); +/* Run svn_ra_get_log2 for PATHS, one or more paths relative to RA_SESSION's + common parent, for each revision in REVISION_RANGES, an array of + rev_range_t. + + RA_SESSION is an open session pointing to ACTUAL_LOC. + + LOG_SEGMENTS is an array of svn_location_segment_t * items representing the + history of PATHS from the oldest to youngest revisions found in + REVISION_RANGES. + + The TARGETS, LIMIT, DISCOVER_CHANGED_PATHS, STRICT_NODE_HISTORY, + INCLUDE_MERGED_REVISIONS, REVPROPS, REAL_RECEIVER, and REAL_RECEIVER_BATON + parameters are all as per the svn_client_log5 API. */ +static svn_error_t * +run_ra_get_log(apr_array_header_t *revision_ranges, + apr_array_header_t *paths, + apr_array_header_t *log_segments, + svn_client__pathrev_t *actual_loc, + svn_ra_session_t *ra_session, + /* The following are as per svn_client_log5. */ + const apr_array_header_t *targets, + int limit, + svn_boolean_t discover_changed_paths, + svn_boolean_t strict_node_history, + svn_boolean_t include_merged_revisions, + const apr_array_header_t *revprops, + svn_log_entry_receiver_t real_receiver, + void *real_receiver_baton, + svn_client_ctx_t *ctx, + apr_pool_t *scratch_pool) +{ + int i; + pre_15_receiver_baton_t rb = {0}; + apr_pool_t *iterpool; + svn_boolean_t has_log_revprops; - SVN_ERR(svn_ra_has_capability(ra_session, &has_log_revprops, - SVN_RA_CAPABILITY_LOG_REVPROPS, pool)); + SVN_ERR(svn_ra_has_capability(ra_session, &has_log_revprops, + SVN_RA_CAPABILITY_LOG_REVPROPS, + scratch_pool)); - if (!has_log_revprops) { + if (!has_log_revprops) + { /* See above pre-1.5 notes. */ rb.ctx = ctx; /* Create ra session on first use */ - rb.ra_session_pool = pool; + rb.ra_session_pool = scratch_pool; rb.ra_session_url = actual_loc->url; } - } /* It's a bit complex to correctly handle the special revision words * such as "BASE", "COMMITTED", and "PREV". For example, if the @@ -559,35 +804,55 @@ svn_client_log5(const apr_array_header_t * epg wonders if the repository could send a unified stream of log * entries if the paths and revisions were passed down. */ - iterpool = svn_pool_create(pool); + iterpool = svn_pool_create(scratch_pool); for (i = 0; i < revision_ranges->nelts; i++) { - svn_revnum_t start_revnum, end_revnum, youngest_rev = SVN_INVALID_REVNUM; + const char *old_session_url; const char *path = APR_ARRAY_IDX(targets, 0, const char *); const char *local_abspath_or_url; - svn_opt_revision_range_t *range; + const char *segment_url = actual_loc->url; + rev_range_t *range; limit_receiver_baton_t lb; svn_log_entry_receiver_t passed_receiver; void *passed_receiver_baton; const apr_array_header_t *passed_receiver_revprops; + svn_location_segment_t **matching_segment; + svn_revnum_t younger_rev; svn_pool_clear(iterpool); if (!svn_path_is_url(path)) - SVN_ERR(svn_dirent_get_absolute(&local_abspath_or_url, path, iterpool)); + SVN_ERR(svn_dirent_get_absolute(&local_abspath_or_url, path, + iterpool)); else local_abspath_or_url = path; - range = APR_ARRAY_IDX(revision_ranges, i, svn_opt_revision_range_t *); + range = APR_ARRAY_IDX(revision_ranges, i, rev_range_t *); - SVN_ERR(svn_client__get_revision_number(&start_revnum, &youngest_rev, - ctx->wc_ctx, local_abspath_or_url, - ra_session, &range->start, - iterpool)); - SVN_ERR(svn_client__get_revision_number(&end_revnum, &youngest_rev, - ctx->wc_ctx, local_abspath_or_url, - ra_session, &range->end, - iterpool)); + /* Issue #4355: Account for renames spanning requested + revision ranges. */ + younger_rev = MAX(range->range_start, range->range_end); + matching_segment = bsearch(&younger_rev, log_segments->elts, + log_segments->nelts, log_segments->elt_size, + compare_rev_to_segment); + SVN_ERR_ASSERT(*matching_segment); + + /* A segment with a NULL path means there is gap in the history. + We'll just proceed and let svn_ra_get_log2 fail with a useful + error...*/ + if ((*matching_segment)->path != NULL) + { + /* ...but if there is history, then we must account for issue + #4355 and make sure our RA session is pointing at the correct + location. */ + segment_url = svn_path_url_add_component2( + actual_loc->repos_root_url, (*matching_segment)->path, + scratch_pool); + SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, + ra_session, + segment_url, + scratch_pool)); + } if (has_log_revprops) { @@ -617,9 +882,9 @@ svn_client_log5(const apr_array_header_t } SVN_ERR(svn_ra_get_log2(ra_session, - condensed_targets, - start_revnum, - end_revnum, + paths, + range->range_start, + range->range_end, limit, discover_changed_paths, strict_node_history, @@ -642,3 +907,98 @@ svn_client_log5(const apr_array_header_t return SVN_NO_ERROR; } + +/*** Public Interface. ***/ + +svn_error_t * +svn_client_log5(const apr_array_header_t *targets, + const svn_opt_revision_t *peg_revision, + const apr_array_header_t *opt_rev_ranges, + int limit, + svn_boolean_t discover_changed_paths, + svn_boolean_t strict_node_history, + svn_boolean_t include_merged_revisions, + const apr_array_header_t *revprops, + svn_log_entry_receiver_t real_receiver, + void *real_receiver_baton, + svn_client_ctx_t *ctx, + apr_pool_t *pool) +{ + svn_ra_session_t *ra_session; + const char *url_or_path; + const char *old_session_url; + const char *ra_target; + svn_opt_revision_t youngest_opt_rev; + svn_revnum_t youngest_rev; + svn_revnum_t oldest_rev; + svn_opt_revision_t peg_rev; + svn_client__pathrev_t *actual_loc; + apr_array_header_t *log_segments; + apr_array_header_t *revision_ranges; + apr_array_header_t *relative_targets; + + if (opt_rev_ranges->nelts == 0) + { + return svn_error_create + (SVN_ERR_CLIENT_BAD_REVISION, NULL, + _("Missing required revision specification")); + } + + /* Make a copy of PEG_REVISION, we may need to change it to a + default value. */ + peg_rev = *peg_revision; + + SVN_ERR(resolve_log_targets(&relative_targets, &ra_target, &peg_rev, + targets, ctx, pool, pool)); + + /* Use the passed URL, if there is one. */ + url_or_path = APR_ARRAY_IDX(targets, 0, const char *); + + /* Convert OPT_REV_RANGES to an array of rev_range_t and find the youngest + and oldest revision range that spans all of OPT_REV_RANGES. */ + SVN_ERR(convert_opt_rev_array_to_rev_range_array(&revision_ranges, + &ra_session, &actual_loc, + &youngest_rev, + &oldest_rev, + ra_target, + opt_rev_ranges, &peg_rev, + ctx, pool, pool)); + youngest_opt_rev.kind = svn_opt_revision_number; + youngest_opt_rev.value.number = youngest_rev; + + /* If we didn't already open a RA session above, do so now. */ + if (!ra_session) + { + SVN_ERR(svn_client__ra_session_from_path2(&ra_session, &actual_loc, + ra_target, NULL, + &peg_rev, &youngest_opt_rev, + ctx, pool)); + } + else + { + /* We already opened a session above to resolve dates or head to + revision numbers. So just make sure RA_SESSION points to the + right URL. */ + SVN_ERR(svn_client__resolve_rev_and_url(&actual_loc, ra_session, + ra_target, &peg_rev, + &youngest_opt_rev, ctx, pool)); + SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, ra_session, + actual_loc->url, pool)); + } + + /* Get the svn_location_segment_t's representing the requested log ranges. */ + SVN_ERR(svn_client__repos_location_segments(&log_segments, ra_session, + actual_loc->url, + actual_loc->rev, /* peg */ + actual_loc->rev, /* start */ + oldest_rev, /* end */ + ctx, pool)); + + SVN_ERR(run_ra_get_log(revision_ranges, relative_targets, log_segments, + actual_loc, ra_session, targets, limit, + discover_changed_paths, strict_node_history, + include_merged_revisions, revprops, real_receiver, + real_receiver_baton, ctx, pool)); + + return SVN_NO_ERROR; +} Modified: subversion/trunk/subversion/libsvn_client/ra.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/ra.c?rev=1478220&r1=1478219&r2=1478220&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_client/ra.c (original) +++ subversion/trunk/subversion/libsvn_client/ra.c Wed May 1 22:27:31 2013 @@ -447,39 +447,14 @@ svn_client_open_ra_session2(svn_ra_sessi scratch_pool)); } - - - -/* Given PATH_OR_URL, which contains either a working copy path or an - absolute URL, a peg revision PEG_REVISION, and a desired revision - REVISION, find the path at which that object exists in REVISION, - following copy history if necessary. If REVISION is younger than - PEG_REVISION, then check that PATH_OR_URL is the same node in both - PEG_REVISION and REVISION, and return @c - SVN_ERR_CLIENT_UNRELATED_RESOURCES if it is not the same node. - - If PEG_REVISION->kind is 'unspecified', the peg revision is 'head' - for a URL or 'working' for a WC path. If REVISION->kind is - 'unspecified', the operative revision is the peg revision. - - Store the actual location of the object in *RESOLVED_LOC_P. - - RA_SESSION should be an open RA session pointing at the URL of - PATH_OR_URL, or NULL, in which case this function will open its own - temporary session. - - Use authentication baton cached in CTX to authenticate against the - repository. - - Use POOL for all allocations. */ -static svn_error_t * -resolve_rev_and_url(svn_client__pathrev_t **resolved_loc_p, - svn_ra_session_t *ra_session, - const char *path_or_url, - const svn_opt_revision_t *peg_revision, - const svn_opt_revision_t *revision, - svn_client_ctx_t *ctx, - apr_pool_t *pool) +svn_error_t * +svn_client__resolve_rev_and_url(svn_client__pathrev_t **resolved_loc_p, + svn_ra_session_t *ra_session, + const char *path_or_url, + const svn_opt_revision_t *peg_revision, + const svn_opt_revision_t *revision, + svn_client_ctx_t *ctx, + apr_pool_t *pool) { svn_opt_revision_t peg_rev = *peg_revision; svn_opt_revision_t start_rev = *revision; @@ -545,9 +520,9 @@ svn_client__ra_session_from_path2(svn_ra if (corrected_url && svn_path_is_url(path_or_url)) path_or_url = corrected_url; - SVN_ERR(resolve_rev_and_url(&resolved_loc, ra_session, - path_or_url, peg_revision, revision, - ctx, pool)); + SVN_ERR(svn_client__resolve_rev_and_url(&resolved_loc, ra_session, + path_or_url, peg_revision, revision, + ctx, pool)); /* Make the session point to the real URL. */ SVN_ERR(svn_ra_reparent(ra_session, resolved_loc->url, pool)); @@ -1009,9 +984,9 @@ svn_client__youngest_common_ancestor(con path_or_url1, NULL, revision1, revision1, ctx, sesspool)); - SVN_ERR(resolve_rev_and_url(&loc2, session, - path_or_url2, revision2, revision2, - ctx, scratch_pool)); + SVN_ERR(svn_client__resolve_rev_and_url(&loc2, session, + path_or_url2, revision2, revision2, + ctx, scratch_pool)); SVN_ERR(svn_client__get_youngest_common_ancestor( &ancestor, loc1, loc2, session, ctx, result_pool, scratch_pool)); Modified: subversion/trunk/subversion/tests/cmdline/authz_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/authz_tests.py?rev=1478220&r1=1478219&r2=1478220&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/authz_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/authz_tests.py Wed May 1 22:27:31 2013 @@ -576,7 +576,8 @@ def authz_log_and_tracing_test(sbox): if sbox.repo_url.startswith('http'): expected_err2 = expected_err else: - expected_err2 = ".*svn: E220001: Item is not readable.*" + expected_err2 = ".*svn: E220001: Unreadable path encountered; " \ + "access denied.*" # if we do the same thing directly on the unreadable file, we get: # svn: Item is not readable Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1478220&r1=1478219&r2=1478220&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/log_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Wed May 1 22:27:31 2013 @@ -2369,7 +2369,6 @@ def merge_sensitive_log_with_search(sbox # Test for issue #4355 'svn_client_log5 broken with multiple revisions # which span a rename'. @Issue(4355) -@XFail() @SkipUnless(server_has_mergeinfo) def log_multiple_revs_spanning_rename(sbox): "log for multiple revs which span a rename" @@ -2403,6 +2402,7 @@ def log_multiple_revs_spanning_rename(sb svntest.main.file_write(msg_file, msg) svntest.main.file_append(mu_path2, "4") svntest.main.run_svn(None, 'ci', '-F', msg_file, wc_dir) + svntest.main.run_svn(None, 'up', wc_dir) # Check that log can handle a revision range that spans a rename. exit_code, output, err = svntest.actions.run_and_verify_svn( @@ -2456,7 +2456,8 @@ def log_multiple_revs_spanning_rename(sb log_chain = parse_log_output(output) check_log_chain(log_chain, [1,4,3,2]) - # As above, but revision ranges from younger to older revs fail: + # As above, but revision ranges from younger to older. Previously this + # failed with: # # >svn log ^/trunk -r1:1 -r2:4 # ------------------------------------------------------------------------
