Ping...Has anybody had a chance to look at this? If nothing happens in
the next few days, I'll open an issue.
-Hyrum
Lieven Govaerts wrote:
> Hi,
>
> as part of the complete solution for issue #1711 this patch adds support
> for X: paths. Previous patches already added support for working copies
> on 'X:/' (root of a drive).
>
> The patch also contains lots of extra testcases for the path functions,
> I've added tests for all impacted functions in this patch.
>
> Lieven.
>
>
> [[[
> Follow up change for issue #1711: add support for 'X:' paths. On Windows,
> 'X:' is a symbolic link to the current working directory on the X drive.
>
> Added unit tests for all the impacted path functions.
>
> * subversion/libsvn_subr/path.c
> (svn_path_join_many, previous_segment,
> svn_path_basename, svn_path_is_ancestor): small fixes to support X:
> paths on Windows
>
> * subversion/tests/libsvn_subr/path-test.c
> (test_join, test_canonicalize, test_remove_component,
> test_is_root, ): added extra test cases for Windows paths (X: and X:/)
> (test_dirname): new test for svn_path_dirname
> (test_path_is_ancestor): new test for svn_path_is_ancestor
> (test_funcs): run the two new tests
>
> * subversion/tests/cmdline/update_tests.py
> (update_wc_on_windows_drive): expand the test with a testcase for X:
> path support
>
> * subversion/tests/cmdline/svntest/tree.py
> (create_from_path): on Windows, the 'X:' in 'X:foo' is a folder, so
> consider the ':' in the first element of a path as a folder separator
> on that platform.
> ]]]
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/libsvn_subr/path.c
> ===================================================================
> --- subversion/libsvn_subr/path.c (revision 21396)
> +++ subversion/libsvn_subr/path.c (working copy)
> @@ -185,13 +185,13 @@
> if (nargs++ < MAX_SAVED_LENGTHS)
> saved_lengths[nargs] = len;
>
> - if (svn_path_is_absolute(s, strlen(s), pool))
> + if (svn_path_is_absolute(s, len, pool))
> {
> /* an absolute path. skip all components to this point and reset
> the total length. */
> total_len = len;
> base_arg = nargs;
> - base_is_root = len == 1;
> + base_is_root = svn_path_is_root(s, len, pool);
> base_is_empty = FALSE;
> }
> else if (nargs == base_arg
> @@ -254,7 +254,8 @@
> (which can happen when base_arg is set). also, don't put in a slash
> if the prior character is a slash (occurs when prior component
> is "/"). */
> - if (p != path && p[-1] != '/')
> + if (p != path && p[-1] != '/' &&
> + ! (nargs - 1 == base_arg && base_is_root))
> *p++ = '/';
>
> /* copy the new component and advance the pointer */
> @@ -311,8 +312,13 @@
> if (len == 0)
> return 0;
>
> - while (len > 0 && path[--len] != '/')
> - ;
> + --len;
> + while (len > 0 && path[len] != '/'
> +#if defined(WIN32)
> + && path[len] != ':'
> +#endif /* WIN32 */
> + )
> + --len;
>
> /* check if the remaining segment including trailing '/' is a root path */
> if (svn_path_is_root(path, len + 1, NULL))
> @@ -387,7 +393,11 @@
> else
> {
> start = len;
> - while (start > 0 && path[start - 1] != '/')
> + while (start > 0 && path[start - 1] != '/'
> +#if defined(WIN32)
> + && path[start - 1] != ':'
> +#endif /* WIN32 */
> + )
> --start;
> }
>
> @@ -698,19 +708,22 @@
> {
> apr_size_t path1_len = strlen(path1);
>
> - /* If path1 is empty and path2 is not absoulte, then path1 is an ancestor. */
> + /* If path1 is empty and path2 is not absolute, then path1 is an ancestor. */
> if (SVN_PATH_IS_EMPTY(path1))
> return *path2 != '/';
>
> /* If path1 is a prefix of path2, then:
> - - If path1 ends in a path separator,
> + - If path1 ends in a path separator ('/' or ':' on Windows),
> - If the paths are of the same length
> OR
> - path2 starts a new path component after the common prefix,
> then path1 is an ancestor. */
> if (strncmp(path1, path2, path1_len) == 0)
> - return path1[path1_len - 1] == '/'
> - || (path2[path1_len] == '/' || path2[path1_len] == '\0');
> + return path1[path1_len - 1] == '/' ||
> +#if defined (WIN32)
> + path1[path1_len - 1] == ':' ||
> +#endif /* WIN32 */
> + (path2[path1_len] == '/' || path2[path1_len] == '\0');
>
> return FALSE;
> }
> Index: subversion/tests/cmdline/svntest/tree.py
> ===================================================================
> --- subversion/tests/cmdline/svntest/tree.py (revision 21396)
> +++ subversion/tests/cmdline/svntest/tree.py (working copy)
> @@ -276,10 +276,21 @@
> ### we should raise a less generic error here. which?
> raise SVNTreeError
>
> - root_node = SVNTreeNode(elements[0], None)
> + root_node = None
> +
> + # if this is Windows: if the path contains a drive name (X:), make it
> + # the root node.
> + if os.name == 'nt':
> + m = re.match("([a-zA-Z]:)(.+)", elements[0])
> + if m:
> + root_node = SVNTreeNode(m.group(1), None)
> + elements[0] = m.group(2)
> + add_elements_as_path(root_node, elements[0:])
> +
> + if not root_node:
> + root_node = SVNTreeNode(elements[0], None)
> + add_elements_as_path(root_node, elements[1:])
>
> - add_elements_as_path(root_node, elements[1:])
> -
> # deposit contents in the very last node.
> node = root_node
> while 1:
> Index: subversion/tests/cmdline/update_tests.py
> ===================================================================
> --- subversion/tests/cmdline/update_tests.py (revision 21396)
> +++ subversion/tests/cmdline/update_tests.py (working copy)
> @@ -2181,7 +2181,7 @@
> mu_path = os.path.join(wc_dir, 'A', 'mu')
> svntest.main.file_append (mu_path, '\nAppended text for mu')
> zeta_path = os.path.join(wc_dir, 'zeta')
> - svntest.main.file_append(zeta_path, "This is the file 'zeta'.\n")
> + svntest.main.file_append(zeta_path, "This is the file 'zeta'\n")
> svntest.main.run_svn(None, 'add', zeta_path)
>
> # Commit.
> @@ -2204,7 +2204,7 @@
> os.mkdir(dir1_path)
> svntest.main.run_svn(None, 'add', '-N', dir1_path)
> file1_path = os.path.join(dir1_path, 'file1')
> - svntest.main.file_append(file1_path, "This is the file 'file1'.\n")
> + svntest.main.file_append(file1_path, "This is the file 'file1'\n")
> svntest.main.run_svn(None, 'add', '-N', file1_path)
>
> expected_output = svntest.wc.State(wc_dir, {
> @@ -2239,7 +2239,34 @@
> None, None, None, None, None, 0,
> '-r', '1', wc_dir)
>
> - svntest.main.run_svn(None, 'update', wc_dir)
> + os.chdir(was_cwd)
> +
> + # update to the latest version, but use the relative path 'X:'
> + wc_dir = drive + ":"
> + expected_output = svntest.wc.State(wc_dir, {
> + 'A/mu' : Item(status='U '),
> + 'zeta' : Item(status='A '),
> + 'dir1' : Item(status='A '),
> + 'dir1/file1' : Item(status='A '),
> + })
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
> + expected_status.tweak(wc_rev=3)
> + expected_status.add({
> + 'dir1' : Item(status=' ', wc_rev=3),
> + 'dir1/file1' : Item(status=' ', wc_rev=3),
> + 'zeta' : Item(status=' ', wc_rev=3),
> + })
> + expected_disk.add({
> + 'zeta' : Item("This is the file 'zeta'\n"),
> + 'dir1/file1': Item("This is the file 'file1'\n"),
> + })
> + expected_disk.tweak('A/mu', contents = expected_disk.desc['A/mu'].contents
> + + '\nAppended text for mu')
> + svntest.actions.run_and_verify_update(wc_dir,
> + expected_output,
> + expected_disk,
> + expected_status)
> +
> finally:
> os.chdir(was_cwd)
> # cleanup the virtual drive
> Index: subversion/tests/libsvn_subr/path-test.c
> ===================================================================
> --- subversion/tests/libsvn_subr/path-test.c (revision 21396)
> +++ subversion/tests/libsvn_subr/path-test.c (working copy)
> @@ -491,6 +491,13 @@
> { "X:/abc", "/", "/" },
> { "X:/abc", "X:/", "X:/" },
> { "X:/abc", "X:/def", "X:/def" },
> + { "X:",SVN_EMPTY_PATH, "X:" },
> + { "X:","abc", "X:abc" },
> + { "X:", "/def", "/def" },
> + { "X:abc", "/d", "/d" },
> + { "X:abc", "/", "/" },
> + { "X:abc", "X:/", "X:/" },
> + { "X:abc", "X:/def", "X:/def" },
> #endif /* WIN32 */
> };
>
> @@ -560,6 +567,27 @@
> TEST_MANY((pool, SVN_EMPTY_PATH, "/", SVN_EMPTY_PATH, NULL), "/");
> TEST_MANY((pool, SVN_EMPTY_PATH, SVN_EMPTY_PATH, "/", NULL), "/");
>
> +#if defined(WIN32)
> + TEST_MANY((pool, "X:/", "def", "ghi", NULL), "X:/def/ghi");
> + TEST_MANY((pool, "abc", "X:/", "ghi", NULL), "X:/ghi");
> + TEST_MANY((pool, "abc", "def", "X:/", NULL), "X:/");
> + TEST_MANY((pool, "X:/", "X:/", "ghi", NULL), "X:/ghi");
> + TEST_MANY((pool, "X:/", "X:/", "/", NULL), "/");
> + TEST_MANY((pool, "X:/", SVN_EMPTY_PATH, "ghi", NULL), "X:/ghi");
> + TEST_MANY((pool, "X:/", "def", SVN_EMPTY_PATH, NULL), "X:/def");
> + TEST_MANY((pool, SVN_EMPTY_PATH, "X:/", "ghi", NULL), "X:/ghi");
> + TEST_MANY((pool, "X:/", SVN_EMPTY_PATH, SVN_EMPTY_PATH, NULL), "X:/");
> + TEST_MANY((pool, SVN_EMPTY_PATH, "X:/", SVN_EMPTY_PATH, NULL), "X:/");
> + TEST_MANY((pool, SVN_EMPTY_PATH, SVN_EMPTY_PATH, "X:/", NULL), "X:/");
> +
> + TEST_MANY((pool, "X:", "def", "ghi", NULL), "X:def/ghi");
> + TEST_MANY((pool, "X:", "X:/", "ghi", NULL), "X:/ghi");
> + TEST_MANY((pool, "X:", "X:/", "/", NULL), "/");
> + TEST_MANY((pool, "X:", SVN_EMPTY_PATH, "ghi", NULL), "X:ghi");
> + TEST_MANY((pool, "X:", "def", SVN_EMPTY_PATH, NULL), "X:def");
> + TEST_MANY((pool, SVN_EMPTY_PATH, "X:", "ghi", NULL), "X:ghi");
> +#endif /* WIN32 */
> +
> /* ### probably need quite a few more tests... */
>
> return SVN_NO_ERROR;
> @@ -592,6 +620,8 @@
> #if defined(WIN32)
> { "X:/", "X:/" },
> { "X:/abc", "abc" },
> + { "X:", "X:" },
> + { "X:abc", "abc" },
> #endif /* WIN32 */
> };
>
> @@ -617,6 +647,58 @@
>
>
> static svn_error_t *
> +test_dirname(const char **msg,
> + svn_boolean_t msg_only,
> + svn_test_opts_t *opts,
> + apr_pool_t *pool)
> +{
> + int i;
> + char *result;
> +
> + static const char * const paths[][2] = {
> + { "abc", "" },
> + { "/abc", "/" },
> + { "/x/abc", "/x" },
> + { "/xx/abc", "/xx" },
> + { "a", "" },
> + { "/a", "/" },
> + { "/b/a", "/b" },
> + { "/", "/" },
> + { SVN_EMPTY_PATH, SVN_EMPTY_PATH },
> +#if defined(WIN32)
> + { "X:/", "X:/" },
> + { "X:/abc", "X:/" },
> + { "X:abc", "X:" },
> + { "X:", "X:" },
> +#else
> + /* on non-Windows platforms, ':' is allowed in pathnames */
> + { "X:", "" },
> + { "X:abc", "" },
> +#endif /* WIN32 */
> + };
> +
> + *msg = "test svn_path_dirname";
> + if (msg_only)
> + return SVN_NO_ERROR;
> +
> + for (i = sizeof(paths) / sizeof(paths[0]); i--; )
> + {
> + const char *path = paths[i][0];
> + const char *expect = paths[i][1];
> +
> + result = svn_path_dirname(path, pool);
> + if (strcmp(result, expect))
> + return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
> + "svn_path_dirname(\"%s\") returned "
> + "\"%s\". expected \"%s\"",
> + path, result, expect);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
> +static svn_error_t *
> test_decompose(const char **msg,
> svn_boolean_t msg_only,
> svn_test_opts_t *opts,
> @@ -722,6 +804,7 @@
> { "//hst/./", "/hst" },
> { "X:/foo", "X:/foo" },
> { "X:/", "X:/" },
> + { "X:", "X:" },
> { "X:foo", "X:foo" },
> #endif /* WIN32 or Cygwin */
> { NULL, NULL }
> @@ -764,6 +847,9 @@
> #if defined(WIN32)
> { "X:/foo/bar", "X:/foo" },
> { "X:/foo", "X:/" },
> + { "X:/", "X:/" },
> + { "X:foo", "X:" },
> + { "X:", "X:" },
> #endif /* WIN32 */
> { NULL, NULL }
> };
> @@ -810,6 +896,8 @@
> #if defined(WIN32)
> "X:/foo",
> "X:/",
> + "X:foo",
> + "X:",
> #endif /* WIN32 */
> "",
> };
> @@ -819,10 +907,11 @@
> FALSE,
> FALSE,
> TRUE,
> +#if defined(WIN32)
> FALSE,
> -#if defined(WIN32)
> TRUE,
> FALSE,
> + TRUE
> #endif /* WIN32 */
> };
>
> @@ -846,6 +935,86 @@
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *
> +test_path_is_ancestor(const char **msg,
> + svn_boolean_t msg_only,
> + svn_test_opts_t *opts,
> + apr_pool_t *pool)
> +{
> + apr_size_t i;
> +
> + static const char * const paths[][2] = {
> + { "/foo", "/foo/bar" },
> + { "/foo/bar", "/foo/bar/" },
> + { "/", "/foo" },
> + { SVN_EMPTY_PATH, "foo" },
> + { SVN_EMPTY_PATH, ".bar", },
> +
> + { "/.bar", "/" },
> + { "foo/bar", "foo" },
> + { "/foo/bar", "/foo" },
> + { "foo", "foo/bar" },
> + { "foo.", "foo./.bar" },
> +
> + { "../foo", ".." },
> + { SVN_EMPTY_PATH, SVN_EMPTY_PATH },
> + { "/", "/" },
> +
> +#if defined(WIN32)
> + { "X:/", "X:/" },
> + { "X:/foo", "X:/" },
> + { "X:/", "X:/foo" },
> + { "X:", "X:foo" },
> + { "X:foo", "X:bar" },
> +#endif /* WIN32 */
> + };
> +
> + /* Expected results of the tests. */
> + static const svn_boolean_t retvals[] = {
> + TRUE,
> + TRUE,
> + TRUE,
> + TRUE,
> + TRUE,
> +
> + FALSE,
> + FALSE,
> + FALSE,
> + TRUE,
> + TRUE,
> +
> + FALSE,
> + TRUE,
> + TRUE,
> +#if defined(WIN32)
> + TRUE,
> + FALSE,
> + TRUE,
> + TRUE,
> + FALSE
> +#endif /* WIN32 */
> + };
> +
> + *msg = "test svn_path_is_ancestor";
> +
> + if (msg_only)
> + return SVN_NO_ERROR;
> +
> + for (i = 0; i < sizeof(paths) / sizeof(paths[0]); i++)
> + {
> + svn_boolean_t retval;
> +
> + retval = svn_path_is_ancestor(paths[i][0], paths[i][1]);
> + if (retvals[i] != retval)
> + return svn_error_createf
> + (SVN_ERR_TEST_FAILED, NULL,
> + "svn_path_is_ancestor (%s, %s) returned %s instead of %s",
> + paths[i][0], paths[i][1], retval ? "TRUE" : "FALSE",
> + retvals[i] ? "TRUE" : "FALSE");
> + }
> + return SVN_NO_ERROR;
> +}
> +
>
> /* The test table. */
>
> @@ -862,9 +1031,11 @@
> SVN_TEST_PASS(test_uri_from_iri),
> SVN_TEST_PASS(test_join),
> SVN_TEST_PASS(test_basename),
> + SVN_TEST_PASS(test_dirname),
> SVN_TEST_PASS(test_decompose),
> SVN_TEST_PASS(test_canonicalize),
> SVN_TEST_PASS(test_remove_component),
> SVN_TEST_PASS(test_is_root),
> + SVN_TEST_PASS(test_path_is_ancestor),
> SVN_TEST_NULL
> };
>
>
>
> ------------------------------------------------------------------------
Received on Mon Sep 18 15:55:58 2006