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

On Fri, Jan 26, 2018 at 8:33 AM, Branko Čibej <brane_at_apache.org> wrote:
> On 25.01.2018 19:47, Johan Corveleyn wrote:
>> 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.
>
> Choosing short-term convenience over correctness is the Git way, not the
> Subversion way. The Subversion way is to attain convenient correctness.
> This kind of thoughtless +1 is just intellectual laziness where instead
> there should be discussion of tradeoffs and side effects.

Thanks, Brane. I should have expected such a reply from you,
downplaying my answer as a "thoughtless +1" and "intellectual
laziness". Very useful comment.

Feel free to continue the discussion of tradeoffs and side effects, I
didn't do anything to block that.

> The issues you mention are actual bugs. Recording a replacement where a
> replacement was intended is not. Making it impossible to record a
> replacement when it was intended is a regression.

Note that I said "+1 for not producing replacements *by default*". I
didn't say we should make it impossible. I suppose in the 0,1 % of the
cases where someone really wants a replacement, they should be able to
use some option.

-- 
Johan