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.
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.
-- Brane
Received on 2018-01-26 08:33:55 CET