[darcs-users] [patch262] Resolve issue1014: mark the test as pass... (and 4 more)

Petr Rockai me at mornfall.net
Fri Jun 4 13:22:17 UTC 2010


Re,

Eric Kow <kowey at darcs.net> writes:
> On Thu, Jun 03, 2010 at 15:32:43 +0000, Petr Ročkai wrote:
>> Thu Jun  3 14:29:57 CEST 2010  Petr Rockai <me at mornfall.net>
>>   * Resolve issue1014: mark the test as passing. Likely addressed by NewSet.
>
> I find this a bit too good to be true... but here's David's patch
> * resolve issue1014: use merge_them in pull
>
> I can understand that the bug in get_extra just goes away in the NewSet
> work.  But is the fundamental bug solved actually solved, or has it
> swept under the rug?
Well, if this still induced commutation failures, they would still crash
darcs. So no, we are not silently ignoring the bug. I don't quite
understand how this bug actually came into existence, since I don't
think the problem is in any way inherent. It indeed looks like an
implementation barf in get_extra. Which no longer exists...

Also, my implementation is somewhat different, since in our branch,
darcs actually needs the common/uncommon sequences for features that did
not exist at the time David did this. So I have re-implemented
findCommonAndUncommon (used to be get_common_and_uncommon) in terms of
findCommonWithThem which is just a glorified partitionFL. I.e. this
should all be safe. There's no way really it could care the least about
duplicate patches.

If it was however an inherent bug, the partitionFL would break, since
the partitioning point would be ill-defined: there would be patches that
need to go to the left while at the same time they depend on patches
that need to go to the right. This however does not happen. It would
still happen in cases 1 and 2 that you describe below.

> The wiki says that bug in get_extra happens when 
>
>   This bug is triggered if for whatever reason Darcs finds itself in a position
>   where a patch which is shared by both repos appears to depend on a patch
>   which is local to one of them only. Such a dependency would be absurd. We can
>   divide this error into three known cases:
>   
>   1. Darcs thinks a patch is shared, but it's actually local to one of the repositories
>   2. The contrary: Darcs mistakenly believes a shared patch is local
>   3. There is a real dependency and it indicates a deep Darcs bug (see issue1014)
>
> Sorry if that's in confusing kowey-language.  Anyway the point is that
> issue1014 is one of the few cases where the bug in get_extra really is
> legitimate and not just some incidental nonsense thing like an encoding
> problem.
>
> So I'm a bit surprised that it's just been fixed like that, since it's about
> the duplicate patch shenanigans.  If we're not making error messages anymore,
> then it almost sounds like we just need to be testing more aggressively to
> reveal the real problem.  :-/
I don't think so. Hopefully, the above explanation is good enough.

> Hmm!  Hopefully you'll come back to me with a sort of "ah, but NewSet *does*
> fundamentally address these issues because X Y and Z" and I'll just apply these
> patches and be happy.
I think there's no fundamental issue in the first place.

[snip]

Yours,
   Petr.


More information about the darcs-users mailing list