It is currently impossible to pull between current darcs-stable
and darcs-unstable. It gives the error:

  darcs: bug in get_extra commuting patch:

One has to pull via a middle repo to avoid certain patch
commutations. What I did was I made a tag in darcs-stable and
pulled it to darcs-unstable (via such middle repo). A 'darcs
optimize --reorder' then failed with:

  Failure commuting patches in commute_by called by gpit!

which I guess is the same bug.

I converted the repo to the new darcs2 format (after modernizing
it to remove some 0.9 mergers), and darcs2 still fails with:

  Failure commuting patches in commute_by called by gpit!

As I understand it, darcs Convert flattens the mergers, but the
commutation failure obviously applies to flat mergers as well.
(I'm assuming the commutation failure can not happen to
non-mergers.)

Thanks for the report, Tommy. 

Is there any chance you'd be able to prepared a reduced test case for the issue?

I wonder if the bug would be able to appear if a project was starting with
darcs-2 format repos, instead of trying to switch to them once there's a
conflict like this.

On Sun, Jan 13, 2008 at 03:13:56AM -0000, Mark Stosberg wrote:
> Is there any chance you'd be able to prepared a reduced test case for the issue?
> 
> I wonder if the bug would be able to appear if a project was starting with
> darcs-2 format repos, instead of trying to switch to them once there's a
> conflict like this.

That's what I'm hoping too, that this commutation failure only
can happen in converted repos, not in new ones.

Alas, this part of darcs is (yet) beyond my capabilities. The
only way I know how to construct failing mergers is with bad
luck. It is something about mergers forming (or transforming
through commutations) in a special way that the commutation code
can not inverse, and there is no fix because the very merger
format is insufficient to handle this case.

I think David knows what's it all about. I was only surprised to
see it happen in the darcs2 format.

Tommy, am I right that this involves converting two different darcs1
repositories, and then pulling between them? If so, then this is an expected
problem.  The convert program can only usefully be run once per project (or
rather, related set of repositories), because of how it flattens out mergers
(which depends on the order of patches in a repository.  It may be possible in
some cases to make convert lose less information, but in general, darcs-2 is too
different from darcs-1 to allow lossless transformation.  :(

David

On Mon, Jan 14, 2008 at 08:15:36PM -0000, David Roundy wrote:
> Tommy, am I right that this involves converting two different darcs1
> repositories, and then pulling between them? If so, then this is an expected

Unfortunately not. :-( I pulled the patches together using
darcs1 repos, then I converted this single repo to darcs2
format, and did optimize --reorder in the newly created
converted repo.

I've tared up the repos here (about 5 MiB each):

  http://www.lysator.liu.se/~ptp/pub/d1.tgz
  http://www.lysator.liu.se/~ptp/pub/d2.tgz

d1 is darcs-unstable in darcs1 format with two extra patches on top.
d2 is the result of converting d1 to darcs2 format.
Both repos check --complete ok.
Run darcs2 optimize --reorder to trigger the bug (in both repos).

On Mon, Jan 14, 2008 at 08:15:36PM -0000, David Roundy wrote:
> Tommy, am I right that this involves converting two different darcs1
> repositories, and then pulling between them? If so, then this is an expected

Unfortunately not. :-( I pulled the patches together using
darcs1 repos, then I converted this single repo to darcs2
format, and did optimize --reorder in the newly created
converted repo.

I've tared up the repos here (about 5 MiB each):

  http://www.lysator.liu.se/~ptp/pub/d1.tgz
  http://www.lysator.liu.se/~ptp/pub/d2.tgz

d1 is darcs-unstable in darcs1 format with two extra patches on top.
d2 is the result of converting d1 to darcs2 format.
Both repos check --complete ok.
Run darcs2 optimize --reorder to trigger the bug (in both repos).

On Mon, Jan 14, 2008 at 11:50:06PM -0000, Tommy Pettersson wrote:
> On Mon, Jan 14, 2008 at 08:15:36PM -0000, David Roundy wrote:
> > Tommy, am I right that this involves converting two different darcs1
> > repositories, and then pulling between them? If so, then this is an expected
> 
> Unfortunately not. :-( I pulled the patches together using
> darcs1 repos, then I converted this single repo to darcs2
> format, and did optimize --reorder in the newly created
> converted repo.

I see, yes, this makes sense.  Simply flattening mergers changes the
dependency structure and can make old tags inaccessible.  That's not so
good.  I think (if I get a break from fixing bugs and major performance
regressions) I will go ahead and implement a more faithful convert.
-- 
David Roundy
Department of Physics
Oregon State University

On Mon, Jan 14, 2008 at 11:50:06PM -0000, Tommy Pettersson wrote:
> On Mon, Jan 14, 2008 at 08:15:36PM -0000, David Roundy wrote:
> > Tommy, am I right that this involves converting two different darcs1
> > repositories, and then pulling between them? If so, then this is an expected
> 
> Unfortunately not. :-( I pulled the patches together using
> darcs1 repos, then I converted this single repo to darcs2
> format, and did optimize --reorder in the newly created
> converted repo.

I see, yes, this makes sense.  Simply flattening mergers changes the
dependency structure and can make old tags inaccessible.  That's not so
good.  I think (if I get a break from fixing bugs and major performance
regressions) I will go ahead and implement a more faithful convert.
-- 
David Roundy
Department of Physics
Oregon State University

I'm upgrading this to "urgent" since it would surely affect a number of other
people if Darcs 2 was in wider to use.

I'm working on a patch that may address this, by converting mergers into the
equivalent conflictors.  However, there are many conflicts for which a lossless
conversion isn't possible, so it isn't a complete solution.

Any chance someone can retry this after the relevant patches (one of which
mentions this issue) hit unstable?

Okay, it turns out that my fix doesn't solve Tommy's reported problem,
presumably because the conflict was too complicated to translate losslessly.  I
see two options:  one is to downgrade tags when performing the conversion, so
that they're no longer "real" tags.  The other is to release darcs 2.0 with this
bug, and a stronger caveat that convert doesn't maintain old history.  The third
option would be to only downgrade tags that would exhibit this behavior.  This
third option would probably be optimal, but would greatly slow down convert
(which is already quite slow enough), and would be a royal pain to code up.

David

David Roundy wrote:
> 
> Okay, it turns out that my fix doesn't solve Tommy's reported problem,
> presumably because the conflict was too complicated to translate losslessly.  I
> see two options:  one is to downgrade tags when performing the conversion, so
> that they're no longer "real" tags.  The other is to release darcs 2.0 with this
> bug, and a stronger caveat that convert doesn't maintain old history.  The third
> option would be to only downgrade tags that would exhibit this behavior.  This
> third option would probably be optimal, but would greatly slow down convert
> (which is already quite slow enough), and would be a royal pain to code up.

Thanks for looking into this, David.

Regarding option 1:  What would be the properties of tags which are are 
not "real" tags? Could you still use "--to-tag" and other tag related 
flags with them? If fake tags don't cause more problems than they solve, 
I vote for this option first.

Regarding option 2: What does it mean to not maintain old history? A.) 
That this bug would still happen? Or B) would everything beyond some tag 
just become one giant patch?

Regarding option 3: This sounds worth deferring. If it is a "royal pain" 
to code, then it sounds like it could be a significant delay to the 
release, and might be more likely to have bugs, and might perhaps 
frustrate users due to the slowness.

    Mark

On Fri, Mar 07, 2008 at 03:25:39PM -0000, Mark Stosberg wrote:
> 
> 
> David Roundy wrote:
> > 
> > Okay, it turns out that my fix doesn't solve Tommy's reported problem,
> > presumably because the conflict was too complicated to translate losslessly.  I
> > see two options:  one is to downgrade tags when performing the conversion, so
> > that they're no longer "real" tags.  The other is to release darcs 2.0 with this
> > bug, and a stronger caveat that convert doesn't maintain old history.  The third
> > option would be to only downgrade tags that would exhibit this behavior.  This
> > third option would probably be optimal, but would greatly slow down convert
> > (which is already quite slow enough), and would be a royal pain to code up.
> 
> Thanks for looking into this, David.
> 
> Regarding option 1:  What would be the properties of tags which are are 
> not "real" tags? Could you still use "--to-tag" and other tag related 
> flags with them? If fake tags don't cause more problems than they solve, 
> I vote for this option first.

Fake tags wouldn't work with any of the tag-related flags.

> Regarding option 2: What does it mean to not maintain old history? A.) 
> That this bug would still happen? Or B) would everything beyond some tag 
> just become one giant patch?

Well, we don't store information related to old conflicts.  This means that
the meaning of patches may have changed.  This has already been the case.
Basically, (A).  We'd still have all the old patches, they just won't
necessarily be the same as the original patches.

> Regarding option 3: This sounds worth deferring. If it is a "royal pain" 
> to code, then it sounds like it could be a significant delay to the 
> release, and might be more likely to have bugs, and might perhaps 
> frustrate users due to the slowness.

Agreed.  I don't recall how many hours it took to convert the ghc
repository, but it was quite a while.

It seems like either option 1 or option 2 is all right.  Option 1
needlessly hurts folks who have no conflicts, but option 2 could lead to
erroneous reports that darcs has bugs.

Option 3 might not be so bad, but also might be pretty bad.  I don't know.
In the worst case, it changes convert from being O(N) to O(N^2), which is
pretty much a non-starter.  Also, with option 1, we could safely and easily
preserve any "in-order" tags, which is usually the most common variety.
-- 
David Roundy
Department of Physics
Oregon State University

> with option 1, we could safely and easily
> preserve any "in-order" tags, which is usually the most common variety.

What about preserving the "in-order" tags and for the rest of them, 
maybe they could just be...removed?

I suppose I'd rather they be removed than be broken and confusing.  If 
they are removed, perhaps "convert" should remove which ones are being 
removed.

The "FixForDarcs20" list is getting shorter by the day!

    Mark

On Fri, Mar 07, 2008 at 03:57:11PM -0000, Mark Stosberg wrote:
> 
> 
> > with option 1, we could safely and easily
> > preserve any "in-order" tags, which is usually the most common variety.
> 
> What about preserving the "in-order" tags and for the rest of them, 
> maybe they could just be...removed?
> 
> I suppose I'd rather they be removed than be broken and confusing.  If 
> they are removed, perhaps "convert" should remove which ones are being 
> removed.

We could, but that seems overly-zealous.  The ones which are converted out
of being tags would still contain the dependency information present in the
original tag, it's just that darcs wouldn't assume that they are
"well-behaved".  So I think they'd still be useful.
-- 
David Roundy
Department of Physics
Oregon State University

> We could, but that seems overly-zealous.  The ones which are converted out
> of being tags would still contain the dependency information present in the
> original tag, it's just that darcs wouldn't assume that they are
> "well-behaved".  So I think they'd still be useful.

That sounds worth trying out.

Thanks again for your work on this.

    Mark

The following patch updated the status of issue585 to be resolved in the unstable branch:

* resolve issue585: make old tags that are out-of-order into non-tag patches. 
I also fix a problem where we made all tags invalid.

You can view the patch details online here: 
http://darcs.net/cgi-bin/darcs.cgi/unstable/?c=annotate&p=20080319155917-72aca-fbc4efdc22b6c07987900e6b3c962e1d9acf4c78.gz