Re: SVN-4709 shelve: deleted file becomes 'replaced'

On Thu, Jan 25, 2018 at 1:41 PM, Julian Foad <julianfoad_at_apache.org> wrote:
> Branko Čibej wrote:
>>
>> On 24.01.2018 22:32, Julian Foad wrote:
>>>
>>> When 'svn patch' applies an 'add file' patch onto a WC path whose
>>> local schedule is 'delete', it changes the schedule to 'replace'.
>>> [...]
>>> stsp and I discussed on IRC (
>>> http://colabti.org/irclogger/irclogger_log/svn-dev?date=2017-12-15#l19
>>> ) and agreed that this is not what users would generally want or expect.
>>>
>>> I propose to make 'patch' always generate a 'modified' (or unmodified)
>>> schedule when it applies an 'add file' diff (or reverse-applies a
>>> 'delete file' diff) onto a schedule 'deleted' working copy file.
>>> [...]
>>
>>
>> Why is this not what users would expect? "Delete" + "add" has always
>> been "replace" in Subversion. The only other reasonable option I can
>> think of would be to generate a delete/add tree conflict and let the
>> user decide what to do about it. Silently undoing an "svn rm" in the
>> working copy is exactly what I would _not_ expect. Both 'svn rm' and
>> 'svn patch' are explicit user operations and we can't just assume that
>> one or the other were mistakes.
>
>
> I'm not assuming anything was a mistake.
>
> Stefan commented in the IRC chat, "replacements are causing more grief than
> good in general, especially if they happen by accident. i've seen people
> block replacements in pre-commit hooks entirely so if we're given a choice
> between having the default behaviour be replacement or modification, then
> i'd always argue for modification by default. note also that many other vcs
> don't have a replacement concept unless the node kind has changed and nobody
> complains about that."
>
> The theoretical rationale is this. The patch format does not carry ancestry
> information, not even implicitly. Whether a pair of patch operations should
> cause a break in Subversion ancestry is a completely free choice for
> Subversion to make.
>
> 'svn delete' and 'svn add' are explicit *Subversion* operations which carry
> implications about ancestry (and after a local delete the user is free to
> choose "svn revert" instead of "svn add" to get the other result). The 'add'
> and 'delete' operations in a patch file are not and do not.
>
> Formally, yes, it would be nice to offer both outcomes. However, raising a
> conflict without having a nice framework for setting up an automatic
> conflict handling policy is just another barrier to users.

+1 for not producing replacements by default. Intentional replacements
are extremely rare compared to wanting to keep the ancestry intact.

This makes me think of these old issues:

https://issues.apache.org/jira/browse/SVN-3429 ("svn mv A B; svn mv B
A" generates replace without history)

https://issues.apache.org/jira/browse/SVN-4302 (move and move back
breaks nested moves)

I'm glad I don't have to deal with those unintended replacements anymore.

-- 
Johan