[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1039398 - in /subversion/trunk/subversion: include/svn_dirent_uri.h libsvn_delta/path_driver.c libsvn_subr/dirent_uri.c tests/libsvn_subr/dirent_uri-test.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 30 Nov 2010 15:43:09 +0200

Hi Julian,

julianfoad_at_apache.org wrote on Fri, Nov 26, 2010 at 14:58:17 -0000:
> Author: julianfoad
> Date: Fri Nov 26 14:58:17 2010
> New Revision: 1039398
>
> URL: http://svn.apache.org/viewvc?rev=1039398&view=rev
> Log:
> Add some more svn_fspath__* APIs, with tests for some of them. These tests
> pass both with and without FSPATH_USE_URI defined.
>
> * subversion/include/svn_dirent_uri.h,
> subversion/libsvn_subr/dirent_uri.c
> (svn_fspath__dirname, svn_fspath__split, svn_fspath__skip_ancestor,
> svn_fspath__is_ancestor, svn_fspath__get_longest_ancestor): New functions.
>
> * subversion/libsvn_delta/path_driver.c
> (svn_fspath__get_longest_ancestor): Remove this temporary macro.
>
> * subversion/tests/libsvn_subr/dirent_uri-test.c
> (test_fspath_basename): Rename and extend to become ...
> (test_fspath_dirname_basename_split): ... this new test.
> (test_fspath_get_longest_ancestor): New tests.
> (test_funcs): Add the new tests.
> ### TODO: Tests.
>

What is that ### comment? Is it the mailer saying you should have added
a few more tests to this commit?

Daniel

> Modified:
> subversion/trunk/subversion/include/svn_dirent_uri.h
> subversion/trunk/subversion/libsvn_delta/path_driver.c
> subversion/trunk/subversion/libsvn_subr/dirent_uri.c
> subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
>
> Modified: subversion/trunk/subversion/include/svn_dirent_uri.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_dirent_uri.h?rev=1039398&r1=1039397&r2=1039398&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_dirent_uri.h (original)
> +++ subversion/trunk/subversion/include/svn_dirent_uri.h Fri Nov 26 14:58:17 2010
> @@ -812,12 +812,12 @@ svn_uri_get_file_url_from_dirent(const c
> apr_pool_t *pool);
>
>
> -/**
> +/**************************************************************************
> * A private API for Subversion filesystem paths, otherwise known as paths
> * within a repository, that start with a '/'. The rules for a fspath are
> * the same as for a relpath except for the leading '/'. A fspath never
> * ends with '/' except when the whole path is just '/'.
> - */
> + **************************************************************************/
>
> /** Return TRUE iff @a fspath is canonical.
> * @a fspath need not be canonical, of course.
> @@ -827,6 +827,49 @@ svn_uri_get_file_url_from_dirent(const c
> svn_boolean_t
> svn_fspath__is_canonical(const char *fspath);
>
> +
> +/** Return the dirname of @a fspath, defined as the path with its basename
> + * removed. If @a fspath is "/", return "/".
> + *
> + * Allocate the result in @a pool.
> + *
> + * @since New in 1.7.
> + */
> +const char *
> +svn_fspath__dirname(const char *fspath,
> + apr_pool_t *pool);
> +
> +/** Return the last component of @a fspath. The returned value will have no
> + * slashes in it. If @a fspath is "/", return "".
> + *
> + * If @a pool is NULL, return a pointer to within @a fspath, else allocate
> + * the result in @a pool.
> + *
> + * @since New in 1.7.
> + */
> +const char *
> +svn_fspath__basename(const char *fspath,
> + apr_pool_t *pool);
> +
> +/** Divide the canonical @a fspath into @a *dirpath and @a
> + * *base_name, allocated in @a pool.
> + *
> + * If @a dirpath or @a base_name is NULL, then don't set that one.
> + *
> + * Either @a dirpath or @a base_name may be @a fspath's own address, but they
> + * may not both be the same address, or the results are undefined.
> + *
> + * If @a fspath has two or more components, the separator between @a dirpath
> + * and @a base_name is not included in either of the new names.
> + *
> + * @since New in 1.7.
> + */
> +void
> +svn_fspath__split(const char **dirpath,
> + const char **base_name,
> + const char *fspath,
> + apr_pool_t *result_pool);
> +
> /** Return the fspath composed of @a fspath with @a relpath appended.
> * Allocate the result in @a result_pool.
> *
> @@ -855,16 +898,37 @@ svn_fspath__is_child(const char *parent_
> const char *child_fspath,
> apr_pool_t *pool);
>
> -
> -/** Return the last component of @a fspath. The returned value will have no
> - * slashes in it. If @a pool is NULL, return a pointer to within @a fspath,
> - * else allocate the result in @a pool.
> +/** Return the relative path part of @a child_fspath that is below
> + * @a parent_fspath, or just "" if @a parent_fspath is equal to
> + * @a child_fspath. If @a child_fspath is not below @a parent_relpath,
> + * return @a child_fspath.
> + *
> + * ### Returning the child in the no-match case is a bad idea.
> *
> * @since New in 1.7.
> */
> const char *
> -svn_fspath__basename(const char *fspath,
> - apr_pool_t *pool);
> +svn_fspath__skip_ancestor(const char *parent_fspath,
> + const char *child_fspath);
> +
> +/** Return TRUE if @a parent_fspath is an ancestor of @a child_fspath or
> + * the fspaths are equal, and FALSE otherwise.
> + *
> + * @since New in 1.7.
> + */
> +svn_boolean_t
> +svn_fspath__is_ancestor(const char *parent_fspath,
> + const char *child_fspath);
> +
> +/** Return the longest common path shared by two fspaths, @a fspath1 and
> + * @a fspath2. If there's no common ancestor, return "/".
> + *
> + * @since New in 1.7.
> + */
> +char *
> +svn_fspath__get_longest_ancestor(const char *fspath1,
> + const char *fspath2,
> + apr_pool_t *result_pool);
>
>
> #ifdef __cplusplus
>
> Modified: subversion/trunk/subversion/libsvn_delta/path_driver.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/path_driver.c?rev=1039398&r1=1039397&r2=1039398&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_delta/path_driver.c (original)
> +++ subversion/trunk/subversion/libsvn_delta/path_driver.c Fri Nov 26 14:58:17 2010
> @@ -129,14 +129,6 @@ count_components(const char *path)
>
>
> /*** Public interfaces ***/
> -
> -/* ### Temporary definition, until this is available in dirent_uri.h */
> -#define svn_fspath__get_longest_ancestor(path1, path2, pool) \
> - apr_pstrcat((pool), \
> - "/", \
> - svn_relpath_get_longest_ancestor((path1)+1, (path2)+1, (pool)), \
> - NULL)
> -
> svn_error_t *
> svn_delta_path_driver(const svn_delta_editor_t *editor,
> void *edit_baton,
> @@ -225,7 +217,7 @@ svn_delta_path_driver(const svn_delta_ed
> /*** Step C - Open any directories between the common ancestor
> and the parent of the current path. ***/
> if (*path == '/')
> - svn_uri_split(&pdir, &bname, path, iterpool);
> + svn_fspath__split(&pdir, &bname, path, iterpool);
> else
> svn_relpath_split(&pdir, &bname, path, iterpool);
> if (strlen(pdir) > common_len)
>
> Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=1039398&r1=1039397&r2=1039398&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Fri Nov 26 14:58:17 2010
> @@ -2475,47 +2475,78 @@ svn_fspath__is_canonical(const char *fsp
> return fspath[0] == '/' && relpath_is_canonical(fspath + 1);
> }
>
> -char *
> -svn_fspath__join(const char *fspath,
> - const char *relpath,
> - apr_pool_t *result_pool)
> +
> +const char *
> +svn_fspath__is_child(const char *parent_fspath,
> + const char *child_fspath,
> + apr_pool_t *pool)
> {
> - char *result;
> - assert(svn_fspath__is_canonical(fspath));
> - assert(relpath_is_canonical(relpath));
> + const char *result;
> + assert(svn_fspath__is_canonical(parent_fspath));
> + assert(svn_fspath__is_canonical(child_fspath));
>
> #ifdef FSPATH_USE_URI
> - result = svn_uri_join(fspath, relpath, result_pool);
> + result = svn_uri_is_child(parent_fspath, child_fspath, pool);
> #else
> - if (relpath[0] == '\0')
> - result = apr_pstrdup(result_pool, fspath);
> - else if (fspath[1] == '\0')
> - result = apr_pstrcat(result_pool, "/", relpath, (char *)NULL);
> - else
> - result = apr_pstrcat(result_pool, fspath, "/", relpath, (char *)NULL);
> + result = svn_relpath_is_child(parent_fspath + 1, child_fspath + 1, pool);
> #endif
>
> - assert(svn_fspath__is_canonical(result));
> + assert(result == NULL || svn_relpath_is_canonical(result, pool));
> return result;
> }
>
> -
> const char *
> -svn_fspath__is_child(const char *parent_fspath,
> - const char *child_fspath,
> - apr_pool_t *pool)
> +svn_fspath__skip_ancestor(const char *parent_fspath,
> + const char *child_fspath)
> {
> const char *result;
> assert(svn_fspath__is_canonical(parent_fspath));
> assert(svn_fspath__is_canonical(child_fspath));
>
> #ifdef FSPATH_USE_URI
> - result = svn_uri_is_child(parent_fspath, child_fspath, pool);
> + result = svn_uri_skip_ancestor(parent_fspath, child_fspath);
> #else
> - result = svn_relpath_is_child(parent_fspath + 1, child_fspath + 1, pool);
> + if (svn_relpath_is_ancestor(parent_fspath + 1, child_fspath + 1))
> + result = svn_relpath_skip_ancestor(parent_fspath + 1, child_fspath + 1);
> + else
> + result = child_fspath;
> +#endif
> +
> + assert(svn_relpath_is_canonical(result, NULL)
> + || strcmp(result, child_fspath) == 0);
> + return result;
> +}
> +
> +svn_boolean_t
> +svn_fspath__is_ancestor(const char *parent_fspath,
> + const char *child_fspath)
> +{
> + assert(svn_fspath__is_canonical(parent_fspath));
> + assert(svn_fspath__is_canonical(child_fspath));
> +
> +#ifdef FSPATH_USE_URI
> + return svn_uri_is_ancestor(parent_fspath, child_fspath);
> +#else
> + return svn_relpath_is_ancestor(parent_fspath + 1, child_fspath + 1);
> +#endif
> +}
> +
> +
> +const char *
> +svn_fspath__dirname(const char *fspath,
> + apr_pool_t *pool)
> +{
> + const char *result;
> + assert(svn_fspath__is_canonical(fspath));
> +
> +#ifdef FSPATH_USE_URI
> + result = svn_uri_dirname(fspath, pool);
> +#else
> + result = apr_pstrcat(pool, "/", svn_relpath_dirname(fspath + 1, pool),
> + (char *)NULL);
> #endif
>
> - assert(result == NULL || relpath_is_canonical(result));
> + assert(svn_fspath__is_canonical(result));
> return result;
> }
>
> @@ -2537,4 +2568,64 @@ svn_fspath__basename(const char *fspath,
> return result;
> }
>
> +void
> +svn_fspath__split(const char **dirpath,
> + const char **base_name,
> + const char *fspath,
> + apr_pool_t *result_pool)
> +{
> + assert(dirpath != base_name);
> +
> + if (dirpath)
> + *dirpath = svn_fspath__dirname(fspath, result_pool);
> +
> + if (base_name)
> + *base_name = svn_fspath__basename(fspath, result_pool);
> +}
> +
> +char *
> +svn_fspath__join(const char *fspath,
> + const char *relpath,
> + apr_pool_t *result_pool)
> +{
> + char *result;
> + assert(svn_fspath__is_canonical(fspath));
> + assert(svn_relpath_is_canonical(relpath, result_pool));
> +
> +#ifdef FSPATH_USE_URI
> + result = svn_uri_join(fspath, relpath, result_pool);
> +#else
> + if (relpath[0] == '\0')
> + result = apr_pstrdup(result_pool, fspath);
> + else if (fspath[1] == '\0')
> + result = apr_pstrcat(result_pool, "/", relpath, (char *)NULL);
> + else
> + result = apr_pstrcat(result_pool, fspath, "/", relpath, (char *)NULL);
> +#endif
> +
> + assert(svn_fspath__is_canonical(result));
> + return result;
> +}
> +
> +char *
> +svn_fspath__get_longest_ancestor(const char *fspath1,
> + const char *fspath2,
> + apr_pool_t *result_pool)
> +{
> + char *result;
> + assert(svn_fspath__is_canonical(fspath1));
> + assert(svn_fspath__is_canonical(fspath2));
>
> +#ifdef FSPATH_USE_URI
> + result = svn_uri_get_longest_ancestor(fspath1, fspath2, result_pool);
> +#else
> + result = apr_pstrcat(result_pool, "/",
> + svn_relpath_get_longest_ancestor(fspath1 + 1,
> + fspath2 + 1,
> + result_pool),
> + NULL);
> +#endif
> +
> + assert(svn_fspath__is_canonical(result));
> + return result;
> +}
>
> Modified: subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c?rev=1039398&r1=1039397&r2=1039398&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Fri Nov 26 14:58:17 2010
> @@ -2961,30 +2961,95 @@ test_fspath_is_child(apr_pool_t *pool)
> }
>
> static svn_error_t *
> -test_fspath_basename(apr_pool_t *pool)
> +test_fspath_dirname_basename_split(apr_pool_t *pool)
> {
> int i;
>
> - struct {
> + static const struct {
> const char *path;
> - const char *result;
> + const char *dirname;
> + const char *basename;
> } tests[] = {
> - { "/", "" },
> - { "/a", "a" },
> - { "/abc", "abc" },
> - { "/x/abc", "abc" },
> + { "/", "/", "" },
> + { "/a", "/", "a" },
> + { "/abc", "/", "abc" },
> + { "/x/abc", "/x", "abc" },
> + { "/x/y/abc", "/x/y", "abc" },
> };
>
> for (i = 0; i < COUNT_OF(tests); i++)
> {
> - const char *result = svn_fspath__basename(tests[i].path, pool);
> + const char *result_dirname, *result_basename;
> +
> + result_dirname = svn_fspath__dirname(tests[i].path, pool);
> + SVN_TEST_STRING_ASSERT(result_dirname, tests[i].dirname);
>
> - SVN_TEST_STRING_ASSERT(result, tests[i].result);
> + result_basename = svn_fspath__basename(tests[i].path, pool);
> + SVN_TEST_STRING_ASSERT(result_basename, tests[i].basename);
> +
> + svn_fspath__split(&result_dirname, &result_basename, tests[i].path,
> + pool);
> + SVN_TEST_STRING_ASSERT(result_dirname, tests[i].dirname);
> + SVN_TEST_STRING_ASSERT(result_basename, tests[i].basename);
> }
>
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *
> +test_fspath_get_longest_ancestor(apr_pool_t *pool)
> +{
> + int i;
> +
> + /* Paths to test and their expected results. Same as in
> + * test_relpath_get_longest_ancestor() but with '/' prefix. */
> + struct {
> + const char *path1;
> + const char *path2;
> + const char *result;
> + } tests[] = {
> + { "/foo", "/foo/bar", "/foo" },
> + { "/foo/bar", "/foo/bar", "/foo/bar" },
> + { "/", "/foo", "/" },
> + { "/", "/foo", "/" },
> + { "/", "/.bar", "/" },
> + { "/.bar", "/", "/" },
> + { "/foo/bar", "/foo", "/foo" },
> + { "/foo/bar", "/foo", "/foo" },
> + { "/rif", "/raf", "/" },
> + { "/foo", "/bar", "/" },
> + { "/foo", "/foo/bar", "/foo" },
> + { "/foo.", "/foo./.bar", "/foo." },
> + { "/", "/", "/" },
> + { "/http:/test", "/http:/test", "/http:/test" },
> + { "/http:/test", "/http:/taste", "/http:" },
> + { "/http:/test", "/http:/test/foo", "/http:/test" },
> + { "/http:/test", "/file:/test/foo", "/" },
> + { "/http:/test", "/http:/testF", "/http:" },
> + { "/file:/A/C", "/file:/B/D", "/file:" },
> + { "/file:/A/C", "/file:/A/D", "/file:/A" },
> + { "/X:/foo", "/X:", "/X:" },
> + { "/X:/folder1", "/X:/folder2", "/X:" },
> + { "/X:", "/X:foo", "/" },
> + { "/X:foo", "/X:bar", "/" },
> + };
> +
> + for (i = 0; i < COUNT_OF(tests); i++)
> + {
> + const char *result;
> +
> + result = svn_fspath__get_longest_ancestor(tests[i].path1, tests[i].path2,
> + pool);
> + SVN_TEST_STRING_ASSERT(tests[i].result, result);
> +
> + /* changing the order of the paths should return the same result */
> + result = svn_fspath__get_longest_ancestor(tests[i].path2, tests[i].path1,
> + pool);
> + SVN_TEST_STRING_ASSERT(tests[i].result, result);
> + }
> + return SVN_NO_ERROR;
> +}
> +
>
> /* The test table. */
>
> @@ -3091,7 +3156,9 @@ struct svn_test_descriptor_t test_funcs[
> "test svn_fspath__join"),
> SVN_TEST_PASS2(test_fspath_is_child,
> "test svn_fspath__is_child"),
> - SVN_TEST_PASS2(test_fspath_basename,
> - "test svn_fspath__basename"),
> + SVN_TEST_PASS2(test_fspath_dirname_basename_split,
> + "test svn_fspath__dirname/basename/split"),
> + SVN_TEST_PASS2(test_fspath_get_longest_ancestor,
> + "test svn_fspath__get_longest_ancestor"),
> SVN_TEST_NULL
> };
>
>
Received on 2010-11-30 14:45:13 CET

This is an archived mail posted to the Subversion Dev mailing list.