[darcs-devel] [patch2161] resolve issue2674 (and a few more)

Ben Franksen bugs at darcs.net
Sun May 9 09:38:16 UTC 2021


Ben Franksen <ben.franksen at online.de> added the comment:

> One other scenario I thought of: what if a fixed darcs pulls from a
> broken repo? I guess it'll fail trying to apply the patch, and the user
> will then have to go and repair the *remote* repo. Is that right? Do we
> need to clearly signpost which repo it is in that case?

It would be great if we could improve the diagnostics to indicate where
the broken patch comes from. The problem is that this is difficult to do.

>>  * resolve issue2674: moving unadded files
> 
>>  * fail in Darcs.Util.Tree.Monad.rename if source does not exist
> 
> I guess this is a lower-level fix that also addresses the same problem?
> (Definitely a good thing to have two guards)

Yes. There are basically two independent ways patch application is
implemented. There is D.R.ApplyPatches which operates directly in IO
i.e. it affects the plain tree at the current working directory, such as
e.g. the working tree. The other one is in TreeMonad, which operates on
a either a plain or a hashed tree; for plain trees the only
implementation we currently use is virtualTreeIO which never modifies
the plain tree (but we can retrieve the modified tree as part of the
result of running virtualTreeIO), whereas with hashedTreeIO we actually
modify the underlying hashed tree by adding new hashed files under
_darcs/pristine.hashed.

This patch adds the missing guards in TreeMonad. Whereas

>>   * remove catching of exceptions for bad moves in DefaultIO

does the same for D.R.ApplyPatches.

[Remark: One reason we cannot simply refactor D.R.ApplyPatches to also
use the TreeMonad (with different initial values for the 'update'
parameter) is that D.R.ApplyPatches handles setpref changes, whereas
TreeMonad ignores them. Indeed, a faithful representation of ApplyState
Prim.V1 would have to include the prefs settings. This is another can of
worms I am reluctant to open but which I'd like to get rid of when we
make the darcs-3 transition.]

>>  * add a check/repair rule for bad move patches
> 
> OK. I had to think a bit about what it means for repository
> compatibility to change the content of a patch without changing the
> patch name, but it's safe as the repaired patch will always have the
> same effect, as you point out in your comments in msg22713. Even if it's
> commuted it could never have turned into something that affects a real file.

Yes. I should add that *all* repair rules modify patches without
changing their identity.

>>  * test that we cannot record patches that depend on broken moves
> 
> Do we really care about this test working before the fix for issue2674?
> The approach seems a bit convoluted.

It is, but I wanted to make sure this is covered because of the concerns
you raised on #darcs when we first discussed this. This test gives
justification (in addition to the theoretical considerations) that we
can really drop the broken move changes from existing patches without
breaking anything, even if they are burried deep inside a repo's history.

If we agree that the changes in this bundle are correct so far, then the
next question is how much of that to include in a new patch release 3.16.4.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch2161>
__________________________________


More information about the darcs-devel mailing list