Hello again.
I've noticed other problems in SVN (this time they are bugs) for the case of nested working copies.
I. For the case
working_copy_root
|
+----nested_working_copy_root
+----not_versioned_symlink
if "working_copy_root" is SVN 1.6 working copy and "not_versioned_symlink" points to
"nested_working_copy_root" which is SVN 1.7 working copy, "svn info" on the symlink behaves
unexpectedly.
$ svn info not_versioned_symlink
svn: E155036: Please see the 'svn upgrade' command
svn: E155036: Working copy 'working_copy_root' is too old (format 10, created by Subversion 1.6)
I debugged "svn_wc__db_wcroot_parse_local_abspath". The problem is that
svn_wc__db_pdh_create_wcroot "throws an exception" (if I can say so about it) if it is called on SVN
wc 1.6. Instead, it should better silently create an object, which is returned if no better
candidate (like "nested_working_copy_root" in my example
) is found later. And if you would like to change it to that value, don't forget to reset wc_format
to 0 before "goto" call.
II. For the case (see attachments)
working_copy_root
|
+----nested_working_copy_root
|
+----file
Both working copies are 1.7.
working_copy_root has URL "file:///tmp/test/dir1"
nested_working_copy_root has URL file:///tmp/test/dir2/nested_working_copy_root
file has URL file:///tmp/test/dir2/nested_working_copy_root/file
$ svn status -v
3 2 dmit10 .
3 2 dmit10 nested_working_copy_root
S 3 3 dmit10 nested_working_copy_root/file
That is not expected (to my opinion, either "nested_working_copy_root" should be switched, or "nested_working_copy_root/file" should be unversioned).
I don't know what is expected, actually, do you?
I think this is somehow relates to the working copy by path detection.
III. Not a bug, but just some reasonings. You proposed me to add a flag would indicate whether
an unversioned symlink is about to be added to fix a potential problem with working copy by symlink detection.
As SVNKit developer I tried to solve the same problem in SVNKit by adding such a flag (you may have a look at r8996 and r8998 of http://svn.svnkit.com/repos/svnkit/trunk/).
As result I had to add it not into addition method (svn_wc__db_op_add_file analog), but everywhere in "svn add"
implementation to be sure a correct WC is locked, to be sure a correct WC generation (1.6 or 1.7) is detected and so on, in a number of places. I guess in SVN one should do the same that is not beautiful.
By summarazing that all I would like to conclude: maybe it's better to use the method "svn_wc__db_wcroot_parse_local_abspath" only once per SVN command just to detect the working copy root to work with, and then
to pass this working copy root everywhere (to read_info, to XXX_op_add_file, and so on.) to avoid potentially buggy WC root detection every time and to save time?
Originally I wanted to propose a simple patch but the things become to complex for me, so I think I won't propose you the patch.
> On Wed, Mar 14, 2012 at 09:53:02PM +0100, Dmitry Pavlenko wrote:
> > Hello!
> >
> > I've found a potential bug in
> > svn_wc__db_wcroot_parse_local_abspath
> > function.
> >
> > Suppose the following directories structure:
> >
> > working_copy_root
> >
> > +----nested_working_copy_root
> > +----not_versioned_symlink
> >
> > Suppose also that not_versioned_symlink points to
> > "nested_working_copy_root".
> >
> > I want to find the working copy root to which "not_versioned_symlink"
> > belongs, so I call "svn_wc__db_wcroot_parse_local_abspath" with a path
> > to "not_versioned_symlink".
> >
> > There're 2 options.
> > 1. if the cache (db->dir_data in this function) is empty the function
> > will check the symlink's parent (i.e. "working_copy_root") and will call
> > "svn_wc__db_read_info_internal" on it, so that "retry_if_dir" variable
> > will be set to TRUE,
> > and "not_versioned_symlink" will be eventually found as working copy root
> > (because it points to a working copy root).
> >
> > 2. But if the cache already contains a record about "working_copy_root",
> > the function will check the symlink's parent (i.e. "working_copy_root"),
> > discover it in the cache, and hence return "working_copy_root" as a
> > resulting root.
> >
> > So the function depends on the cache contents.
> >
> > The only reason why "svn add" find the correct (in the context of
> > addition) root (i.e. "working_copy_root") is that a record about
> > "working_copy_root" gets into the cache while obtaining a lock on
> > "working_copy_root", so it's just a luck.
>
> This is definitely a potential problem but not a critical bug since
> everything is working as expected -- the current behaviour just hinges
> on an implementation detail and as the person who wrote this code I agree
> that this isn't nice.
>
> To fix this, we could skip the cache lookup for symlinks, and pass a flag
> from adm_ops.c:add_from_disk() via svn_wc__db_op_add_file() to
> svn_wc__db_wcroot_parse_local_abspath(). This flag would indicate whether
> an unversioned symlink is about to be added. This would allow the function
> to select the right wcroot for the symlink.
>
> Would you like to submit a patch for this?
>
> > And btw "svn info not_versioned_symlink" thinks
> > that "not_versioned_symlink" itself is a working copy, not
> > "working_copy_root".
> >
> > The question is what's the expected working copy root should this
> > function return on this example?
>
> We want to allow people to use unversioned symlinks to point at a
> different working copy. For a reason which eludes me some people really
> like to use symlinks that point at externals and were complaining that
> this stopped working after they upgraded from 1.6 to 1.7.
>
> So if the symlink is not versioned and it points to a directory,
> 'svn info <symlink>' should operate on the working copy the directory
> belongs to. If the symlink is versioned, 'svn info <symlink>' should
> operate on the symlink itself.
>
> This rule applies to all subcommands except 'add' which is a special
> case as you rightly point out. It needs to add an unversioned symlink
> to its parent working copy.
Received on 2012-03-20 16:24:06 CET