Hi Julian, julianf...@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 > }; > >
