darcs

Issue 13 pull => bug in function reconcile_unwindings (1.0.8)

Title pull => bug in function reconcile_unwindings (1.0.8)
Priority bug Status duplicate
Milestone Resolved in
Superseder pull => bug in function reconcile_unwindings (** minimal test case **)
View: 194
Nosy List brendan, darcs-devel, dmitry.kurochkin, kowey, thorkilnaur, tommy, wilx
Assigned To droundy
Topics Conflicts

Created on 2005-11-18.13:35:47 by brendan, last changed 2009-08-27.13:50:49 by admin.

Files
File name Uploaded Type Edit Remove
pull.log brendan, 2005-11-19.20:23:47 text/plain
tailor.log brendan, 2005-11-19.20:23:46 text/plain
Messages
msg35 (view) Author: brendan Date: 2005-11-18.13:35:46
Darcs 1.0.4 on OS X.4.3 (from the current fink package): testing
tailor I tried to convert the tailor repository itself from darcs to
hg and received the following message eventually:

$ darcs pull --all --quiet --match "hash 20050705185703-97f81-df90b7216fddae7b78b3ef213a49d418c32d5a19.gz"
returned status 1 saying
darcs: bug in darcs!
Impossible case at PatchCommute.lhs:1321 compiled 22:26:57 Nov 15 2005
Please report this to bugs@darcs.net,
If possible include the output of 'darcs --exact-version'.

That output, FWIW, is:

darcs compiled on Nov 15 2005, at 22:36:22
unknown

Additional datapoints:
I originally pulled the repository onto a case-insensitive HFS
partition. This test was conducted on a case-sensitive HFS disk image,
but the source repository was copied from the insensitive partition
first...
msg50 (view) Author: droundy Date: 2005-11-19.13:00:56
On Fri, Nov 18, 2005 at 01:35:47PM +0000, Brendan Cully wrote:
> Darcs 1.0.4 on OS X.4.3 (from the current fink package): testing
> tailor I tried to convert the tailor repository itself from darcs to
> hg and received the following message eventually:
> 
> $ darcs pull --all --quiet --match "hash 20050705185703-97f81-df90b7216fddae7b78b3ef213a49d418c32d5a19.gz"
> returned status 1 saying
> darcs: bug in darcs!
> Impossible case at PatchCommute.lhs:1321 compiled 22:26:57 Nov 15 2005
> Please report this to bugs@darcs.net,
> If possible include the output of 'darcs --exact-version'.

It looks like tailor is an old repository using particularly old formats.
It's also odd that this commute is even being attempted.  A solution is to
run darcs optimize --modernize-patches on the tailor repository before
doing the conversion.  Let me know if this fixes the problem, and if it
does we can modify darcs to give a more informative error message in this
case.

I don't think this is something worth fixing apart from that (assuming I've
diagnosed it properly), since it's only an issue for very old repositories
(I think we stopped creating merger 0.9s back before 0.9.14 or something),
and only when doing "interesting" stuff with merge conflicts that happened
back when darcs didn't know how to handle them as well as it does now.

Thanks for the bug report! ...and boy is it nice to have that line number
information!  :)
-- 
David Roundy
http://www.darcs.net
msg52 (view) Author: brendan Date: 2005-11-19.20:23:47
On Saturday, 19 November 2005 at 13:00, David Roundy wrote:
> 
> David Roundy <droundy@darcs.net> added the comment:
> 
> On Fri, Nov 18, 2005 at 01:35:47PM +0000, Brendan Cully wrote:
> > Darcs 1.0.4 on OS X.4.3 (from the current fink package): testing
> > tailor I tried to convert the tailor repository itself from darcs to
> > hg and received the following message eventually:
> > 
> > $ darcs pull --all --quiet --match "hash 20050705185703-97f81-df90b7216fddae7b78b3ef213a49d418c32d5a19.gz"
> > returned status 1 saying
> > darcs: bug in darcs!
> > Impossible case at PatchCommute.lhs:1321 compiled 22:26:57 Nov 15 2005
> > Please report this to bugs@darcs.net,
> > If possible include the output of 'darcs --exact-version'.
> 
> It looks like tailor is an old repository using particularly old formats.
> It's also odd that this commute is even being attempted.  A solution is to
> run darcs optimize --modernize-patches on the tailor repository before
> doing the conversion.  Let me know if this fixes the problem, and if it
> does we can modify darcs to give a more informative error message in this
> case.

I tried this, but the results seem to be the same. I've attached a
larger piece of the tailor log in case more context helps. I've also
attached a bit of the result of darcs pull --dry-run after tailor has
bootstrapped the target repository (tailor will proceed to pull
patches one at a time in the order they appear in --dry-run, until it
reaches the problematic patch). It looks like something's gone wrong
with the patch marked 'UNDO'? I'm a little suspicious of the fact that
the patch marked "(bis)" is getting pulled before the original and the
undo...

by the way, there's one other nit regarding --dry-run: I tried
converting the darcs repository too, but dry-run produces patch dates
according to TZ. This is fine for the current patch format, but
there's no way for tailor to use it to produce the correct hash for
old patches where the date field was in the form:
 Sun Oct 20 20:04:01 EDT 2002. The old hash IDs are based on this
timestamp in its original timezone, and that tz isn't available in
--dry-run. I can't think of a solution that doesn't involve modifying
the output of dry-run. Ideally it would just give the hash and save
tailor the parsing :)

> I don't think this is something worth fixing apart from that (assuming I've
> diagnosed it properly), since it's only an issue for very old repositories
> (I think we stopped creating merger 0.9s back before 0.9.14 or something),
> and only when doing "interesting" stuff with merge conflicts that happened
> back when darcs didn't know how to handle them as well as it does now.
> 
> Thanks for the bug report! ...and boy is it nice to have that line number
> information!  :)

if you say so. I tried reading that line and discovered that I don't
know haskell :)
Attachments
msg64 (view) Author: droundy Date: 2005-11-22.14:52:26
I think this *may* be a fix for this bug.  It looks like the problem wasn't
with old mergers, but rather with rolled-back mergers, and if I had read
the code more carefully the first time, I would have seen that.  Brendan,
can you try compiling darcs with the (hopefully properly) attached patch,
and see if this solves your problem?

I'm a bit hesitant with this patch, since I don't really understand when
this will happen, which is why it was marked as an impossible case.  It may
turn out that the case really *is* impossible, and that something else is
going wrong that is leading to this situation.  :(

David

Tue Nov 22 09:30:05 EST 2005  David Roundy <droundy@darcs.net>
  * [issue13] remove impossible case at PatchCommutes.lhs line 1321.

New patches:

[[issue13] remove impossible case at PatchCommutes.lhs line 1321.
David Roundy <droundy@darcs.net>**20051122143005] {
hunk ./PatchCommute.lhs 1321
-          unglump _ = impossible
+          unglump (Merger False g' _ _ p1' p2') = glump g' p1' p2'
}

Context:

[TAG 1.0.4
David Roundy <droundy@darcs.net>**20051113134431] 
Patch bundle hash:
ac6f3d79db12663ad05cb0b6e155a571d314a46f
msg68 (view) Author: brendan Date: 2005-11-23.05:26:24
On Tuesday, 22 November 2005 at 14:52, David Roundy wrote:
> 
> David Roundy <droundy@darcs.net> added the comment:
> 
> I think this *may* be a fix for this bug.  It looks like the problem wasn't
> with old mergers, but rather with rolled-back mergers, and if I had read
> the code more carefully the first time, I would have seen that.  Brendan,
> can you try compiling darcs with the (hopefully properly) attached patch,
> and see if this solves your problem?
> 
> I'm a bit hesitant with this patch, since I don't really understand when
> this will happen, which is why it was marked as an impossible case.  It may
> turn out that the case really *is* impossible, and that something else is
> going wrong that is leading to this situation.  :(

After applying this patch, I got the following error when building
darcs:

ghc  -cpp  -package parsec -package unix -O -funbox-strict-fields -Wall -Werror -package util -I. -DHAVE_CURSES -DHAVE_CURL -c PatchCommute.lhs

PatchCommute.lhs:1320:10:
    Warning: Pattern match(es) are non-exhaustive
             In the definition of `unglump':
                 Patterns not matched:
                     NamedP _ _ _
                     Move _ _
                     DP _ _
                     FP _ _
                     ...
make: *** [PatchCommute.o] Error 1

I'm willing to bet the fix is trivial, but I can barely read ocaml and
haskell still sends me crying to mommy.
msg71 (view) Author: droundy Date: 2005-11-23.12:34:09
...
> PatchCommute.lhs:1320:10:
>     Warning: Pattern match(es) are non-exhaustive
>              In the definition of `unglump':
...
> I'm willing to bet the fix is trivial, but I can barely read ocaml and
> haskell still sends me crying to mommy.

That just means I was lame and lazy and didn't bother even compiling the
patch.  Sorry about that.  The following patch will at least compile.  In
case you're curious, the error comes because we have -Werr on, and the
compiler warns when we don't explicitely mention all possible arguments
when using pattern matching--and I removed the "impossible" case, which was
the catchall for other possible errors.

David

New patches:

[[issue13] remove impossible case at PatchCommutes.lhs line 1321.
David Roundy <droundy@darcs.net>**20051123122636] hunk ./PatchCommute.lhs 1321
+          unglump (Merger False g' _ _ p1' p2') = glump g' p1' p2'

Context:

[TAG 1.0.4
David Roundy <droundy@darcs.net>**20051113134431] 
Patch bundle hash:
49ecc7562eb4ade0c1a0fbcf227405773fbc5be6
msg86 (view) Author: brendan Date: 2005-11-26.05:28:06
On Wednesday, 23 November 2005 at 12:34, David Roundy wrote:
> 
> David Roundy <droundy@darcs.net> added the comment:
> 
> ...
> > PatchCommute.lhs:1320:10:
> >     Warning: Pattern match(es) are non-exhaustive
> >              In the definition of `unglump':
> ...
> > I'm willing to bet the fix is trivial, but I can barely read ocaml and
> > haskell still sends me crying to mommy.
> 
> That just means I was lame and lazy and didn't bother even compiling the
> patch.  Sorry about that.  The following patch will at least compile.  In
> case you're curious, the error comes because we have -Werr on, and the
> compiler warns when we don't explicitely mention all possible arguments
> when using pattern matching--and I removed the "impossible" case, which was
> the catchall for other possible errors.
> 
> David
> 
> New patches:
> 
> [[issue13] remove impossible case at PatchCommutes.lhs line 1321.
> David Roundy <droundy@darcs.net>**20051123122636] hunk ./PatchCommute.lhs 1321
> +          unglump (Merger False g' _ _ p1' p2') = glump g' p1' p2'
> 
> Context:
> 
> [TAG 1.0.4
> David Roundy <droundy@darcs.net>**20051113134431] 
> Patch bundle hash:
> 49ecc7562eb4ade0c1a0fbcf227405773fbc5be6

I tried this patch, and it turned into a different bug report:

21:22:47 [I] Changeset "Handle "INITIAL" as --revision under CVS (bis)"
21:22:47 [I] Log message: It's now possible to give "INITIAL" as --revision argument when
bootstrapping from a CVS repository: tailor will look for the first
change in the specified branch. This is a rewrite of the same patch
done in the "old" branch.
21:22:47 [I] /Volumes/Casey/darcshg $ darcs pull --all --quiet --match "hash 20050705234259-97f81-6adff844011f0bb8922c6c43f392c755df74e3f2.gz"
21:22:51 [I] [Ok]
21:22:51 [I] /Volumes/Casey/darcshg $ darcs changes --match "hash 20050705234259-97f81-6adff844011f0bb8922c6c43f392c755df74e3f2.gz" --xml-output --summ
21:22:51 [I] [Ok]
vcpx/cvs.py
vcpx/cvsps.py
21:22:51 [I] -*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*
21:22:51 [I] Changeset "Handle "INITIAL" as --revision under CVS"
21:22:51 [I] Log message: It's now possible to give "INITIAL" as --revision argument when
bootstrapping from a CVS repository: tailor will look for the first
change in the specified branch.
21:22:51 [I] /Volumes/Casey/darcshg $ darcs pull --all --quiet --match "hash 20050705185703-97f81-9d5e40ba59786ffb484e2c145377322d3b3bd295.gz"
21:22:55 [I] [Ok]
21:22:55 [W] Conflict after 'darcs pull': vcpx/cvs.py  vcpx/cvsps.py
21:22:55 [I] /Volumes/Casey/darcshg $ darcs changes --match "hash 20050705185703-97f81-9d5e40ba59786ffb484e2c145377322d3b3bd295.gz" --xml-output --summ
21:22:55 [I] [Ok]
21:22:55 [I] Reverting changes to ./vcpx/cvs.py  ./vcpx/cvsps.py, to solve the conflict
21:22:55 [I] /Volumes/Casey/darcshg $ darcs revert --all "./vcpx/cvs.py " ./vcpx/cvsps.py
21:22:56 [I] [Ok]
README
vcpx/cvs.py
vcpx/cvsps.py
vcpx/tailor.py
21:22:56 [I] -*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*
21:22:56 [I] Changeset "UNDO: Handle "INITIAL" as --revision under CVS"
21:22:56 [I] Log message: It's now possible to give "INITIAL" as --revision argument when
bootstrapping from a CVS repository: tailor will look for the first
change in the specified branch.
21:22:56 [I] /Volumes/Casey/darcshg $ darcs pull --all --quiet --match "hash 20050705185703-97f81-df90b7216fddae7b78b3ef213a49d418c32d5a19.gz"
21:22:59 [W] [Status 1]
21:22:59 [C] Couldn't apply changeset
21:22:59 [I] 585 pending changesets in state file
21:22:59 [C] Upstream change application failed
Failure applying upstream changes: /Volumes/Casey/darcshg $ darcs pull --all --quiet --match "hash 20050705185703-97f81-df90b7216fddae7b78b3ef213a49d418c32d5a19.gz" returned status 1 saying
darcs: bug in darcs!
in function reconcile_unwindings
Original patch:
merger 0.0 (
merger 0.0 (
hunk ./vcpx/cvs.py 317
-        
-        entries = CvsEntries(root)
-        youngest_entry = entries.getYoungestEntry()
-        if youngest_entry is None:
-            raise EmptyRepositoriesFoolsMe("The working copy '%s' of the CVS repository seems empty, don't know how to deal with that." % root)
-        
+v v v v v v v
+
+        if branch is None:
+            entries = CvsEntries(root)
+            youngest_entry = entries.getYoungestEntry()
+            if youngest_entry is None:
+                raise EmptyRepositoriesFoolsMe("The working copy '%s' of the CVS repository seems empty, don't know how to deal with that." % root)
+
+            branch = ''
+            fname = join(root, 'CVS', 'Tag')
+            if exists(fname):
+                tag = open(fname).read()
+                if tag[0] in 'NT':
+                    branch=tag[1:-1]
+
+^ ^ ^ ^ ^ ^ ^
regrem 0.0 (
hunk ./vcpx/cvs.py 317
-        
-        entries = CvsEntries(root)
-        youngest_entry = entries.getYoungestEntry()
-        if youngest_entry is None:
-            raise EmptyRepositoriesFoolsMe("The working copy '%s' of the CVS repository seems empty, don't know how to deal with that." % root)
-        
+
+        if branch is None:
+            entries = CvsEntries(root)
+            youngest_entry = entries.getYoungestEntry()
+            if youngest_entry is None:
+                raise EmptyRepositoriesFoolsMe("The working copy '%s' of the CVS repository seems empty, don't know how to deal with that." % root)
+
+            branch = ''
+            fname = join(root, 'CVS', 'Tag')
+            if exists(fname):
+                tag = open(fname).read()
+                if tag[0] in 'NT':
+                    branch=tag[1:-1]
+
hunk ./vcpx/cvs.py 317
-        
-        entries = CvsEntries(root)
-        youngest_entry = entries.getYoungestEntry()
-        if youngest_entry is None:
-            raise EmptyRepositoriesFoolsMe("The working copy '%s' of the CVS repository seems empty, don't know how to deal with that." % root)
-        
+
+        if branch is None:
+            entries = CvsEntries(root)
+            youngest_entry = entries.getYoungestEntry()
+            if youngest_entry is None:
+                raise EmptyRepositoriesFoolsMe("The working copy '%s' of the CVS repository seems empty, don't know how to deal with that." % root)
+
+            branch = ''
+            fname = join(root, 'CVS', 'Tag')
+            if exists(fname):
+                tag = open(fname).read()
+                if tag[0] in 'NT':
+                    branch=tag[1:-1]
+
)
)
merger 0.0 (
regrem 0.0 (
hunk ./vcpx/cvs.py 336
-            youngest_ts = youngest_entry.timestamp
-            youngest_ts -= timedelta(days=15)
-            since = youngest_ts.isoformat(sep=' ')
+            since = None
hunk ./vcpx/cvs.py 336
-            youngest_ts = youngest_entry.timestamp
-            youngest_ts -= timedelta(days=15)
-            since = youngest_ts.isoformat(sep=' ')
+            since = None
)
hunk ./vcpx/cvs.py 336
-            youngest_ts = youngest_entry.timestamp
-            youngest_ts -= timedelta(days=15)
-            since = youngest_ts.isoformat(sep=' ')
+v v v v v v v
+            since = None
+^ ^ ^ ^ ^ ^ ^
)
)
Please report this to bugs@darcs.net
If possible include the output of 'darcs --exact-version'.
msg953 (view) Author: wilx Date: 2006-09-02.13:30:18
I think I've stumbled on this, too. But in 1.0.8 while trying to mirror tailor
from darcs repo to monotone repo:

darcs: bug in darcs!
Impossible case at PatchCommute.lhs:1322 compiled 09:45:36 Jul  5 2006
Please report this to bugs@darcs.net,
If possible include the output of 'darcs --exact-version'.

logout::wilx:~/mtn-tailor> darcs --exact-version
darcs compiled on Jul  5 2006, at 09:51:33
# configured Fri Jun 16 14:55:21 EDT 2006
./configure --no-create --no-recursion

Context:

[TAG 1.0.8
Tommy Pettersson <ptp@lysator.liu.se>**20060616160213]
msg1992 (view) Author: kowey Date: 2007-08-03.17:58:17
Marking as duplicate of issue194.  Thanks for the report!
History
Date User Action Args
2005-11-18 13:35:47brendancreate
2005-11-19 13:00:56droundysetstatus: unread -> unknown
messages: + msg50
2005-11-19 20:23:47brendansetfiles: + tailor.log, pull.log
messages: + msg52
2005-11-22 14:52:27droundysetmessages: + msg64
2005-11-23 05:26:24brendansetmessages: + msg68
2005-11-23 12:34:10droundysetmessages: + msg71
2005-11-26 05:28:06brendansetmessages: + msg86
2006-09-02 13:30:23wilxsetnosy: + wilx
messages: + msg953
2007-07-18 07:59:13koweysettopic: + Conflicts
nosy: + kowey, beschmi
title: Impossible case in PatchCommute.lhs:1321 -> pull => bug in function reconcile_unwindings (1.0.8)
2007-07-18 08:02:33koweysetsuperseder: + pull => bug in function reconcile_unwindings (** minimal test case **)
2007-08-03 17:58:17koweysetstatus: unknown -> duplicate
messages: + msg1992
2009-08-06 17:36:50adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, dagit, mornfall, simon, thorkilnaur, - droundy, brendan, wilx
2009-08-06 20:33:53adminsetnosy: - beschmi
2009-08-10 21:44:14adminsetnosy: + wilx, brendan, - markstos, darcs-devel, zooko, jast, dagit, Serware, mornfall
2009-08-25 17:50:45adminsetnosy: + darcs-devel, - simon
2009-08-27 13:50:49adminsetnosy: tommy, kowey, brendan, darcs-devel, wilx, thorkilnaur, dmitry.kurochkin