On Fri, Jan 26, 2018 at 9:36 AM, Branko Čibej <brane_at_apache.org> wrote:
> On 26.01.2018 09:14, Johan Corveleyn wrote:
>> 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.
>
> Yup. You're welcome.
>
>> 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.
>
> That's the same as saying that "svn merge" should solve any particular
> tree conflict a given way by default and only start asking questions if
> given some option. After all, the solution to the vast majority of tree
> (and even text) conflicts is usually obvious.
Yes, and +1 to that, very thoughtlessly and intellectually lazy.
If there is an obvious resolution, just do it (only if there is
ambiguity, ask the user). Except for those users that really, really
want to be bogged down by every trivial conflict. I'd say less that
0,1 % of our users is even interested in thinking about tree
conflicts.
One of the problems with unintended replacements is that it's
impossible to fix the damage afterwards. Line of history gets broken,
and we can't fix that in the history in the backend.
--
Johan
Received on 2018-01-26 10:00:35 CET