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

Re: [PATCH] added support for 'X:' paths on Windows (issue 1711)

From: <svnlgo_at_mobsol.be>
Date: 2006-09-19 10:07:13 CEST

Hyrum,

this is a patch for issue 1711, so you can use that ticket.

It's too bad nobody has time to review this patch though. It's the last part
from a series of patches to handle both issue 1711 and issue 2556. As far as
I'm concerned both issues can be closed after committing this patch.

I know it's a large patch and it's Windows only, both more than half of the
patch are much needed extra C and Python testcases.

Anyway, thanks for bringing it back under attention.

Lieven.

Quoting "Hyrum K. Wright" <hyrum_wright@mail.utexas.edu>:

> 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
> > };
> >
> >
> >
> > ------------------------------------------------------------------------
>
>

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 19 10:07:33 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.