[darcs-devel] inconsistencies in Darcs.Patch.Depends

Ben Franksen ben.franksen at online.de
Wed Jun 13 21:29:06 UTC 2018


Am 10.06.2018 um 13:24 schrieb Ganesh Sittampalam:
> On 09/06/2018 21:17, Ben Franksen wrote:
>> The docs for Darcs.Patch.Depends.getUncovered say
>>
>> -- | @getUncovered ps@ returns the 'PatchInfo' for all the patches in
>> --   @ps@ that are not depended on by anything else *through explicit
>> --   dependencies*. [...]
>>
>> This is a sensible specification but the implementation does not satisfy
>> it: it only considers explicit dependencies of tags and completely
>> ignores those of normal patches.
>>
> [...]
>>
>> IMO a sensible definition of "covered" would be: the transitive closure
>> of the relation "explicitly depended upon". So, a is covered by b if b
>> explicitly depends on a or there is a patch p covered by b that
>> explicitly depends on a.
>>
>> If getUncovered were renamed to uncoveredByTags then it would be
>> consistently named with respect to this definition.
> 
> I agree the implementation of getUncovered only considers explicit
> dependencies of tags. Doesn't that mean that the definition of covered
> should be the transitive closure of "explicitly depended on by a tag"?
> Other that, I agree with your suggestion.
> 
> I can't figure out why getUncovered only looks at tags - it doesn't seem
> logically necessary,

I think it is really a matter of definition and the way it is defined is
exactly as you say and observed below. The call (getUncovered patchset)
gives you exactly the list of patch infos that a new tag recorded on top
of the patchset would explicitly depend on. This is the way it is used
in the implementation of the tag command. And it is consistent with how
it is used in splitOnTag and slightlyOptimizePatchset: here we test, for
some patch info i of a tag, if (getUncovered patchset == [i]), in order
to see if the tag is clean.

It would be possible to use another definition that takes explicit
dependencies between non-tags into account. This would (slightly) reduce
the dependency list of tags, but that would be a change in patch
semantics and we must not do that (see below).

> and it violates my expectation that a tag is just a
> patch with no content and a special name.

I think this is still true, technically, it is just that the internal
representation of tags does not make use of explicit dependencies of
non-tags. This is probably due to history, I guess explicit dependencies
for non-tags were introduced as an afterthought.

> I double-checked that this behaviour flows through to the UI, and it
> does: if you have 'b' depending on 'a', a tag will still depend on both
> 'a' and 'b'.

Hm, and this means tags are also stored in this way, so this behavior
must be preserved, otherwise older darcs versions will misinterpret new
tags.

Thanks for the clarification. I will update the docs accordingly.

Cheers
Ben



More information about the darcs-devel mailing list