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

r877014 - on Windows, invalid path => svn_node_none [was: svn commit: r930333 - /subversion/branches/1.6.x/STATUS]

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Tue, 06 Apr 2010 13:39:07 +0100

Hi Paul, Bert.

I'm wondering about this r877014 change, which is proposed for 1.6.x
back-port:

[[[
r877014 | rhuijben | 2009-04-02 14:01:48 +0100 (Thu, 02 Apr 2009) | 7 lines

* subversion/libsvn_subr/io.c
  (io_check_path): On Windows, treat a path containing invalid characters as
     a non-existing path. (We already detected invalid drives and invalid
     network paths.) This enables locating the repository root via
     svn_repos_find_root_path() in cases like:
     $ svn mkdir --parents file:///G:/repos/d/f:r/s:t -m ""

------------------------------------------------------------------------
   if (APR_STATUS_IS_ENOENT(apr_err))
     *kind = svn_node_none;
- else if (APR_STATUS_IS_ENOTDIR(apr_err))
+ else if (APR_STATUS_IS_ENOTDIR(apr_err)
+#if WIN32
+ || (APR_TO_OS_ERROR(apr_err) == ERROR_INVALID_NAME)
+#endif
+ )
     *kind = svn_node_none;
   else if (apr_err)
     return svn_error_wrap_apr(apr_err, _("Can't check path '%s'"),
]]]

There seems to be a funny interdependency between io_check_path() and
check_repos_path() and svn_repos_find_root_path().

io_check_path() returns 'none' if the requested entry is not on disk,
obviously, but this change now makes it also return 'none' if the name
is invalid, which it didn't do before.

check_repos_path() says "Return TRUE on errors (which would be
permission errors, probably)", which is a rather rash assumption.

svn_repos_find_root_path() first checks whether the path has some kinds
of invalid characters:

[[[
      /* Try to decode the path, so we don't fail if it contains characters
         that aren't supported by the OS filesystem. The subversion fs
         isn't restricted by the OS filesystem character set. */
      err = svn_utf_cstring_from_utf8(&decoded, candidate, pool);
      if (!err && check_repos_path(candidate, pool))
        [then return 'candidate']
]]]

It looks like the desired effect is being achieved by a rather oddly
layered set of assumptions and interactions in this chain of functions.

Changing [svn_]io_check_path() has far wider potential repercussions
than just the intended result.

What do you think?

- Julian

On Fri, 2010-04-02, pburba_at_apache.org wrote:
> * STATUS: Nominate r877014, including rhuijben's vote via IRC.
 
> + * r877014
> + On Windows, treat a path containing invalid characters as a non-existing
> + path (svn_node_none) rather than raising an error.
> + Justification:
> + Fixes basic_tests.py 37 'use folders with names like 'c:hi'' on
> + Windows, which has been failing over ra_local since r923779 (the
> + reintegration of the 1.6.x-issue-3242-partial branch).
> + Votes:
> + +1: pburba, rhuijben (via IRC)
> +
Received on 2010-04-06 14:39:42 CEST

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