You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2011/04/10 00:38:23 UTC

svn commit: r1090699 - in /subversion/trunk/subversion: include/svn_dirent_uri.h libsvn_subr/dirent_uri.c tests/libsvn_subr/dirent_uri-test.c tests/libsvn_subr/path-test.c

Author: rhuijben
Date: Sat Apr  9 22:38:23 2011
New Revision: 1090699

URL: http://svn.apache.org/viewvc?rev=1090699&view=rev
Log:
Make the conversion functions from internal path style to local style assume
that their input is canonical. But just to be safe don't make the dirent
variant fail on non canonical paths.

These functions are used in too many error messages to make them take the
penalty of full canonicalization on all their input. (My profiler says that
just this canonicalize in the to local style function takes over 0.5% of
my checkout time, while it is never needed.)

* subversion/include/svn_dirent_uri.h
  (header): Remove *_local_style from the list of functions that allows non
    canonical paths.

* subversion/libsvn_subr/dirent_uri.c
  (SVN_USE_DOS_PATHS): Update comment.
  (local_style): Remove expensive canonicalization.
  (svn_relpath_local_style): Add maintainer mode only path style verification,
    like we do in other path functions.

* subversion/tests/libsvn_subr/dirent_uri-test.c
  (SVN_USE_DOS_PATHS): Update comment.
  (test_dirent_local_style): Add two testcases.
  (test_relpath_local_style): Remove now invalid case.
  (test_dirent_internal_style): Add new case.

* subversion/tests/libsvn_subr/path-test.c
  (SVN_USE_DOS_PATHS): Define macro and update all users.
  (test_path_canonicalize): Enable testvalue that was disabled.
  (test_path_local_style): Update expected result.

Modified:
    subversion/trunk/subversion/include/svn_dirent_uri.h
    subversion/trunk/subversion/libsvn_subr/dirent_uri.c
    subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
    subversion/trunk/subversion/tests/libsvn_subr/path-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=1090699&r1=1090698&r2=1090699&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_dirent_uri.h (original)
+++ subversion/trunk/subversion/include/svn_dirent_uri.h Sat Apr  9 22:38:23 2011
@@ -60,11 +60,9 @@
  *    - @c svn_dirent_canonicalize()
  *    - @c svn_dirent_is_canonical()
  *    - @c svn_dirent_internal_style()
- *    - @c svn_dirent_local_style()
  *    - @c svn_relpath_canonicalize()
  *    - @c svn_relpath_is_canonical()
  *    - @c svn_relpath_internal_style()
- *    - @c svn_relpath_local_style()
  *    - @c svn_uri_canonicalize()
  *    - @c svn_uri_is_canonical()
  *

Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=1090699&r1=1090698&r2=1090699&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Sat Apr  9 22:38:23 2011
@@ -52,7 +52,8 @@
    the OS! */
 #define SVN_PATH_IS_PLATFORM_EMPTY(s,n) ((n) == 1 && (s)[0] == '.')
 
-/* This check must match the check on top of dirent_uri-tests.c */
+/* This check must match the check on top of dirent_uri-tests.c and
+   path-tests.c */
 #if defined(WIN32) || defined(__CYGWIN__) || defined(__OS2__)
 #define SVN_USE_DOS_PATHS
 #endif
@@ -118,19 +119,6 @@ internal_style(path_type_t type, const c
 static const char *
 local_style(path_type_t type, const char *path, apr_pool_t *pool)
 {
-  switch (type)
-    {
-      case type_dirent:
-        path = svn_dirent_canonicalize(path, pool);
-        break;
-      case type_relpath:
-        path = svn_relpath_canonicalize(path, pool);
-        break;
-      case type_uri:
-      default:
-        return apr_pstrdup(pool, path);
-    }
-
   /* Internally, Subversion represents the current directory with the
      empty string.  But users like to see "." . */
   if (SVN_PATH_IS_EMPTY(path))

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=1090699&r1=1090698&r2=1090699&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Sat Apr  9 22:38:23 2011
@@ -42,7 +42,7 @@
 
 #define SVN_EMPTY_PATH ""
 
-/* This check must match the check on top of dirent_uri.c */
+/* This check must match the check on top of dirent_uri.c and path-tests.c */
 #if defined(WIN32) || defined(__CYGWIN__) || defined(__OS2__)
 #define SVN_USE_DOS_PATHS
 #endif
@@ -2325,8 +2325,6 @@ test_dirent_local_style(apr_pool_t *pool
 #ifdef SVN_USE_DOS_PATHS
     { "A:/",                 "A:\\" },
     { "A:/file",             "A:\\file" },
-    { "a:/",                 "A:\\" },
-    { "a:/file",             "A:\\file" },
     { "dir/file",            "dir\\file" },
     { "/",                   "\\" },
     { "//server/share/dir",  "\\\\server\\share\\dir" },
@@ -2363,7 +2361,6 @@ test_relpath_local_style(apr_pool_t *poo
     const char *result;
   } tests[] = {
     { "",                     "." },
-    { ".",                    "." },
     { "c:hi",                 "c:hi" },
 #ifdef SVN_USE_DOS_PATHS
     { "dir/file",             "dir\\file" },
@@ -2407,6 +2404,7 @@ test_dirent_internal_style(apr_pool_t *p
     { "A:\\file",            "A:/file" },
     { "A:file",              "A:file" },
     { "a:\\",                "A:/" },
+    { "a:/",                 "A:/" },
     { "a:\\file",            "A:/file" },
     { "a:file",              "A:file" },
     { "dir\\file",           "dir/file" },

Modified: subversion/trunk/subversion/tests/libsvn_subr/path-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/path-test.c?rev=1090699&r1=1090698&r2=1090699&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/path-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/path-test.c Sat Apr  9 22:38:23 2011
@@ -44,6 +44,12 @@
    representations */
 #define SVN_EMPTY_PATH ""
 
+/* This check must match the check on top of dirent_uri.c and
+   dirent_uri-tests.c */
+#if defined(WIN32) || defined(__CYGWIN__) || defined(__OS2__)
+#define SVN_USE_DOS_PATHS
+#endif
+
 static svn_error_t *
 test_path_is_child(apr_pool_t *pool)
 {
@@ -179,7 +185,7 @@ test_path_is_url(apr_pool_t *pool)
     { "file:/",                           FALSE },
     { "file:",                            FALSE },
     { "file",                             FALSE },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "X:/foo",        FALSE },
     { "X:foo",         FALSE },
     { "X:",            FALSE },
@@ -436,7 +442,7 @@ test_path_join(apr_pool_t *pool)
     { "file:///X:", "bar", "file:///X:/bar" },
     { "file:///X:foo", "bar", "file:///X:foo/bar" },
     { "http://svn.dm.net", "repos", "http://svn.dm.net/repos" },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
 /* These will fail, see issue #2028
     { "//srv/shr",     "fld",     "//srv/shr/fld" },
     { "//srv",         "shr/fld", "//srv/shr/fld" },
@@ -524,7 +530,7 @@ test_path_join(apr_pool_t *pool)
   TEST_MANY((pool, SVN_EMPTY_PATH, "/", SVN_EMPTY_PATH, NULL), "/");
   TEST_MANY((pool, SVN_EMPTY_PATH, SVN_EMPTY_PATH, "/", NULL), "/");
 
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
 /* These will fail, see issue #2028
   TEST_MANY((pool, "X:", "def", "ghi", NULL), "X:def/ghi");
   TEST_MANY((pool, "X:", SVN_EMPTY_PATH, "ghi", NULL), "X:ghi");
@@ -594,8 +600,7 @@ test_path_basename(apr_pool_t *pool)
     { "X:/abc", "abc" },
     { "X:", "X:" },
 
-#if defined(WIN32) || defined(__CYGWIN__)
-
+#ifdef SVN_USE_DOS_PATHS
 /* These will fail, see issue #2028
     { "X:/", "X:/" },
     { "X:abc", "abc" },
@@ -646,7 +651,7 @@ test_path_dirname(apr_pool_t *pool)
     { "/", "/" },
     { SVN_EMPTY_PATH, SVN_EMPTY_PATH },
     { "X:abc/def", "X:abc" },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "//srv/shr/fld",  "//srv/shr" },
     { "//srv/shr/fld/subfld", "//srv/shr/fld" },
 
@@ -794,7 +799,7 @@ test_path_canonicalize(apr_pool_t *pool)
     { "X:/foo",               "X:/foo" },
     { "X:",                   "X:" },
     { "X:foo",                "X:foo" },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "file:///c:/temp/repos", "file:///C:/temp/repos" },
     { "file:///c:/temp/REPOS", "file:///C:/temp/REPOS" },
     { "file:///C:/temp/REPOS", "file:///C:/temp/REPOS" },
@@ -807,9 +812,7 @@ test_path_canonicalize(apr_pool_t *pool)
     { "//server/share/",      "//server/share" },
     { "//server/SHare/",      "//server/SHare" },
     { "//SERVER/SHare/",      "//server/SHare" },
-/* These will fail, see issue #2028
     { "X:/",                  "X:/" },
-*/
 #else /* WIN32 or Cygwin */
     { "file:///c:/temp/repos", "file:///c:/temp/repos" },
     { "file:///c:/temp/REPOS", "file:///c:/temp/REPOS" },
@@ -848,7 +851,7 @@ test_path_remove_component(apr_pool_t *p
     { "foo/bar",              "foo" },
     { "/foo/bar",             "/foo" },
     { "/foo",                 "/" },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "X:/foo/bar",           "X:/foo" },
     { "//srv/shr/fld",        "//srv/shr" },
     { "//srv/shr/fld/subfld", "//srv/shr/fld" },
@@ -966,7 +969,7 @@ test_path_is_ancestor(apr_pool_t *pool)
     { "http://",        "http://test",     FALSE},
 */
     { "X:foo",           "X:bar",         FALSE},
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "//srv/shr",       "//srv",         FALSE},
     { "//srv/shr",       "//srv/shr/fld", TRUE },
     { "//srv",           "//srv/shr/fld", TRUE },
@@ -1066,7 +1069,7 @@ test_compare_paths(apr_pool_t *pool)
     { "X:foo",        "X:foo",         0},
     { "X:",           "X:foo",         -1},
     { "X:foo",        "X:",            1},
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "//srv/shr",    "//srv",         1},
     { "//srv/shr",    "//srv/shr/fld", -1 },
     { "//srv/shr/fld", "//srv/shr",    1 },
@@ -1129,7 +1132,7 @@ test_path_get_longest_ancestor(apr_pool_
     { "file:///A/C",    "file:///B/D",     ""},
     { "file:///A/C",    "file:///A/D",     "file:///A"},
 
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "X:/",            "X:/",             "X:/"},
     { "X:/foo/bar/A/D/H/psi", "X:/foo/bar/A/B", "X:/foo/bar/A" },
     { "X:/foo/bar/boo", "X:/foo/bar/baz/boz", "X:/foo/bar"},
@@ -1341,7 +1344,7 @@ test_path_is_canonical(apr_pool_t *pool)
     { "fILe:///Users/jrandom/wc", FALSE },
     { "fiLE:///",              FALSE },
     { "fiLE://",               FALSE },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "file:///c:/temp/repos", FALSE },
     { "file:///c:/temp/REPOS", FALSE },
     { "file:///C:/temp/REPOS", TRUE },
@@ -1386,9 +1389,10 @@ test_path_local_style(apr_pool_t *pool)
     { "",                     "." },
     { ".",                    "." },
     { "http://host/dir",      "http://host/dir" }, /* Not with local separator */
-#if defined(WIN32) || defined(__CYGWIN__)
-    { "a:/",                 "A:\\" },
-    { "a:/file",             "A:\\file" },
+#ifdef SVN_USE_DOS_PATHS
+    { "A:/",                 "A:\\" },
+    { "a:/",                 "a:\\" },
+    { "A:/file",             "A:\\file" },
     { "dir/file",            "dir\\file" },
     { "/",                   "\\" },
     { "//server/share/dir",  "\\\\server\\share\\dir" },
@@ -1429,7 +1433,7 @@ test_path_internal_style(apr_pool_t *poo
     { ".",                    "" },
     { "http://host/dir",      "http://host/dir" },
     { "/",                    "/" },
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
     { "a:\\",                 "A:/" },
     { "a:\\file",             "A:/file" },
     { "dir\\file",            "dir/file" },
@@ -1464,7 +1468,7 @@ test_path_internal_style(apr_pool_t *poo
 
 
 /* local define to support XFail-ing tests on Windows/Cygwin only */
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef SVN_USE_DOS_PATHS
 #define WINDOWS_OR_CYGWIN TRUE
 #else
 #define WINDOWS_OR_CYGWIN FALSE