Patch 2161 resolve issue2674 (and a few more)

Title resolve issue2674 (and a few more)
Superseder Nosy List bf
Related Issues
Status needs-review Assigned To

Created on 2021-03-10.11:32:47 by bf, last changed 2021-04-06.22:02:07 by bf.

File name Status Uploaded Type Edit Remove
add-a-check_repair-rule-for-bad-move-patches.dpatch bf, 2021-04-06.22:00:20 application/x-darcs-patch
patch-preview.txt bf, 2021-03-10.11:32:45 text/x-darcs-patch
patch-preview.txt bf, 2021-04-06.22:00:20 text/x-darcs-patch
resolve-issue2674_-moving-unadded-files.dpatch bf, 2021-03-10.11:32:45 application/x-darcs-patch
unnamed bf, 2021-03-10.11:32:45 text/plain
unnamed bf, 2021-04-06.22:00:20 text/plain
See mailing list archives for discussion on individual patches.
msg22654 (view) Author: bf Date: 2021-03-10.11:32:45
5 patches for repository http://darcs.net/screened:

patch 46bb7acd45b236920abf807efeef7544de76d7d0
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Mar  9 17:39:18 CET 2021
  * resolve issue2674: moving unadded files

patch f28ec42284caa0477537bb5741762abf0ab31b02
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Mar  9 17:40:02 CET 2021
  * fail in Darcs.Util.Tree.Monad.rename if source does not exist

patch 7668ca9348d86ef0c7e5772fbc5c91116bffadbf
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Mar  9 18:29:10 CET 2021
  * TreeMonad: improve display of paths in error messages

patch 23bcf38240654ed5bda0f984950c8c490620bf52
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Mar  9 20:05:02 CET 2021
  * bugfix in readPendingAndMovesAndUnrecorded
  This bug was revealed by patch f28ec42284caa0477537bb5741762abf0ab31b02
  "fail in Darcs.Util.Tree.Monad.rename if source does not exist". We must not
  apply the detected moves to the working tree, since detecting these moves
  means that the working tree already has those moves "applied". Also, in case
  we don't use the index, we want to restrict the plain working tree with the
  modified pending tree (i.e. with the detected moves applied), not the
  unmodified one.

patch 5a9ad2a061d26df2db8541590ea4257677a4d1d8
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Mar  9 19:54:37 CET 2021
  * clean up comments and variable names in readPendingAndMovesAndUnrecorded
msg22702 (view) Author: ganesh Date: 2021-04-03.18:20:08
My understanding from discussion on IRC is that you are planning followups
for this, so I'm marking it as followup-in-progress for now. Please change
it back if it is actually ready to review.
msg22713 (view) Author: bf Date: 2021-04-06.22:00:20
Here are my follow-up patches, thanks for the reminder.

As regards what to include in 2.16.4, I think that besides the front-end fix
and the test scripts we should /at least/ include the check/repair fix, so
that people can fix their repos if they are affected.

Repairing means to eliminate the bad moves, which is possible for the same
reason that the bug is, after all, not quite as bad as I thought at first:
since bad move changes are ignored when applying a patch containing them,
they are, effectively, a no-op. In particular, you cannot record anything
that depends on the target of the bad move, which is what the new test
script verifies. It is written so that it works before /and/ after the fix
for issue2674.

With the internal checks added, some commutations that involve bad move
patches will crash darcs. Running 'darcs repair' will fix that.

There is also the question of how and when to repair our own upstream repos.
Compiling a current darcs on darcs.net seems nigh impossible.

Note that the existence of the PInvalid constructor of data PatchMod shows
that the bug was known a long time ago. Indeed the comment mentions the bad
move patch in our darcs repos:

  | PInvalid a
    -- ^ This is an invalid patch
    --   e.g. there is a patch 'Move Autoconf.lhs Autoconf.lhs.in'
    --   where there is no Autoconf.lhs in the darcs repo

This kludge is now thankfully obsolete.

4 patches for repository http://darcs.net/screened:

patch ef6ebddbfa74f22df12bfa5109cd5401f3868276
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Mar 14 10:54:26 CET 2021
  * add a check/repair rule for bad move patches
  This drops a move patch with either non-existing source or existing target.

patch 5e642a9553bd07eb22172bd884ce4c592d95cd96
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Mar 14 11:46:12 CET 2021
  * remove PInvalid constructor from data PatchMod

patch 96537ad0338fa352f27e008e2c6c2f2484ed882c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Mar 14 15:06:22 CET 2021
  * test that we cannot record patches that depend on broken moves

patch 36d18cea5159459c77d7e10081cb05f8bca0f5d7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Mar 17 08:45:45 CET 2021
  * tests/broken_move.sh: if we fail to apply, darcs check should see the bad move
Date User Action Args
2021-03-10 11:32:47bfcreate
2021-03-10 14:40:14bfsetstatus: needs-screening -> needs-review
2021-04-03 18:20:08ganeshsetstatus: needs-review -> followup-in-progress
messages: + msg22702
2021-04-06 22:00:21bfsetfiles: + patch-preview.txt, add-a-check_repair-rule-for-bad-move-patches.dpatch, unnamed
messages: + msg22713
2021-04-06 22:02:07bfsetstatus: followup-in-progress -> needs-review