[darcs-users] darcs patch: resolve issue864: check non-force replace against pending (and 2 more)

[David Roundy]

On Wed, Oct 22, 2008 at 12:46:52AM +0200, Tommy Pettersson wrote:
> Hi,
> 
> On Thu, Oct 16, 2008 at 07:09:52PM +0100, Eric Kow wrote:
> > Hi Tommy,
> > 
> > On Sun, Oct 05, 2008 at 15:44:47 +0200, Tommy Pettersson wrote:
> > > Here is a fix for issue864 -- sort of.
> > 
> > Do you still want these patches to be reviewed / go in?
> > 
> > Thanks!
> 
> > > Sun Oct  5 01:57:19 CEST 2008  Tommy Pettersson <ptp at lysator.liu.se>
> > >   * resolve issue864: check non-force replace against pending
> > >   The replace was checked against pure pristine, so the answer to if it could
> > >   be applied to pending without force was sometimes wrong.
> 
> I started looking deeper into this, but never finished. (I think
> the dialog for reverting changes can be improved.) Anyway, if my
> understanding that new pending changes are added at the end of
> previous pending changes, then I think the above patch is
> correct and should be applied. If my understanding is wrong the
> patch may be incorrect, but the intended change is still how to
> fix issue864.
> 
> > > Sun Oct  5 02:03:16 CEST 2008  Tommy Pettersson <ptp at lysator.liu.se>
> > >   * make replace safe (again)
> > >   If the new token exists in either pending or working, the force flag must
> > >   be used, because otherwise some unrecorded changes may be destroyed when
> > >   those tokens are modified to allow the replace.
> > > 
> > > Sun Oct  5 02:52:23 CEST 2008  Tommy Pettersson <ptp at lysator.liu.se>
> > >   * rewrite issue864 test to check for the real cause of the bug
> 
> These patches will change darcs Replace in a noticeable way, and
> are probably not what the original poster of issue864 intended.
> Why and how is hopefully crystallized in the last paragraph of
> this email.
> 
> Current darcs requires --force only if the replace is unclean in
> *both* recorded and unrecorded. I fail to see the logic in this.
> 
> If the replace is clean in recorded but unclean in unrecorded,
> then unrecorded is modified in a way that can't be exactly
> reverted. This is fine, since editing a file can also change
> unrecorded in a way that is not exactly revertable. If the
> replace is clean in unrecorded but unclean in recorded, then
> darcs will insert hunks in pending. This is also fine, since
> these hunks will not necessarily clash with unrecorded changes,
> so an exact revert is often possible, and when it isn't, the
> situation is no worse than for the above case. But then I wonder
> what's special with an unclean replace in *both* recorded and
> unrecorded. Why does it require the --force flag while the other
> two cases don't? The possible revertability will be no worse in
> this case.
>
> My thinking is the --force flag should be required in all three
> cases, or in none of them, and personally I prefer all of them.

Here's the logic: in only one of the possibilities does force add hunk
patches to pending that haven't been written by the user.  The case
where it's clean in recorded but unclean in unrecorded is a common
case in which the user has already made (or begun) making the change
when she realized that she could be using darcs replace instead.  The
case where it's clean in unrecorded only is the case where she
realized that there would be a conflicting change and has already
fixed it--perhaps by renaming the other variable to something else.
We definitely should *not* require force in either of these cases,
since they're already clean.

However, in the case where replace is going to have to insert new hunk
patches before the insert, it's well worth requiring force, since
we're making changes that the user did not request.

David