[darcs-devel] What should be in a branch?

[Ben Franksen]

Am 10.05.21 um 18:31 schrieb Ganesh Sittampalam:
> On 10/05/2021 11:01, Ben Franksen wrote:
> 
>> When we remove a patch from our repo/branch that doesn't commute past
>> the unrevert changes, we convert it to its effect and try to commute as
>> much of it as we can past the changes to unrevert, dropping those parts
>> we can commute past, and adding the rest as dependencies to the unrevert
>> changes. We already use a very similar approach when creating the
>> unrevert state in the first place, i.e. in the revert command (where,
>> after selecting changes, we commuteWhatWeCanRL (to_keep :> to_revert).
>>
>> This strikes me as more useful and also more user-friendly because it
>> eliminates the "this will make unrevert impossible" warning+prompt. This
>> is something we can do now and is not difficult to implement.
> 
> It sounds fine. To be explicit: by "we can do now" you mean that it
> doesn't depend on branched repositories and could be implemented before
> that?

I meant even without changing the storage format of the unrevert state
from patch bundle to inventory. But this is not quite as unproblematic
as I thought, see below.

> One gotcha would be that if you unpull a patch and then pull it again,
> you can end up with a conflict on unrevert because anything that got
> attached to the unrevert is now also in the main repo.

Yeah, see below, too.

> The logical conclusion of that is that we should also attach the inverse
> of any conflicting patches we pull. At this point it starts looking a
> lot like rebase. I'm not really sure what conclusion to draw from tha
> tthough.

I agree that this would start to resemble rebase. So let's not go there...

>> Now, there is nothing in this new procedure that has to be done eagerly
>> i.e. when we remove the patch. Instead, we could keep the unrevert
>> context untouched and do it only when the user actually wants to
>> unrevert something. This means we don't need to do anything when
>> switching branches (or when we remove patches from a branch).
> 
> Suppose you have two branches, branch 1 containing A and branch 2
> containing A;B, and the user is currently on branch 2. Would you see
> unrevert behaving differently if they unpull B versus switching to branch 1?

No, I think these two scenarios are naturally treated in exactly the
same manner.

> Doing it lazily in all cases also addresses my point above about what
> happens if a patch gets re-pulled, 

Precisely.

> but it could also mean keeping B
> around when the user intended to drop it. I don't know if that actually
> matters.

Good point. Assuming we keep the current implementation of unrevert as a
bundle, obliterating a patch may remove the last reference to it. But in
our unrevert bundle, the patch is only present in the context, that is,
we only have the meta data, not the patch itself. This means we cannot
simply keep the unrevert bundle untouched: we have to re-generate it, so
it includes all patches missing from _darcs/hashed_inventory in full.

An alternative implementation that uses an inventory for unrevert
doesn't have this problem, since inventories contain the content hash of
every patch in the context. (We'd have to add this inventory to the root
set, of course, to ensure these patches won't be GC'ed).

Cheers
Ben
-- 
I would rather have questions that cannot be answered, than answers that
cannot be questioned.  -- Richard Feynman