[darcs-devel] [patch1715] no longer lie about repo witnesses (and 5 more)

Ben Franksen bugs at darcs.net
Sat Oct 6 19:03:23 UTC 2018


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

>> hunk ./src/Darcs/Repository/Merge.hs 250
>> -                tentativelyAddPatches_ DontUpdatePristine r
>> -                    compression verbosity YesUpdatePending them'
>> +                tentativelyAddPatches_ DontUpdatePristine _repo
>> +                    compression verbosity NoUpdatePending them'
>
> What's this change (YesUpdatePending -> NoUpdatePending) about?
> Sorry, I think I missed it on the first review.

A very good question! I had to think about this for a while. The answer
is: it doesn't matter because a few lines down we overwrite pending with
a new version anyway:

        setTentativePending _repo (effect pw' +>+ resolution)

So the NoUpdatePending saves unnecessary work. It is also cleaner and
more obvious this way, since in the Reorder case we pass
NoUpdatePending, too. I should have recorded that change separately.

>>> In Darcs.Repository.State, you've added two commented out
>>> functions, readTentative and readTentativePending, why?
>
>> Initially I thought these could be used to allow
>> tentativelyMergePatches to operate on the tentative state
>> instead of the recorded one. This would avoid having to commit
>> inside doSuspend. But then I decided to
>> postpone this change. I kept the implementations in there in case
>> we decide to do this at a later stage.
>
> In general I think leaving commented-out code around is just
> storing up future confusion, as it probably will stop compiling
> after a while, and future developers (including future-me :-))
> will be inclined to just delete it anyway.

You are right. Go ahead.

> If you do want to keep it around not being used, I'd suggest
> uncommenting it and adding a comment explaining why it's not
> currently used so that a future developer can easily judge
> whether the time has come to drop it.
>
> Anyway, not a big deal if you still want to keep them as they are.

I don't, please delete them. I have given up on the idea of reading the
tentative state for now. I think the correct way to do that is to
abandon all tentative files, hash everything that isn't already hashed
(head inventory, pending, rebase, etc), and store the hashes of any
intermediate ("tentative") states inside the Repository token. Or, even
better, inside a new "Branch" object, and add that to Repository... ;-)
The hashes are written back to disk to a small file that identifies the
branch only when we finalize the repo changes.

This will be a /major/ refactor, because it means we /have/ to return a
new Repository whenever we change anything and we /have/ to use that new
Repository in order for the changes to have any effect. It will give us
branches for free, though (that is, apart from the necessary changes and
additions to the UI).

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


More information about the darcs-devel mailing list