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-14 22:29:14 CET