Re: svn commit: r1362739 - /subversion/trunk/subversion/tests/cmdline/basic_tests.py

On 18.07.2012 12:25, Bert Huijben wrote:
>
>> -----Original Message-----
>> From: Branko Čibej [mailto:brane_at_wandisco.com]
>> Sent: woensdag 18 juli 2012 11:50
>> To: dev_at_subversion.apache.org
>> Subject: Re: svn commit: r1362739 -
>> /subversion/trunk/subversion/tests/cmdline/basic_tests.py
>>
>> On 18.07.2012 10:51, Philip Martin wrote:
>>> "Bert Huijben" <bert_at_qqmail.nl> writes:
>>>
>>>> In 1.6 all these cases worked because we found a .svn directory with
>>>> metadata in the immediate parent (=the symlinked directory).
>>>>
>>>> I'm not sure if we really want to support all these corner cases
>>>> (including those where this symlink is versioned itself). We never
>>>> intentionally broke this feature.
>>> The 1.6 behaviour could be considered a bug.
>> That's a bit too strong, IMO. The fact is that, up to 1.6, every
>> directory in the working copy was effectively a working copy root, and
>> we even had code that specifically made it possible to acces a WC root
>> through a symbolic link (the main use case was wc-root-on-subst-drive on
>> Windows). So it's reasonable to interpret this as supported behaviour,
>> even if we never intended it.
>>
>>> Following unversioned
>>> symlinks to the root of a working copy is probably simple enough, but
>>> following unversioned symlinks inside a working copy looks far harder.
>>>
>>> svnadmin create repo
>>> svn import -mm repo/format file://`pwd`/repo/A/f
>>> svn co file://`pwd`/repo wc
>>> ln -s f wc/A/g1
>>> ln -s A wc/B1
>>> svn add wc/A/g1 wc/B1
>>> svn ci -mm wc
>>> ln -s f wc/A/g2
>>> ln -s A wc/B2
>>>
>>> wc/A/f is a versioned file; the content can be accessed through a
>>> variety of paths containing symlinks: wc/A/g1, wc/A/g2, wc/B1/f,
>>> wc/B2/f, wc/B1/g1, wc/B1/g2, wc/B2/g1 and wc/B2/g2. Some of the
>>> symlinks are versioned and some are unversioned.
>>>
>>> Now consider:
>>>
>>> echo new-f > wc/A/f
>>> svn st wc/A/g1
>>> svn ci -mm wc/A/g1
>>>
>>> wc/A/g1 is versioned so status shows the status of wc/A/g1 not wc/A/f
>>> and commit does nothing unless wc/A/g1 is changed. That behaviour looks
>>> correct to me.
>> Yes. This is the tricky part. IMO the only rational thing to do is to
>> never try to resolve symlinks if the original target resolves to a known
>> node. So in the commit case the user might be surprised by the different
>> behaviour if the symlink is versioned or not, but at least the
>> difference can be explained.
>>
>>> What about the unversioned wc/A/g2? The new 4193 tests seem to
>> assume
>>> that Subversion operations should follow this symlink through to wc/A/f.
>>> Is it sensible for operations on an unversioned symlink to affect the
>>> target while the same operations on a versioned symlink affect the
>>> source? That strikes me as very confusing.
>>>
>>> How should wc/B2/g1 behave? wc/B2 is unversioned but wc/B2/g1 points
>> to
>>> wc/A/g1 which is versioned. Do operations on wc/B2/g1 affect wc/A/f?
>> I think the only thing we could do is to resolve symlinks in explicit
>> targets iff the original path does not resolve to a known versioned
>> node. That would isolate the extra work in the error case and would not
>> change the behaviour of cases that currently work, nor would it change
>> the way recursive operations work.
>>
>> I also wouldn't worry about the paths in notification callbacks but
>> would just use the resolved paths everywhere; e.g., if "svn ci x/f" gets
>> translated to "svn ci a/f", I wouldn't try to show x/f in the commit
>> notification.
> That is nice behavior in theory, but doesn't really work in many use cases. Scripted users of the client library (or 'svn') expect to see the paths they passed in the results they get. E.g. If you perform a query and expect the result in a hashtable, you don't want the key you need to fetch the item to change.
>
> Another problem is that you can have one path in several formats in the target list of an operation, while we only filter duplicates (like in commit)
>
> libsvn_wc assumes that all paths are unique in their canonical absolute format. Converting from one absolute path to another in the lowest layer breaks all assumptions the higher layers make about the library behavior.
> Storing multiple wcroots (after resolving symlinks) doesn't break things, but when we have the (wcroot, local_relpath) pair things will break.

Good point, I hadn't considered that. So this is looking more and more
like a "wontfix".

> The other solution, trying to resolve when parsing the arguments in 'svn' won't work because we at that point don't know what the arguments are used for yet.

Yes, that's clear; we'd have to at least look at what the arguments
resolve to, and that would entail extra up-front work that I wouldn't
want to see in the code.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download