I am continuing to have trouble with Darcs.  Since the darcs community is now active again, I'll try to continue to report my troubles.  I cannot promise that I am reporting distinct problems.

On Windows, using darcs2.

Simon

See the trace below.  I'm using the darcs-all script to pull in patches from a peer directory.  I suspect (but I do not know) that the script isn't doing the Right Thing (see the line in red below).  But regardless, darcs should not crash with "bug in get_extra".   Doing so does not induce confidence.

sh-2.04$  ./darcs-all pull c:/simonpj/darcs/HEAD
== running darcs pull c:/simonpj/darcs/HEAD --repodir .
No remote changes to pull in!
== Required repo utils/hsc2hs is missing! Skipping
== running darcs pull c:/simonpj/darcs/HEAD --repodir libraries/array
Reading inventory of repository c:/simonpj/darcs/HEAD
darcs2.exe: bug in get_extra commuting patch:
Fri Apr 22 18:00:49 GMT Daylight Time 2005  sof
  * [project @ 2005-04-22 17:00:49 by sof]
  [mingw only]
  Better handling of I/O request abortions upon throwing an exception
  to a Haskell thread. As was, a thread blocked on an I/O request was
  simply unblocked, but its corresponding worker thread wasn't notified
  that the request had been abandoned.

  This manifested itself in GHCi upon Ctrl-C being hit at the prompt -- the
  worker thread blocked waiting for input on stdin prior to Ctrl-C would
  stick around even though its corresponding Haskell thread had been
  thrown an Interrupted exception. The upshot was that the worker would
  consume the next character typed in after Ctrl-C, but then just dropping
  it. Dealing with this turned out to be even more interesting due to
  Win32 aborting any console reads when Ctrl-C/Break events are delivered.

  The story could be improved upon (at the cost of portability) by making
  the Scheduler able to abort worker thread system calls; as is, requests
  are cooperatively abandoned. Maybe later.

  Also included are other minor tidyups to Ctrl-C handling under mingw.

  Merge to STABLE.
darcs failed: 256 at ./darcs-all line 49, <IN> line 6.
sh-2.04$

   Try it without the script

sh-2.04$ darcs2 pull c:/simonpj/darcs//HEAD --repodir libraries/array
Pulling from "c:/simonpj/darcs/HEAD"...
darcs2.exe: bug in get_extra commuting patch:
Fri Apr 22 18:00:49 GMT Daylight Time 2005  sof
  * [project @ 2005-04-22 17:00:49 by sof]
  [mingw only]
  Better handling of I/O request abortions upon throwing an exception
  to a Haskell thread. As was, a thread blocked on an I/O request was
  simply unblocked, but its corresponding worker thread wasn't notified
  that the request had been abandoned.

  This manifested itself in GHCi upon Ctrl-C being hit at the prompt -- the
  worker thread blocked waiting for input on stdin prior to Ctrl-C would
  stick around even though its corresponding Haskell thread had been
  thrown an Interrupted exception. The upshot was that the worker would
  consume the next character typed in after Ctrl-C, but then just dropping
  it. Dealing with this turned out to be even more interesting due to
  Win32 aborting any console reads when Ctrl-C/Break events are delivered.

  The story could be improved upon (at the cost of portability) by making
  the Scheduler able to abort worker thread system calls; as is, requests
  are cooperatively abandoned. Maybe later.

  Also included are other minor tidyups to Ctrl-C handling under mingw.

  Merge to STABLE.
sh-2.04$

Attachments

• unnamed

Thanks, Simon.  Could we have your darcs --version, please?

darcs folk, I need a volunteer to
 1. reproduce the crash with 2.0.2, preferably without using the darcs-all script
 2. reproduce it with darcs 1.0.9 to make sure this is not a regression
 3. reproduce it with the latest darcs from trunk to see if it has been fixed.
The repository in question is libraries/array

Marking urgent because of crash, and adding darcs-devel because of
need-volunteer request.

darcs2 --version
2.0.1rc2 (2.0.1rc2 (+ -1 patch))
sh-2.04$

| -----Original Message-----
| From: Eric Kow [mailto:bugs@darcs.net]
| Sent: 22 August 2008 13:11
| To: dagit@codersbase.com; darcs-devel@darcs.net; eric.kow@gmail.com;
| serware@msn.com; Simon Peyton-Jones
| Subject: [issue1026] pull => bug in get_extra commuting patch (2.x)
|
|
| Eric Kow <eric.kow@gmail.com> added the comment:
|
| Thanks, Simon.  Could we have your darcs --version, please?
|
| darcs folk, I need a volunteer to
|  1. reproduce the crash with 2.0.2, preferably without using the darcs-all
| script
|  2. reproduce it with darcs 1.0.9 to make sure this is not a regression
|  3. reproduce it with the latest darcs from trunk to see if it has been
| fixed.
| The repository in question is libraries/array
|
| Marking urgent because of crash, and adding darcs-devel because of
| need-volunteer request.
|
| ----------
| nosy: +Serware, darcs-devel
| priority:  -> urgent
| status: unread -> need-volunteer
| title: Darcs trouble -> pull => bug in get_extra commuting patch (2.x)
| topic: +Darcs2
|
| __________________________________
| Darcs bug tracker <bugs@darcs.net>
| <http://bugs.darcs.net/issue1026>
| __________________________________

On Fri, Aug 22, 2008 at 4:59 AM, Simon Peyton-Jones <bugs@darcs.net> wrote:
>
> New submission from Simon Peyton-Jones <simonpj@microsoft.com>:
>
> I am continuing to have trouble with Darcs.  Since the darcs community is now active again, I'll try to continue to report my troubles.  I cannot promise that I am reporting distinct problems.
>
> On Windows, using darcs2.
>
> Simon
>
> See the trace below.  I'm using the darcs-all script to pull in patches from a peer directory.  I suspect (but I do not know) that the script isn't doing the Right Thing (see the line in red below).  But regardless, darcs should not crash with "bug in get_extra".   Doing so does not induce confidence.

For some reason the red has been stripped out, but in this case it's
still quite clear you're having a problem.

> sh-2.04$ darcs2 pull c:/simonpj/darcs//HEAD --repodir libraries/array
> Pulling from "c:/simonpj/darcs/HEAD"...
> darcs2.exe: bug in get_extra commuting patch:

Eric,

Weren't we discussing last week the circumstances under which a user
could get a bug in get_extra?  What did we decide at that time?  This
is what I recall (pasting realvant parts of the code):

-- | Returns a sub-sequence from @patches@, where all the elements of
@common@ have
-- been removed by commuting them out.
get_extra :: RepoPatch p => [PatchInfo] -- ^ @common@
          -> RL (PatchInfoAnd p) C(u x) -- ^ @patches@
          -> Either MissingPatch (FlippedSeal (RL (PatchInfoAnd p)) C(y))

              case commuteFL (p :> skpd) of
                Just (skipped_patch' :> p') -> do
                    FlippedSeal x <- get_extra_aux (return
skipped_patch') common pps
                    return $ flipSeal (info hp `piap` p' :<: x)
                -- Failure to commute indicates a bug because it means
                -- that a patch was interspersed between the common
                -- patches.  This should only happen if that patch was
                -- commuted there.  This uses 2 properties:
                -- 1) commute is its own inverse
                -- 2) if patches commute in one adjacent context then
                --    they commute in any context where they are
                --    adjacent
                Nothing -> errorDoc $ text "bug in get_extra commuting patch:"
                           $$ human_friendly (info hp)

Triggering this bug basically means, your repositories merged together
at some point in the past, but now the patches fail to commute like
they used to.  This code be a bug in commutation, but I think it's
more likely to be due to index or patch corruption in one or both
copies of the repository.

Thinking about the above, I guess another way this could happen is if
the user amend records a patch and then later tries to merge their
repo with one containing the original patch?

I think we should try printing the patch in skpd that didn't commute
with p.  This could be done by writing a function (untested, but
should be right; and more elegant combining of the Maybe and Either
monads is welcome):
commuteFL2 :: Commute p => (p :> FL p) C(x y) -> Either (Sealed2 p)
((FL p :> p) C(x y))
commuteFL2 (p :> NilFL) = return (NilFL :> p)
commuteFL2 (q :> p :>: ps) = case commute (q :> p) of
                             Just (p' :> q') ->
                               case commuteFL2 (q' :> ps) of
                               Right (ps' :> p'') -> return (p' :>: ps' :> q'')
                               p'' -> p''
                             Nothing -> fail $ seal2 p

Then we could see not only the patch that should have commuted, but
the patch it failed to commute with.  If this is an amend-record
problem it might make it really obvious that it is.  Otherwise, it may
just help with debugging.

Yet another debugging trick to apply is to figure out if this is
happening in the first or second call to get_extra in the function
gcau_simple.  It's especially interesting if it happens in one and not
the other.  I think this could be implemented via a catch in
gcau_simple, but if not we can rewrite get_extra to return the error
message too.  Then we try to compare the patches in question to see if
we can tell why the commute failed.

Another possibility is that we're wrong about the assumptions above
some how, but that is trickery to understand.

We may need to fold David into the discussion on this one.

Jason

Jason, indeed we were just asking ourselves that question, why do we consider it
a bug for an 'extra' patch not to commute past the list of skipped ones.

I agree that we should augment our debugging to say more about commute failures.
  Could something like commuteFL2 just become the new commuteFL?  Could we just
upgrade the commute functions to always return Either?  Would it maybe be
appropriate to upgrade to a MonadError (p,p) m => m 
(modulo witnesses)?

Simon:

Could you a tarball of your libraries/array on darcs.haskell.org?  I'm hoping
that if I download it and do a darcs pull from it, it will produce the get_extra
crash

What Simon PJ seems to be doing here (or rather: What the darcs-all script is 
doing on his behalf) is to do a darcs pull from the GHC HEAD repository into the 
libraries/array repository. I tried that on an Intel Mac OS X 10.5 and got this 
reaction:

> thorkil-naurs-intel-mac-mini:repo thorkilnaur$ darcs --exact-version
> darcs compiled on Aug  7 2008, at 21:50:40
> # configured Mon Jun 23 18:19:52 PDT 2008
> ./configure /usr/local/share/config.site /usr/local/etc/config.site
> 
> Context:
> 
> [TAG 2.0.2
> David Roundy <droundy@darcs.net>**20080624012041] 
> 
> thorkil-naurs-intel-mac-mini:repo thorkilnaur$ cp -R 
/Users/thorkilnaur/tn/GHCDarcsRepository/ghc-HEAD-complete-for-pulling-and-
copying-20070713_1212/ghc/libraries/array array
> thorkil-naurs-intel-mac-mini:repo thorkilnaur$ cd array/
> thorkil-naurs-intel-mac-mini:array thorkilnaur$ darcs pull 
/Users/thorkilnaur/tn/GHCDarcsRepository/ghc-HEAD-complete-for-pulling-and-
copying-20070713_1212/ghc --repodir .
> This is the GHC darcs repository (HEAD branch)
> 
> For more information, visit the GHC developer wiki at
>   http://hackage.haskell.org/trac/ghc
> **********************
> darcs: bug in get_extra commuting patch:
> Fri Apr 22 19:00:49 CEST 2005  sof
>   * [project @ 2005-04-22 17:00:49 by sof]
>   [mingw only]
>   Better handling of I/O request abortions upon throwing an exception
>   to a Haskell thread. As was, a thread blocked on an I/O request was
>   simply unblocked, but its corresponding worker thread wasn't notified
>   that the request had been abandoned.
>   
>   This manifested itself in GHCi upon Ctrl-C being hit at the prompt -- the
>   worker thread blocked waiting for input on stdin prior to Ctrl-C would
>   stick around even though its corresponding Haskell thread had been
>   thrown an Interrupted exception. The upshot was that the worker would
>   consume the next character typed in after Ctrl-C, but then just dropping
>   it. Dealing with this turned out to be even more interesting due to
>   Win32 aborting any console reads when Ctrl-C/Break events are delivered.
>   
>   The story could be improved upon (at the cost of portability) by making
>   the Scheduler able to abort worker thread system calls; as is, requests
>   are cooperatively abandoned. Maybe later.
>   
>   Also included are other minor tidyups to Ctrl-C handling under mingw.
>   
>   Merge to STABLE.
> thorkil-naurs-intel-mac-mini:array thorkilnaur$ 

A similar thing happened on a PPC Mac OS X with:

> Thorkil-Naurs-Computer:~/tn/test/darcs/Issue/1026/work/repo/array thorkilnaur$ 
darcs --exact-version
> darcs compiled on May  8 2007, at 09:50:57
> # 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] 

Finally, I tried the latest (I presume) darcs on (another) PPC Mac OS X that 
hosts a darcs buildbot slave:

> thorkil-naurs-mac-mini:array thorkilnaur$ 
'/Users/thorkilnaur/tn/buildbot/darcs/thorkil_mac/thorkil tn20/build/darcs' --
version
> 2.0.2 (+ 119 patches)

Same reaction. So I would conclude: This reaction has been present for a while 
and it is not gone.

Hi,

Thanks for helping us to reproduce this!

Thorkil, could you make a copy of just the libraries array repository you are
pulling with?  My goal is to have two tarballs, one to pull with, and one to
pull from (i.e. the libraries/array HEAD) without dealing with darcs-all or any
intervening GHC stuff.

The reason I'm asking for tarballs is that I'm still not sure I understand where
the local copy of libraries/array is coming from.  I realise that the darcs-all
script can be used to recursively get subrepositories, but I don't know how the
libraries/array subrepository got into this state (and a reproducible one at that)

Many thanks!

Speaking with Thorkil on IRC, I now understand the situation better.

Ok, so there is no darcs-all fanciness in this whatsoever... and the pull really
just a crazy one of GHC from libraries/array, i.e. two completely unrelated
repositories.

Sorry for the noise, I just kept assuming that there was darcs-all magic
underneath, and that what everybody was talking about was a libraries/array to
libraries/array pull.

Ok, so now we just need to boil this down into a test case!  Thanks!

On Sat, Aug 23, 2008 at 12:03 AM, Eric Kow <bugs@darcs.net> wrote:
>
> Eric Kow <eric.kow@gmail.com> added the comment:
>
> Jason, indeed we were just asking ourselves that question, why do we consider it
> a bug for an 'extra' patch not to commute past the list of skipped ones.
>
> I agree that we should augment our debugging to say more about commute failures.
>  Could something like commuteFL2 just become the new commuteFL?  Could we just
> upgrade the commute functions to always return Either?  Would it maybe be
> appropriate to upgrade to a MonadError (p,p) m => m
> (modulo witnesses)?

Standard commute probably doesn't need upgraded.  When you call
commute, you have all the patches available and you could then deal
with it (by throwing them up the chain as needed).  But, yes this
should become the new commuteFL.  I can do that.  I can also look at
making gcau_simple tell us which call to get_extra is failing or if
both are.

Jason

Considering a failed commute in get_extra a bug is based on the assumption
that commutation of named patches is consistent over time.
This assumption can be broken by: amended patches, duplicate patches
(see Ian's example / issue 1014), repository corruption or a bug in darcs.

I think get_extra should be changed to not bug out on this and try to commute
out as many as possible common patches instead of all. Hopefully the case of
corruption or bugs will be caught elsewhere. I'm not familiar enough with
darcs internals to see if this change has any impact on higher level code.

-- Arjan

I can't reproduce Simon's bug.  I have tried fetching a fresh copy from:
http://hackage.haskell.org/trac/ghc

As well as starting from this tarball:
http://darcs.haskell.org/ghc-STABLE-2007-09-11-ghc-corelibs-testsuite.tar.bz2

The attached patch can be used to improve the error reporting of darcs.  I was
hoping to try it out and see which two patches fail to commute.

Can someone that can reproduce this try for me?  Thorkil could you look at it
please?

Thanks!

Attachments

• improve-reporting-for-bug-in-get_extra.dpatch

I will try, within the next couple of days.

Best regards
Thorkil

| Could you a tarball of your libraries/array on darcs.haskell.org?  I'm hoping
| that if I download it and do a darcs pull from it, it will produce the get_extra
| crash

I think Thorkil has it.  I think he's right that the script is pulling from the wrong repo entirely; and that the problem is really that darcs should complain politely rather than run off and pull patches, and then crash.

So I won't tar up the repo for now unless you need me to.  (I'm not even sure I have it.)

Simon

| -----Original Message-----
| From: Eric Kow [mailto:bugs@darcs.net]
| Sent: 23 August 2008 17:57
| To: dagit@codersbase.com; darcs-devel@darcs.net; eric.kow@gmail.com; naur@post11.tele.dk;
| serware@msn.com; serware@msn.com; Simon Peyton-Jones
| Subject: [issue1026] pull => bug in get_extra commuting patch (2.x)
|
|
| Eric Kow <eric.kow@gmail.com> added the comment:
|
| Hi,
|
| Thanks for helping us to reproduce this!
|
| Thorkil, could you make a copy of just the libraries array repository you are
| pulling with?  My goal is to have two tarballs, one to pull with, and one to
| pull from (i.e. the libraries/array HEAD) without dealing with darcs-all or any
| intervening GHC stuff.
|
| The reason I'm asking for tarballs is that I'm still not sure I understand where
| the local copy of libraries/array is coming from.  I realise that the darcs-all
| script can be used to recursively get subrepositories, but I don't know how the
| libraries/array subrepository got into this state (and a reproducible one at that)
|
| Many thanks!
|
| ----------
| assignedto:  -> thorkilnaur
| status: need-volunteer -> need-info
|
| __________________________________
| Darcs bug tracker <bugs@darcs.net>
| <http://bugs.darcs.net/issue1026>
| __________________________________

On Monday 25 August 2008 03:22, Jason Dagit wrote:
> ...
> The attached patch can be used to improve the error reporting of darcs.  I was
> hoping to try it out and see which two patches fail to commute.
>
> Can someone that can reproduce this try for me?

With your patch applied, the reaction extends to:

> Thorkil-Naurs-Computer:~/tn/test/darcs/Issue/1026/work/repo/array thorkilnaur$ 
/Users/thorkilnaur/tn/darcsDarcsRepository/darcs.net/darcs pull /Users/thorkilnaur/tn/GHCDarcsRepository/ghc-HEAD-complete-for-pulling-
and-copying-20070713_1212/ghc --repodir .
> Pulling from "/Users/thorkilnaur/tn/GHCDarcsRepository/ghc-HEAD-complete-for-pulling-and-copying-20070713_1212/ghc"...
> This is the GHC darcs repository (HEAD branch)
> 
> For more information, visit the GHC developer wiki at
>   http://hackage.haskell.org/trac/ghc
> **********************
> darcs: bug in get_extra commuting patches:                                      
> First patch is:
> Fri Apr 22 19:00:49 CEST 2005  sof
>   * [project @ 2005-04-22 17:00:49 by sof]
>   [mingw only]
>   Better handling of I/O request abortions upon throwing an exception
>   to a Haskell thread. As was, a thread blocked on an I/O request was
>   simply unblocked, but its corresponding worker thread wasn't notified
>   that the request had been abandoned.
>   
>   This manifested itself in GHCi upon Ctrl-C being hit at the prompt -- the
>   worker thread blocked waiting for input on stdin prior to Ctrl-C would
>   stick around even though its corresponding Haskell thread had been
>   thrown an Interrupted exception. The upshot was that the worker would
>   consume the next character typed in after Ctrl-C, but then just dropping
>   it. Dealing with this turned out to be even more interesting due to
>   Win32 aborting any console reads when Ctrl-C/Break events are delivered.
>   
>   The story could be improved upon (at the cost of portability) by making
>   the Scheduler able to abort worker thread system calls; as is, requests
>   are cooperatively abandoned. Maybe later.
>   
>   Also included are other minor tidyups to Ctrl-C handling under mingw.
>   
>   Merge to STABLE.
> Second patch is:
> Fri May  6 02:30:57 CEST 2005  sof
>   * [project @ 2005-05-06 00:30:56 by sof]
>   [mingw only]
>   Work around bug in win32 Console API which showed up in the GHCi UI:
>   if the user typed in characters prior to the appearance of the prompt,
>   the first of these characters always came out as a 'g'. The GHCi UI does
>   for good reasons one-character reads from 'stdin', which causes the
>   underlying APIs to become confused. A simple repro case is the following
>   piece of C code:
>   
>   /*----------------------*/
>   #include <stdio.h>
>   #include <windows.h>
>   int main()
>   {
>       char ch1,ch2;
>       HANDLE hStdIn = GetStdHandle(STD_INPUT_HANDLE);
>       DWORD dw;
>   
>       /* Type in some characters before the prompt appears and be amused.. */
>       sleep(1000); printf("? ");
>       ReadConsoleA(hStdIn,&ch1,1,&dw,NULL);
>       ReadConsoleA(hStdIn,&ch2,1,&dw,NULL);
>   /*  or, if you want to use libc:
>       read(0,&ch1,1); read(0,&ch2,1); */
>   
>       printf("%c%c\n", ch1,ch2);
>       return 0;
>   }
>   /*----------------------*/
>   
>   This happens across win32 OSes, and I can't see anything untoward as far
>   as API usage goes (the GHC IO implementation uses read(), but that
>   reduces to ReadConsoleA() calls.) People inside the Behemoth might want
>   to have a closer look at this..
>   
>   Not much we can do about this except work around the problem by flushing
>   the input buffer prior to reading from stdin. Not ideal, as type-ahead
>   is a useful feature. Flushing is handled by GHC.ConsoleHandler.flushConsole
>   
>   Merge to STABLE.
> Thorkil-Naurs-Computer:~/tn/test/darcs/Issue/1026/work/repo/array thorkilnaur$ 

Best regards
Thorkil

User view again: the new error still looks like debug output from a darcs crash.

But actually the error is: "user error: you are pulling from the wrong repository".

In the darcs model, a repository is just a collection of patches, and doesn't have an "identity" per se.  But somehow it really is wrong to pull patches from a repository of GHC into a repository for an unrelated library.  That is a very plausible user error and darcs should, I believe, report it in a civilised way.

This might in turn require thinking about what it means to for two repos to be "the same".  But that is useful thinking to do.

Simon

| -----Original Message-----
| From: Thorkil Naur [mailto:bugs@darcs.net]
| Sent: 27 August 2008 00:51
| To: arjan.boeijink@gmail.com; dagit@codersbase.com; darcs-devel@darcs.net; eric.kow@gmail.com;
| naur@post11.tele.dk; serware@msn.com; serware@msn.com; Simon Peyton-Jones
| Subject: [issue1026] pull => bug in get_extra commuting patch (2.x)
|
|
| Thorkil Naur <naur@post11.tele.dk> added the comment:
|
| On Monday 25 August 2008 03:22, Jason Dagit wrote:
| > ...
| > The attached patch can be used to improve the error reporting of darcs.  I was
| > hoping to try it out and see which two patches fail to commute.
| >
| > Can someone that can reproduce this try for me?
|
| With your patch applied, the reaction extends to:
|
| > Thorkil-Naurs-Computer:~/tn/test/darcs/Issue/1026/work/repo/array thorkilnaur$
| /Users/thorkilnaur/tn/darcsDarcsRepository/darcs.net/darcs pull
| /Users/thorkilnaur/tn/GHCDarcsRepository/ghc-HEAD-complete-for-pulling-
| and-copying-20070713_1212/ghc --repodir .
| > Pulling from "/Users/thorkilnaur/tn/GHCDarcsRepository/ghc-HEAD-complete-for-pulling-and-copying-
| 20070713_1212/ghc"...
| > This is the GHC darcs repository (HEAD branch)
| >
| > For more information, visit the GHC developer wiki at
| >   http://hackage.haskell.org/trac/ghc
| > **********************
| > darcs: bug in get_extra commuting patches:
| > First patch is:
| > Fri Apr 22 19:00:49 CEST 2005  sof
| >   * [project @ 2005-04-22 17:00:49 by sof]
| >   [mingw only]
| >   Better handling of I/O request abortions upon throwing an exception
| >   to a Haskell thread. As was, a thread blocked on an I/O request was
| >   simply unblocked, but its corresponding worker thread wasn't notified
| >   that the request had been abandoned.
| >
| >   This manifested itself in GHCi upon Ctrl-C being hit at the prompt -- the
| >   worker thread blocked waiting for input on stdin prior to Ctrl-C would
| >   stick around even though its corresponding Haskell thread had been
| >   thrown an Interrupted exception. The upshot was that the worker would
| >   consume the next character typed in after Ctrl-C, but then just dropping
| >   it. Dealing with this turned out to be even more interesting due to
| >   Win32 aborting any console reads when Ctrl-C/Break events are delivered.
| >
| >   The story could be improved upon (at the cost of portability) by making
| >   the Scheduler able to abort worker thread system calls; as is, requests
| >   are cooperatively abandoned. Maybe later.
| >
| >   Also included are other minor tidyups to Ctrl-C handling under mingw.
| >
| >   Merge to STABLE.
| > Second patch is:
| > Fri May  6 02:30:57 CEST 2005  sof
| >   * [project @ 2005-05-06 00:30:56 by sof]
| >   [mingw only]
| >   Work around bug in win32 Console API which showed up in the GHCi UI:
| >   if the user typed in characters prior to the appearance of the prompt,
| >   the first of these characters always came out as a 'g'. The GHCi UI does
| >   for good reasons one-character reads from 'stdin', which causes the
| >   underlying APIs to become confused. A simple repro case is the following
| >   piece of C code:
| >
| >   /*----------------------*/
| >   #include <stdio.h>
| >   #include <windows.h>
| >   int main()
| >   {
| >       char ch1,ch2;
| >       HANDLE hStdIn = GetStdHandle(STD_INPUT_HANDLE);
| >       DWORD dw;
| >
| >       /* Type in some characters before the prompt appears and be amused.. */
| >       sleep(1000); printf("? ");
| >       ReadConsoleA(hStdIn,&ch1,1,&dw,NULL);
| >       ReadConsoleA(hStdIn,&ch2,1,&dw,NULL);
| >   /*  or, if you want to use libc:
| >       read(0,&ch1,1); read(0,&ch2,1); */
| >
| >       printf("%c%c\n", ch1,ch2);
| >       return 0;
| >   }
| >   /*----------------------*/
| >
| >   This happens across win32 OSes, and I can't see anything untoward as far
| >   as API usage goes (the GHC IO implementation uses read(), but that
| >   reduces to ReadConsoleA() calls.) People inside the Behemoth might want
| >   to have a closer look at this..
| >
| >   Not much we can do about this except work around the problem by flushing
| >   the input buffer prior to reading from stdin. Not ideal, as type-ahead
| >   is a useful feature. Flushing is handled by GHC.ConsoleHandler.flushConsole
| >
| >   Merge to STABLE.
| > Thorkil-Naurs-Computer:~/tn/test/darcs/Issue/1026/work/repo/array thorkilnaur$
|
| Best regards
| Thorkil
|
| __________________________________
| Darcs bug tracker <bugs@darcs.net>
| <http://bugs.darcs.net/issue1026>
| __________________________________

On Wed, Aug 27, 2008 at 12:30 AM, Simon Peyton-Jones
<simonpj@microsoft.com> wrote:
> User view again: the new error still looks like debug output from a darcs crash.

Well, that's exactly what that output is for.  I want to understand
the problem, and I can't reproduce it (see below).

>
> But actually the error is: "user error: you are pulling from the wrong repository".

Having fun with the error message:
Perhaps darcs should say, "Sorry my brain just exploded.  Patch does
not commute consistently."  Or maybe,  "The inferred changes are less
commutable than expected."  GHC seems to do fine with such type check
error messages :)

>
> In the darcs model, a repository is just a collection of patches, and doesn't have an "identity" per se.  But somehow it really is wrong to pull patches from a repository of GHC into a repository for an unrelated library.  That is a very plausible user error and darcs should, I believe, report it in a civilised way.

Well, darcs should be reporting it in a civilized way.  That's what
we're trying to work towards.  Making get_extra more transparent so we
can see why it's breaking is a direction we haven't tried yet, hence
my patch.  I was hoping to understand why those patches failed to
commute.  Unfortunately, I still don't know why.  If I could reproduce
the failure locally that might not be the case.  So far no one has
provided me with the files to do that, even though I've asked twice
now. Hint, hint.

> This might in turn require thinking about what it means to for two repos to be "the same".  But that is useful thinking to do.

Well, all repositories start at the same context, the empty
repository.  So from a theoretical standpoint, all repositories are
branches from a common root.  Pretty much anything after that should
just depend on the way patches commute.  But, what we're seeing here
is odd.  As Arjan pointed out, this bug that darcs is reporting is the
result of a fundamental assumption being violated.

Even "unrelated" repositories should be able to agree about the empty
repository and from that point figure out which patches they have in
common (which is the operation that's failing for you).  This
shouldn't lead to the bug you're seeing unless something is going
wrong.  I was looking at the way amend-record works and it seems to
generate a new PatchInfo, so I don't think this bug can be caused by
amend-record.  Can someone construct a counter example?

If our goal was just to make some repositories the same and some
different, probably the easiest thing to do is to make the empty
repository not be a globally unique context.  This would change quite
a few semantics of darcs, but I agree it would have some nice side
effects.  Such as saying, "Hey, array != GHC, stop trying to pull
here!"

Thanks for the feedback and bug reports,
Jason

| commute.  Unfortunately, I still don't know why.  If I could reproduce
| the failure locally that might not be the case.  So far no one has
| provided me with the files to do that, even though I've asked twice
| now. Hint, hint.

Are you directing that at me?  I did reply about this: Thorkil managed to reproduce it this bug I believe http://bugs.darcs.net/msg5668, and described how to do so.  Try this recipe

* Get a GHC repo (I'd start with a tarball from http://darcs.haskell.org)
* Get a packages/array repo
* Pull from the former into the latter

Simon

> Try this recipe
> * Get a GHC repo (I'd start with a tarball from http://darcs.haskell.org)
> * Get a packages/array repo
> * Pull from the former into the latter

For example:

> $ tar xjf /Users/thorkilnaur/tn/download/GHC/darcs.haskell.org/ghc-HEAD-2008-08-17-ghc.tar.bz2
> $ darcs get http://darcs.haskell.org/packages/array
> Finished getting.
> $ ls
> array   ghc
> $ cd array/
> $ darcs pull ../ghc
> darcs: bug in get_extra commuting patch:
> Fri Apr 22 19:00:49 CEST 2005  sof
>   * [project @ 2005-04-22 17:00:49 by sof]
>   [mingw only]
>   Better handling of I/O request abortions upon throwing an exception
>   to a Haskell thread. As was, a thread blocked on an I/O request was
>   simply unblocked, but its corresponding worker thread wasn't notified
>   that the request had been abandoned.
>
>   This manifested itself in GHCi upon Ctrl-C being hit at the prompt -- the
>   worker thread blocked waiting for input on stdin prior to Ctrl-C would
>   stick around even though its corresponding Haskell thread had been
>   thrown an Interrupted exception. The upshot was that the worker would
>   consume the next character typed in after Ctrl-C, but then just dropping
>   it. Dealing with this turned out to be even more interesting due to
>   Win32 aborting any console reads when Ctrl-C/Break events are delivered.
>
>   The story could be improved upon (at the cost of portability) by making
>   the Scheduler able to abort worker thread system calls; as is, requests
>   are cooperatively abandoned. Maybe later.
>
>   Also included are other minor tidyups to Ctrl-C handling under mingw.
>
>   Merge to STABLE.
> $

Best regards
Thorkil

Thanks Thorkil.  I have been able to reproduce the problem locally.

It turns out to be rather interesting!

Both repositories, array and GHC, have both the patchinfos that are shown in the
new error message.  Unfortunately, the patch contents vary depending on which
repository you view those patches from.

Darcs is rightly confused here.  I mentioned this to Ian and Eric, and they
seemed to think these patches were probably created by a tailorisation of the
old GHC cvs repository into the two darcs repositories array and GHC.

Darcs has a pretty pervasive assumption that when patchinfos are equal that the
patches represent the same changes even though the patches might be in different
contexts.  

Arjan suggested that we either take it in stride or throw a catchable error up
the chain.  The former option seems dangerous to me, and the latter approach
might allow us to suggest that the user has tried to merge two very different
repositories.

Eric mentioned that there is a issue already about making patchinfos more unique
than they currently are.  I just glanced but I don't know which issue that is. 
If you find it please connect it to this issue.

That would be http://bugs.darcs.net/issue27

So it sounds like there is not much more we can do for this ticket, except to
give the user better advice when get_extra fails, explaining roughly why darcs
is confused -- it thinks that two patches are the "same" when they really are
not -- and what the user can do about it (not much? maybe this is a mistaken
pull anyway?)

Otherwise, it sounds like we should work on issue27 to prevent future
occurrences of this confusion.

On Thu, Aug 28, 2008 at 1:12 AM, Eric Kow <bugs@darcs.net> wrote:
>
> Eric Kow <eric.kow@gmail.com> added the comment:
>
> That would be http://bugs.darcs.net/issue27
>
> So it sounds like there is not much more we can do for this ticket, except to
> give the user better advice when get_extra fails, explaining roughly why darcs
> is confused -- it thinks that two patches are the "same" when they really are
> not -- and what the user can do about it (not much? maybe this is a mistaken
> pull anyway?)
>
> Otherwise, it sounds like we should work on issue27 to prevent future
> occurrences of this confusion.

The only other thing I can think of to help this bug is to add a
feature where you put a "unique" token somewhere in _darcs/*, perhaps
in _darcs/format, that is assigned during 'init'.  If you try to merge
two repositories that vary by token, then darcs says, "These
repositories seem to be unrelated, proceed? [y/N]"

I don't know how others feel about this feature, but it seems like
what SimonPJ was requesting.  It also doesn't sound like it would be
too hard to add.

Jason

Ok, I have split ancestry-checking off into http://bugs.darcs.net/issue1039

I am also downgrading this to a feature and marking the improved advice (along
with Jason's information commuteFL?) as critical for 2.0.3.

I also consider this to be ProbablyEasy in that the hard part is trying to
phrase our error message so that it is informative without being overwhelming.

...
|
| The only other thing I can think of to help this bug is to add a
| feature where you put a "unique" token somewhere in _darcs/*, perhaps
| in _darcs/format, that is assigned during 'init'.  If you try to merge
| two repositories that vary by token, then darcs says, "These
| repositories seem to be unrelated, proceed? [y/N]"
|
| I don't know how others feel about this feature, but it seems like
| what SimonPJ was requesting.  It also doesn't sound like it would be
| too hard to add.

Something like that, yes.  The point is this: if darcs falls over saying "bug in get_extra" then my reaction is "Darcs is unreliable and I should stop using it".  If it says politely "You are trying to merge patches from one repository into an utterly different one; you've probably got the wrong prefs/defaultrepo" or something like that, then I think "Oh I'm being an idiot, how great Darcs is".

You see the difference?  How it's achieved is a different matter.

Incidentally, even if Darcs *doesn't* crash, I *still don't* want to accidentally merge 10,000 patches from ghc's repo into libraries/array.  Yes, it might in principle be do-able, but I really want Darcs to say "this looks silly to me, are you really sure you want to do this?".

Simon

Hello!

I think I have reproduced this problem with two small repos attached. Run pull
from repo2 to repo1 and you get the bug.

Repo1 contains patch aaa and bbb. Repo2 contains the same patch aaa, then ccc,
and bbb with different content from repo1 bbb but the same id.

To make patch with the same id I edited inventory and patch by hand. Is there a
way to make dacas use a given date for patch? Then we can put this into a test.

Regards,
  Dmitry

Attachments

• repo1.tar.bz2

This worked for me; you may need to adjust the timezone:

(echo 'Mon Sep 18 10:37:06 GMT 2008'; echo 'Dmitry Kurochkin
<dmitry.kurochkin@gmail.com>') | darcs record --pipe -a -m 'aaa'

So, speaking with Jason Dagit on IRC, we have determined that (a) this is a
corner case (unfortunately, one that bit a pretty big repository) and (b) it
will be virtually eliminated because issue27 (the underlying cause) is resolved.
 There isn't much more we can do to improve the advice anyway.  So I'm marking
this wont-fix.

Note that the wont-fix refers to my request to improve the advice given by
darcs.  Simon's suggestion, that darcs attempt to warn you if you are pulling
between unrelated repositories has been implemented in
http://bugs.darcs.net/issue1039 and will be released in darcs 2.1

Thanks, everyone!