Index: subversion/include/private/svn_opt_private.h =================================================================== --- subversion/include/private/svn_opt_private.h (revision 31120) +++ subversion/include/private/svn_opt_private.h (working copy) @@ -34,7 +34,7 @@ * - check that only valid URI characters remain * - check that no back-path ("..") components are present * - canonicalize the separator ("/") characters - * URL_IN is in UTF-8 encoding and has no peg revision specifier. + * URL_IN is in UTF-8 encoding and can have a peg revision specifier. * Set *URL_OUT to the result, allocated from POOL. */ svn_error_t * @@ -50,7 +50,7 @@ * - If the path does not exist (which is valid) the given capitialization * is used. * - canonicalize the separator ("/") characters - * PATH_IN is in UTF-8 encoding and has no peg revision specifier. + * PATH_IN is in UTF-8 encoding and can have a peg revision specifier. * Set *PATH_OUT to the result, allocated from POOL. */ svn_error_t * Index: subversion/libsvn_subr/opt.c =================================================================== --- subversion/libsvn_subr/opt.c (revision 31120) +++ subversion/libsvn_subr/opt.c (working copy) @@ -803,81 +803,123 @@ return SVN_NO_ERROR; } +/* Extract the peg revision, if any, from UTF8_TARGET. Return the peg + * revision in *PEG_REVISION and the true target portion in *TRUE_TARGET. + * *PEG_REVISION will be set to NULL if no peg revision is found. + * + * UTF8_TARGET need not be canonical. *TRUE_TARGET will not be canonical + * unless UTF8_TARGET is. + * + * *PEG_REVISION does still contain the '@' symbol as the first character. + * + * All allocations are done in POOL. + */ +static svn_error_t * +split_arg_at_peg_revision(const char **true_target, + const char **peg_revision, + const char *utf8_target, + apr_pool_t *pool) +{ + const char *peg_start = NULL; /* pointer to the peg revision, if any */ + int j; + for (j = (strlen(utf8_target) - 1); j >= 0; --j) + { + /* If we hit a path separator, stop looking. This is OK + only because our revision specifiers can't contain + '/'. */ + if (utf8_target[j] == '/') + break; + if (utf8_target[j] == '@') + { + peg_start = utf8_target + j; + break; + } + } + + if (peg_start) + { + *true_target = apr_pstrmemdup(pool, + utf8_target, + peg_start - utf8_target); + + *peg_revision = apr_pstrdup(pool, peg_start); + } + else + { + *true_target = utf8_target; + + *peg_revision = NULL; + } + + return SVN_NO_ERROR; +} + svn_error_t * svn_opt_parse_path(svn_opt_revision_t *rev, const char **truepath, const char *path /* UTF-8! */, apr_pool_t *pool) { - int i; + const char *peg_rev; - /* scanning from right to left, just to be friendly to any - screwed-up filenames that might *actually* contain @-signs. :-) */ - for (i = (strlen(path) - 1); i >= 0; i--) + SVN_ERR(split_arg_at_peg_revision(truepath, &peg_rev, path, pool)); + + /* Parse the peg revision, if one was found */ + if (peg_rev) { - /* If we hit a path separator, stop looking. */ - /* This is OK only because our revision specifiers can't contain '/'. */ - if (path[i] == '/') - break; + int ret; + svn_opt_revision_t start_revision, end_revision; - if (path[i] == '@') + end_revision.kind = svn_opt_revision_unspecified; + + if (peg_rev[1] == '\0') /* looking at empty peg revision */ { - int ret; - svn_opt_revision_t start_revision, end_revision; + ret = 0; + start_revision.kind = svn_opt_revision_unspecified; + } + else /* looking at non-empty peg revision */ + { + const char *rev_str = peg_rev + 1; - end_revision.kind = svn_opt_revision_unspecified; - - if (path[i + 1] == '\0') /* looking at empty peg revision */ + /* URLs get treated differently from wc paths. */ + if (svn_path_is_url(path)) { - ret = 0; - start_revision.kind = svn_opt_revision_unspecified; - } - else /* looking at non-empty peg revision */ - { - const char *rev_str = path + i + 1; - - /* URLs get treated differently from wc paths. */ - if (svn_path_is_url(path)) + /* URLs are URI-encoded, so we look for dates with + URI-encoded delimeters. */ + int rev_len = strlen(rev_str); + if (rev_len > 6 + && rev_str[0] == '%' + && rev_str[1] == '7' + && (rev_str[2] == 'B' + || rev_str[2] == 'b') + && rev_str[rev_len-3] == '%' + && rev_str[rev_len-2] == '7' + && (rev_str[rev_len-1] == 'D' + || rev_str[rev_len-1] == 'd')) { - /* URLs are URI-encoded, so we look for dates with - URI-encoded delimeters. */ - int rev_len = strlen(rev_str); - if (rev_len > 6 - && rev_str[0] == '%' - && rev_str[1] == '7' - && (rev_str[2] == 'B' - || rev_str[2] == 'b') - && rev_str[rev_len-3] == '%' - && rev_str[rev_len-2] == '7' - && (rev_str[rev_len-1] == 'D' - || rev_str[rev_len-1] == 'd')) - { - rev_str = svn_path_uri_decode(rev_str, pool); - } + rev_str = svn_path_uri_decode(rev_str, pool); } - ret = svn_opt_parse_revision(&start_revision, - &end_revision, - rev_str, pool); } + ret = svn_opt_parse_revision(&start_revision, + &end_revision, + rev_str, pool); + } - if (ret || end_revision.kind != svn_opt_revision_unspecified) - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL, - _("Syntax error parsing revision '%s'"), - path + i + 1); + if (ret || end_revision.kind != svn_opt_revision_unspecified) + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL, + _("Syntax error parsing revision '%s'"), + peg_rev + 1); - *truepath = apr_pstrmemdup(pool, path, i); - rev->kind = start_revision.kind; - rev->value = start_revision.value; - - return SVN_NO_ERROR; - } + rev->kind = start_revision.kind; + rev->value = start_revision.value; } + else + { + /* Didn't find an peg revision. */ + rev->kind = svn_opt_revision_unspecified; + } - /* Didn't find an @-sign. */ - *truepath = path; - rev->kind = svn_opt_revision_unspecified; - return SVN_NO_ERROR; } @@ -946,40 +988,8 @@ for (i = 0; i < input_targets->nelts; i++) { const char *utf8_target = APR_ARRAY_IDX(input_targets, i, const char *); - const char *peg_start = NULL; /* pointer to the peg revision, if any */ const char *target; /* after all processing is finished */ - int j; - /* Remove a peg revision, if any, in the target so that it can - be properly canonicalized, otherwise the canonicalization - does not treat a ".@BASE" as a "." with a BASE peg revision, - and it is not canonicalized to "@BASE". If any peg revision - exists, it is appended to the final canonicalized path or - URL. Do not use svn_opt_parse_path() because the resulting - peg revision is a structure that would have to be converted - back into a string. Converting from a string date to the - apr_time_t field in the svn_opt_revision_value_t and back to - a string would not necessarily preserve the exact bytes of - the input date, so its easier just to keep it in string - form. */ - for (j = (strlen(utf8_target) - 1); j >= 0; --j) - { - /* If we hit a path separator, stop looking. This is OK - only because our revision specifiers can't contain - '/'. */ - if (utf8_target[j] == '/') - break; - if (utf8_target[j] == '@') - { - peg_start = utf8_target + j; - break; - } - } - if (peg_start) - utf8_target = apr_pstrmemdup(pool, - utf8_target, - peg_start - utf8_target); - /* URLs and wc-paths get treated differently. */ if (svn_path_is_url(utf8_target)) { @@ -987,13 +997,19 @@ } else /* not a url, so treat as a path */ { - const char *base_name; + char *base_name; + char *peg_rev; SVN_ERR(svn_opt__arg_canonicalize_path(&target, utf8_target, pool)); /* If the target has the same name as a Subversion working copy administrative dir, skip it. */ base_name = svn_path_basename(target, pool); + + /* "Delete" the peg revision */ + if (peg_rev = strrchr(base_name, '@')) + *peg_rev = '\0'; + /* FIXME: The canonical list of administrative directory names is maintained in libsvn_wc/adm_files.c:svn_wc_set_adm_dir(). @@ -1011,11 +1027,6 @@ } } - /* Append the peg revision back to the canonicalized target if - there was a peg revision. */ - if (peg_start) - target = apr_pstrcat(pool, target, peg_start, NULL); - APR_ARRAY_PUSH(output_targets, const char *) = target; } @@ -1114,14 +1125,30 @@ return SVN_NO_ERROR; } + svn_error_t * svn_opt__arg_canonicalize_url(const char **url_out, const char *url_in, apr_pool_t *pool) { const char *target; + const char *peg_rev; + /* + * This is needed so that url_in can be properly canonicalized, + * otherwise the canonicalization does not treat a ".@BASE" as a "." + * with a BASE peg revision, and it is not canonicalized to "@BASE". + * If any peg revision exists, it is appended to the final + * canonicalized path or URL. Do not use svn_opt_parse_path() + * because the resulting peg revision is a structure that would have + * to be converted back into a string. Converting from a string date + * to the apr_time_t field in the svn_opt_revision_value_t and back to + * a string would not necessarily preserve the exact bytes of the + * input date, so its easier just to keep it in string form. + */ + SVN_ERR(split_arg_at_peg_revision(&target, &peg_rev, url_in, pool)); + /* Convert to URI. */ - target = svn_path_uri_from_iri(url_in, pool); + target = svn_path_uri_from_iri(target, pool); /* Auto-escape some ASCII characters. */ target = svn_path_uri_autoescape(target, pool); @@ -1140,6 +1167,11 @@ /* strip any trailing '/' and collapse other redundant elements */ target = svn_path_canonicalize(target, pool); + /* Append the peg revision back to the canonicalized target if + there was a peg revision. */ + if (peg_rev) + target = apr_pstrcat(pool, target, peg_rev, NULL); + *url_out = target; return SVN_NO_ERROR; } @@ -1151,9 +1183,24 @@ const char *apr_target; char *truenamed_target; /* APR-encoded */ apr_status_t apr_err; + const char *peg_rev; + /* + * This is needed so that url_in can be properly canonicalized, + * otherwise the canonicalization does not treat a ".@BASE" as a "." + * with a BASE peg revision, and it is not canonicalized to "@BASE". + * If any peg revision exists, it is appended to the final + * canonicalized path or URL. Do not use svn_opt_parse_path() + * because the resulting peg revision is a structure that would have + * to be converted back into a string. Converting from a string date + * to the apr_time_t field in the svn_opt_revision_value_t and back to + * a string would not necessarily preserve the exact bytes of the + * input date, so its easier just to keep it in string form. + */ + SVN_ERR(split_arg_at_peg_revision(&apr_target, &peg_rev, path_in, pool)); + /* canonicalize case, and change all separators to '/'. */ - SVN_ERR(svn_path_cstring_from_utf8(&apr_target, path_in, pool)); + SVN_ERR(svn_path_cstring_from_utf8(&apr_target, apr_target, pool)); apr_err = apr_filepath_merge(&truenamed_target, "", apr_target, APR_FILEPATH_TRUENAME, pool); @@ -1174,6 +1221,11 @@ SVN_ERR(svn_path_cstring_to_utf8(path_out, apr_target, pool)); *path_out = svn_path_canonicalize(*path_out, pool); + /* Append the peg revision back to the canonicalized target if + there was a peg revision. */ + if (peg_rev) + *path_out = apr_pstrcat(pool, *path_out, peg_rev, NULL); + return SVN_NO_ERROR; } Index: subversion/libsvn_client/cmdline.c =================================================================== --- subversion/libsvn_client/cmdline.c (revision 31120) +++ subversion/libsvn_client/cmdline.c (working copy) @@ -196,39 +196,8 @@ for (i = 0; i < input_targets->nelts; i++) { const char *utf8_target = APR_ARRAY_IDX(input_targets, i, const char *); - const char *peg_start = NULL; /* pointer to the peg revision, if any */ const char *target; - int j; - /* Remove a peg revision, if any, in the target so that it can - be properly canonicalized, otherwise the canonicalization - does not treat a ".@BASE" as a "." with a BASE peg revision, - and it is not canonicalized to "@BASE". If any peg revision - exists, it is appended to the final canonicalized path or - URL. Do not use svn_opt_parse_path() because the resulting - peg revision is a structure that would have to be converted - back into a string. Converting from a string date to the - apr_time_t field in the svn_opt_revision_value_t and back to - a string would not necessarily preserve the exact bytes of - the input date, so its easier just to keep it in string - form. */ - for (j = (strlen(utf8_target) - 1); j >= 0; --j) - { - /* If we hit a path separator, stop looking. This is OK - only because our revision specifiers can't contain - '/'. */ - if (utf8_target[j] == '/') - break; - if (utf8_target[j] == '@') - { - peg_start = utf8_target + j; - break; - } - } - if (peg_start) - utf8_target = apr_pstrmemdup(pool, - utf8_target, - peg_start - utf8_target); /* Relative urls will be canonicalized when they are resolved later in * the function @@ -247,7 +216,8 @@ } else /* not a url, so treat as a path */ { - const char *base_name; + char *base_name; + char *peg_rev; SVN_ERR(svn_opt__arg_canonicalize_path(&target, utf8_target, pool)); @@ -256,6 +226,12 @@ working copy administrative dir, skip it. */ base_name = svn_path_basename(target, pool); + /* "Delete" the peg revision because svn_wc_is_adm_dir() cannot + * handle it. + */ + if (peg_rev = strrchr(base_name, '@')) + *peg_rev = '\0'; + if (svn_wc_is_adm_dir(base_name, pool)) { err = svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED, @@ -266,11 +242,6 @@ } } - /* Append the peg revision back to the canonicalized target if - there was a peg revision. */ - if (peg_start) - target = apr_pstrcat(pool, target, peg_start, NULL); - if (rel_url_found) { SVN_ERR(check_root_url_of_target(&root_url, target, Index: subversion/tests/cmdline/basic_tests.py =================================================================== --- subversion/tests/cmdline/basic_tests.py (revision 31120) +++ subversion/tests/cmdline/basic_tests.py (working copy) @@ -2314,6 +2314,47 @@ expected_output, [], 'ls', '^//A/', iota_url) +def basic_relative_url_with_peg_revisions(sbox): + "basic relative url targets with peg revisions" + + sbox.build() + + # First, make a new revision of iota. + iota = os.path.join(sbox.wc_dir, 'iota') + svntest.main.file_append(iota, "New contents for iota\n") + svntest.main.run_svn(None, 'ci', + '-m', '', iota) + + iota_url = sbox.repo_url + '/iota' + + # Now, make a new revision of A/mu . + mu = os.path.join(sbox.wc_dir, 'A', 'mu') + mu_url = sbox.repo_url + '/A/mu' + + svntest.main.file_append(mu, "New contents for mu\n") + svntest.main.run_svn(None, 'ci', '-m', '', mu) + + # Delete the file from the current revision + svntest.main.run_svn(None, 'rm', '-m', '', mu_url) + + expected_output = [ + "B/\n", + "C/\n", + "D/\n", + "mu\n", + "iota\n" + ] + + # Canonical version with peg revision + exit_code, output, error = svntest.actions.run_and_verify_svn(None, + expected_output, [], 'ls', '-r3', + '^/A/@3', iota_url) + + # Non-canonical version with peg revision + exit_code, output, error = svntest.actions.run_and_verify_svn(None, + expected_output, [], 'ls', '-r3', + '^//A/@3', iota_url) + #---------------------------------------------------------------------- ######################################################################## @@ -2365,6 +2406,7 @@ basic_relative_url_using_other_targets, basic_relative_url_multi_repo, basic_relative_url_non_canonical, + basic_relative_url_with_peg_revisions, ] if __name__ == '__main__':