SVN-4709 shelve: deleted file becomes 'replaced'

[Julian Foad <ju...@apache.org>]

When 'svn patch' applies an 'add file' patch onto a WC path whose local 
schedule is 'delete', it changes the schedule to 'replace'.

Sequences such as this...

 > svn rm foo
 > svn diff > rm-foo.diff
 > svn patch --reverse-diff rm-foo.diff

or this...

 > svn add foo
 > svn diff > add-foo.diff
 > svn commit
 > svn patch --reverse-diff add-foo.diff
 > svn patch add-foo.diff

... result in 'foo' being scheduled as '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.

It is not inherently necessary that shelving and manual use of 'svn 
patch' should share the same solution, but in this case I think that is 
best.

- Julian

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

[Troy Curtis Jr <tr...@gmail.com>]

On Wed, Jan 24, 2018 at 6:49 PM Branko Čibej <br...@apache.org> 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'.
> >
> > Sequences such as this...
> >
> > > svn rm foo
> > > svn diff > rm-foo.diff
> > > svn patch --reverse-diff rm-foo.diff
> >
> > or this...
> >
> > > svn add foo
> > > svn diff > add-foo.diff
> > > svn commit
> > > svn patch --reverse-diff add-foo.diff
> > > svn patch add-foo.diff
> >
> > ... result in 'foo' being scheduled as '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.
> >
> > It is not inherently necessary that shelving and manual use of 'svn
> > patch' should share the same solution, but in this case I think that
> > is best.
>
> 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.
>
>
FWIW, I agree with Branko.  By starting a tree conflict the user can chose
a path to go.  If svn marked the file modified, they would not actually
have a way to accomplish a "replace" if that was the desired behavior
(though I suppose a delete, commit, then 'svn patch' could be used in that
scenario).

Troy


-- Brane
>
>

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

[Julian Foad <ju...@apache.org>]

In r1822544 I resolved this issue by making 'shelve' use a revert 
instead of reverse-applying the patch.

In r1822534, r1822549 I gave the 'revert' APIs an option to delete the 
file/dir from disk when reverting a schedule-add file/dir, where 
previously it left the file/dir on disk. This makes such usage much easier.

- Julian

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

[Stefan Sperling <st...@elego.de>]

On Fri, Jan 26, 2018 at 10:00:00AM +0100, Johan Corveleyn wrote:
> 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.

Regardless of this particular issue, making 'svn patch' use the conflict
resolver would be great. It would also make this discussion easier because
we'd have a clear path for handling ambiguous situations.

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

[Johan Corveleyn <jc...@gmail.com>]

On Fri, Jan 26, 2018 at 9:36 AM, Branko Čibej <br...@apache.org> wrote:
> On 26.01.2018 09:14, Johan Corveleyn wrote:
>> On Fri, Jan 26, 2018 at 8:33 AM, Branko Čibej <br...@apache.org> wrote:
>>> On 25.01.2018 19:47, Johan Corveleyn wrote:
>>>> On Thu, Jan 25, 2018 at 1:41 PM, Julian Foad <ju...@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

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

[Branko Čibej <br...@apache.org>]

On 26.01.2018 09:14, Johan Corveleyn wrote:
> On Fri, Jan 26, 2018 at 8:33 AM, Branko Čibej <br...@apache.org> wrote:
>> On 25.01.2018 19:47, Johan Corveleyn wrote:
>>> On Thu, Jan 25, 2018 at 1:41 PM, Julian Foad <ju...@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.

The case for "svn patch" not doing something silently by default is the
same as for "svn merge". Granted that a silent replacement may not be
what the user wants, it is what the user *said*. If there's a conflict
the logical thing to do is to invoke the conflict resolver. That implies
providing the --accept option for default choices.

In this thread we've also ignored a subtle distinction between two cases:

Case A:
$ svn rm foo
$ svn patch x.patch

where x.patch contains a hunk that represents a modification of the
contents of file foo

Case B:
$ svn rm foo
$ svn patch x.patch

where x.patch contains a hunk that represents the creation of file foo

The original post is about case B, but case A is also important. Case A
is a delete/modify conflict whereas case B is a delete/add conflict.
Regardless of what "svn patch" actually does today, I propose that we
should be treating these cases differently. Using --accept=theirs
should, in case A: revert the deletion and apply the edit; in case B:
record a replacement. If the conflict resolver were invoked, it should
offer both options in both cases (and again, I don't know what it
actually does in equivalent cases during merge).

-- Brane

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

[Johan Corveleyn <jc...@gmail.com>]

On Fri, Jan 26, 2018 at 8:33 AM, Branko Čibej <br...@apache.org> wrote:
> On 25.01.2018 19:47, Johan Corveleyn wrote:
>> On Thu, Jan 25, 2018 at 1:41 PM, Julian Foad <ju...@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

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

[Branko Čibej <br...@apache.org>]

On 25.01.2018 19:47, Johan Corveleyn wrote:
> On Thu, Jan 25, 2018 at 1:41 PM, Julian Foad <ju...@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

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

[Johan Corveleyn <jc...@gmail.com>]

On Thu, Jan 25, 2018 at 1:41 PM, Julian Foad <ju...@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

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

[Julian Foad <ju...@apache.org>]

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.

- Julian

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

[Branko Čibej <br...@apache.org>]

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'.
>
> Sequences such as this...
>
> > svn rm foo
> > svn diff > rm-foo.diff
> > svn patch --reverse-diff rm-foo.diff
>
> or this...
>
> > svn add foo
> > svn diff > add-foo.diff
> > svn commit
> > svn patch --reverse-diff add-foo.diff
> > svn patch add-foo.diff
>
> ... result in 'foo' being scheduled as '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.
>
> It is not inherently necessary that shelving and manual use of 'svn
> patch' should share the same solution, but in this case I think that
> is best.

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.

-- Brane

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

[Daniel Shahaf <d....@daniel.shahaf.name>]

Julian Foad wrote on Wed, 24 Jan 2018 21:32 +0000:
> When 'svn patch' applies an 'add file' patch onto a WC path whose local 
> schedule is 'delete', it changes the schedule to 'replace'.
> 
> Sequences such as this...
> 
>  > svn rm foo
>  > svn diff > rm-foo.diff
>  > svn patch --reverse-diff rm-foo.diff
> 
> or this...
> 
>  > svn add foo
>  > svn diff > add-foo.diff
>  > svn commit
>  > svn patch --reverse-diff add-foo.diff
>  > svn patch add-foo.diff
> 
> ... result in 'foo' being scheduled as 'replace'.

In both of these examples, the result is a working copy in which a file
is scheduled to be replaced (without history) by another file that is
textually identical to it.  Do you also have examples for wanting to
reduce a replace into a modification where the replacing file would be
textually different from the replaced file?

> It is not inherently necessary that shelving and manual use of 'svn 
> patch' should share the same solution, but in this case I think that is 
> best.

Cheers,

Daniel