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

RE: [PATCH] fix for issue #2556: support working copies on the root of a (virtual) drive

From: Lieven Govaerts <lgo_at_mobsol.be>
Date: 2006-08-21 23:50:40 CEST

Attached a new version of the issue-2556-wc-on-root patch. The patch is
updated with your remarks (see comments). I also removed the python
regression test (already committed).

Your remarks:
> > + will cause problems elsewhere, anyway. */ if
> > + (svn_path_cstring_from_utf8(&rel_path, rel_path, strpool) !=
> >
> > Here you may leek a pool.

> This function returns an svn_error_t*, which really shouldn't
> be compared to APR_SUCCESS, because they're two totally
> different beasts.
> You'll have to introduce an svn_error_t* variable to hold the
> return value, and call svn_error_clear. For example,
> svn_error_t *err = svn_path_cstring_from_utf8(&rel_path,
> rel_path, strpool); if (err)
> {
> svn_error_clear(err);
> return FALSE;
> }

> Reading it Garrett's way: we may want to rev the APIs just because of the
pool requirement. But, when the pool being
> passed in is NULL (as the old api compat routines would), we'd need to
create our own pool with svn_pool_create(NULL);
> something along these lines:

Included a slightly simpler form of your example.

I didn't rev any of the functions in path.c nor deleted is_canonical. This
can be done, it'll improve the memory usage slightly. Note that this is only
the first patch though, others will follow in which other functions like
svn_path_add_component, svn_path_component_count, svn_path_is_ancestor...
(none of these have a pool parameter) will start using

> There's one more nit: In the patch you say you added X:/ as canonical,
which is true,
> but by using apr_filepath_root, you also added \\?\X:/, \\.\..,
\\server\share and maybe
> some others. I've got no idea how to put it other than 'All forms
considered canonical
> root specs on the platform at hand, amongst which...' or something like
While the new function svn_path_is_root now supports these forms, that
doesn't mean svn now supports them. I don't want to give the impression that
by applying this patch people can actual use those forms, in fact, I'd be
surprised of those work. I used your proposal in the comment of the
svn_path_is_root function.



Prepare fix for issue #2556: abstract root folder checks in
Add support for 'X:/' as a root folder on Windows.

* subversion/include/svn_path.h
  (svn_path_is_root): New function declaration.
  (svn_path_is_empty): Added comment.

* subversion/libsvn_client/commit.c
  (svn_client_commit4): make sure to abort when the iterating through the
   parent folders passese the root folder.

* subversion/libsvn_subr/path.c
  (svn_path_is_root): New function. Tests for all forms considered canonical

   root specs on the platform at hand ('/', 'X:/' etc.)
  (is_canonical): 'X:/' syntax on Windows is canonical.
  (svn_path_join, svn_path_dirname, svn_path_basename,
   svn_path_join_many, previous_segment,
   get_path_ancestor_length): Support the new type of root path on Windows,
    mostly by replacing direct comparisons of path and '/' with a call to
  (svn_path_canonicalize): don't strip the trailing slash if the path is
   of the 'X:/' syntax.

* subversion/tests/libsvn_subr/path-test.c
  (test_is_root): New test for validation of svn_path_is_root.
  (test_path_split, test_join,
   test_basename, test_remove_component): add extra cases for 'X:/'.
  (test_funcs): run the new test test_is_root.

Fix an issue where the status of a working copy on the root of a drive is
'S' (switched), as part of issue #2556.
This is not Windows specific, this code didn't work for working copies on
'/' either.

* subversion/libsvn_wc/status.c
  (assemble_status): add support for working copies at the root of a

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Received on Mon Aug 21 23:52:55 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.