From: Jonathan Nieder Date: Sat, 28 Jul 2012 19:30:29 GMT Subject: Re: [PATCH 2/7] Change canonicalize_url() to use the SVN 1.7 API when available. Message-ID: <20120728193029.GB3107@burratino> In-Reply-To: <50143700.80900@pobox.com> Michael G Schwern wrote: > On 2012.7.28 6:50 AM, Jonathan Nieder wrote: >> If I am reading Subversion r873487 correctly, in ancient times, >> svn_path_canonicalize() did the appropriate tweaking for URIs. Today >> its implementation is comforting: >> >> const char * >> svn_path_canonicalize(const char *path, apr_pool_t *pool) >> { >> if (svn_path_is_url(path)) >> return svn_uri_canonicalize(path, pool); >> else >> return svn_dirent_canonicalize(path, pool); >> } [...] > I didn't know about that. I don't know what your SVN backwards compat > requirements are, but if that behavior goes back far enough in SVN to satisfy > you folks, then canonicalize_url() should fall back to > SVN::_Core::svn_path_canonicalize(). svn_path_canonicalize() has been usable for this kind of thing since SVN 1.1, possibly earlier. > But try it at the end of the patch > series. The code has to be prepared for canonicalization first. Then how it > actually does it can be improved. Since this part of the series is not tested with SVN 1.7, this is basically adding dead code, right? That could be avoided by reordering the changes to keep "canonicalize_url" as-is until later in the series when the switchover is safe. Thanks. Will play around with this code more. Jonathan