darcs

Issue 1095 send => fromJust errorsrc/Darcs/Commands/Send.lhs:196 (2.0.3pre2)

Title send => fromJust errorsrc/Darcs/Commands/Send.lhs:196 (2.0.3pre2)
Priority bug Status given-up
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, kowey, thorkilnaur, twb
Assigned To
Topics

Created on 2008-09-23.09:03:05 by twb, last changed 2009-08-27.14:33:18 by admin.

Messages
msg6093 (view) Author: twb Date: 2008-09-23.09:03:03
On Tue, Sep 23, 2008 at 09:41:49AM +0100, Eric Kow wrote:
> > Well, fuck.  I just assumed that darcs sent it OK and deleted the
> > local copy.  Given that darcs had a weird pseudocrash at the GPG step,
> > that was probably not a smart move.  (This was happening repeatedly in
> > .3pre1, so I downgraded to .2.)
> 
> That's very interesting to hear!
> 
> Did you file a bug report?  I don't recall seeing one.
> 
> Hmm... sounds like something that might be tricky to reproduce in an
> automated script

Not yet, it only just came up this afternoon and I was busy with other
stuff.  I'm guessing it relates to having this in ~/.darcs/defaults:

    send sign
    send edit-description

...and/or something to do with gpg-agent.  gpg-agent wanted to ask for
my passphrase; IIRC it wasn't erroring if the passphrase was still
cached in gpg-agent.
msg6127 (view) Author: twb Date: 2008-09-24.02:00:42
# This mail was generated by a wrapper that twb is
# writing to interact with a remote roundup BTS from the
# command line.

@
msg6179 (view) Author: droundy Date: 2008-09-30.16:16:17
Okay, from looking at the code (at the line indicated), it's clear that we're
using the wrong slurpy to create the context, and that's what's causing the problem.

It would be nice to have a test case for this, and shouldn't be *too* hard. 
We'd just need to create a sequence of patches and send just the right
subsequence.  (I'm being vague because I'm uncertain of the details, not because
I'm obtuse.)  It also should be something I/we can fix without a test case just
by staring at the code, but I'd rather have the test case so we can avoid a
regression of this.

Thanks for the bug report, by the way! (and I'm sorry the bug tracker isn't
working right...)

David

On Mon, Sep 29, 2008 at 11:27 PM, Trent W. Buck wrote:
>    $ darcs send -p make.installdocs --to trentbuck@gmail.com http://darcs.net/
>    Tue Sep 30 13:03:15 EST 2008  Trent W. Buck <trentbuck@gmail.com>
>      * make installdocs should not install TeX intermediaries.
>      I'm unilaterally classing the DVI and PostScript versions as
>      "intermediaries" to the PDF version, and only installing the latter.
>    Shall I send this patch? (1/1)  [ynWsfvpxdaqjk], or ? for help: y
>
>    darcs: bug in darcs!
>    fromJust errorsrc/Darcs/Commands/Send.lhs:196 compiled Sep 30 2008 12:39:48
at src/Darcs/Commands/Send.lhs:196 compiled Sep 30 2008 12:39:48
>    This is a prerelease version of darcs.
>    You can check to see if this bug is already known at http://bugs.darcs.net/
>    If it is not, please report this to bugs@darcs.net
>    If possible include the output of 'darcs --exact-version'.
>    $
msg6182 (view) Author: thorkilnaur Date: 2008-10-01.09:09:19
It took me a while to unwind what this issue is, so I'll just make some notes here, to make it easier for me (at least) to figure out 
what goes on at a later time.

For reference, I'll quote the complete mail which is also quoted in part in the above http://bugs.darcs.net/msg6179 
(http://lists.osuosl.org/pipermail/darcs-users/2008-September/014164.html):

> Trent W. Buck trentbuck at gmail.com 
> Tue Sep 30 03:27:19 UTC 2008
> ...
> [CCing list as BTS is apparently having problems.]
> 
> I get this behaviour with both 2.0.3pre1 and 2.1.0pre2 when trying to
> send a patch from a repo that contains both http://darcs.net/ (as at
> --tag 2.1.0pre2) and Debian integration patches from
> http://repos.mornfall.net/darcs/debian.
> 
> I cannot reproduce this fault in 2.0.2; this is a REGRESSION.
> 
>     $ darcs send -p make.installdocs http://darcs.net/
>     Patch bundle will be sent to: droundy at darcs.net, darcs-users at darcs.net
>     Tue Sep 30 13:03:15 EST 2008  Trent W. Buck <trentbuck at gmail.com>
>       * make installdocs should not install TeX intermediaries.
>       I'm unilaterally classing the DVI and PostScript versions as
>       "intermediaries" to the PDF version, and only installing the latter.
>     Shall I send this patch? (1/1)  [ynWsfvpxdaqjk], or ? for help: y
> 
> 
>     You need a passphrase to unlock the secret key for
>     user: "Trent W. Buck <trentbuck at gmail.com>"
>     1024-bit DSA key, ID 24EDC406, created 2006-12-06
> 
>     darcs: bug in darcs!
>     fromJust errorsrc/Darcs/Commands/Send.lhs:196 compiled Sep 30 2008 12:39:48 at src/Darcs/Commands/Send.lhs:196 compiled Sep 30 
2008 12:39:48
>     This is a prerelease version of darcs.
>     You can check to see if this bug is already known at http://bugs.darcs.net/
>     If it is not, please report this to bugs at darcs.net
>     If possible include the output of 'darcs --exact-version'.
>     Reading pristine 59/499
>     Successfully sent patch bundle to: droundy at darcs.net, darcs-users at darcs.net.
>     $
> 
> I can also reproduce this without a ~/.darcs:
> 
>     $ mv ~/.darcs ~/.darcs~
>     $ darcs send -p make.installdocs --to trentbuck at gmail.com http://darcs.net/
>     Tue Sep 30 13:03:15 EST 2008  Trent W. Buck <trentbuck at gmail.com>
>       * make installdocs should not install TeX intermediaries.
>       I'm unilaterally classing the DVI and PostScript versions as
>       "intermediaries" to the PDF version, and only installing the latter.
>     Shall I send this patch? (1/1)  [ynWsfvpxdaqjk], or ? for help: y
> 
>     darcs: bug in darcs!
>     fromJust errorsrc/Darcs/Commands/Send.lhs:196 compiled Sep 30 2008 12:39:48 at src/Darcs/Commands/Send.lhs:196 compiled Sep 30 
2008 12:39:48
>     This is a prerelease version of darcs.
>     You can check to see if this bug is already known at http://bugs.darcs.net/
>     If it is not, please report this to bugs at darcs.net
>     If possible include the output of 'darcs --exact-version'.
>     $

There is some additional discussion related to this problem in the thread that starts at http://lists.osuosl.org/pipermail/darcs-
users/2008-September/013928.html.

As this is designated a regression, I am marking it release-critical for release 2.1. Also, it appears that it would be useful to 
have a test case for this, so is anyone volunteering for producing such a test?

Thanks and best regards
Thorkil
msg6187 (view) Author: thorkilnaur Date: 2008-10-01.16:15:44
Here is a reponse from twb that I enter using the Web interface with his permission (in these days of issue1114):

> ...
>  Date: Wed, 1 Oct 2008 20:24:53 +1000
>  From: "Trent W. Buck" <...>
>  To: Thorkil Naur <bugs@darcs.net>
>  Cc: dagit...,
>  dmitry.kurochkin...,
>  droundy...,
>  kowey...,
>  naur...,
>  simon...
>  Subject: Re: [issue1095] darcs send error w/ gpg and --edit in 2.0.3pre1
>  ...
> On Wed, Oct 01, 2008 at 09:09:27AM -0000, Thorkil Naur wrote:
> > As this is designated a regression, I am marking it release-critical
> > for release 2.1. Also, it appears that it would be useful to have a
> > test case for this, so is anyone volunteering for producing such a
> > test?
> 
> Unfortunately, my first attempt results in a false negative for
> 2.1.0pre2:
> 
>   #!/bin/bash -ev
>   darcs init --repodir x
>   darcs init --repodir y
>   cd x
>   touch x
>   darcs reco -lam x x
>   darcs send -a ../y --to x@x.x --sendmail-command /bin/true
> 
> It even gives a false positive on just the darcs.net repo:
> 
>   $ with-temp-dir bash -exc 'darcs get --lazy http://darcs.net/; cd */; touch x; darcs rec --no-test -lam x x; darcs 
send -a --sendmail-command /bin/true http://darcs.net/'
>   with-temp-dir: entering directory `/tmp/with-temp-dir.WaDCWb'
>   + darcs get --lazy http://darcs.net/
>   Welcome to the darcs stable repository.
>   **********************
>   Finished getting.
>   + cd darcs.net/
>   + touch x
>   + darcs rec --no-test -lam x x
>   Recording changes in "x":
> 
>   Finished recording patch 'x'
>   + darcs send -a --sendmail-command /bin/true http://darcs.net/
>   Patch bundle will be sent to: droundy@darcs.net, darcs-users@darcs.net
>   Successfully sent patch bundle to: droundy@darcs.net, darcs-users@darcs.net.
>   with-temp-dir: leaving directory `/tmp/with-temp-dir.WaDCWb'
> 
> So perhaps there's something strange happening with the mornfall repo
> mentioned in the initial report, or it only happens when you're
> dealing with >1 repo.
msg6199 (view) Author: kowey Date: 2008-10-02.12:13:08
Trent, do you think the boil-down methodology in
http://wiki.darcs.net/index.html/Forensics could help?

It's just a systematic way of turning reproducible test cases into minimal ones.
It's still a bit of an art, but hopefully these guidelines can help us to make
progress.
msg6265 (view) Author: kowey Date: 2008-10-07.15:43:29
Hi David,

This is our last 2.1 blocker, I think!

Unfortunately, we're having a tough time reproducing this one (i.e. producing a
test case by working backwards).  Maybe we can create a test case by working
'forwards' (that is by staring at the code).  Hopefully I can have a chance to
do this in the evening today, but it's not certain.

Also any ideas why this would have happened in a recent darcs, but not in 2.0.2?
msg6267 (view) Author: kowey Date: 2008-10-07.19:25:44
Ok, I think I can at least understand parts David is saying when he says we're
using the wrong slurpy to generate the context.  

First: context here refers to the lines surrounding hunk adds/minuses, as in
diff --unified

Second: to generate this context-diff info, we need a representation of the
common repository tree, i.e. what the repository would look like with only
patches that we have in common with the sendee.  We already have a
representation of our uncommon patches (us'), which get by commuting them out of
our recorded state, past all the common patches (I assume my memory of gcau is
about right).  So all we do is invert these patches and then apply them to the
pristine slurpy, thus generating a common slurpy.

So I'm not too sure what's wrong.  The code here claims that there was a failure
applying these inverted patches to the pristine slurpy, but if we only obtained
these patches through commutation on our own stuff, that should work, right? 
That's why we're using fromJust; it's 'impossible' for this to fail.

Am I missing something?
msg6268 (view) Author: kowey Date: 2008-10-07.19:39:06
Ah-hah! I think *now* I get what David is saying.  The trick is that the patches
to be sent should /not/ be compared against the common slurpy; they should be
compared against the common slurpy PLUS the patches we chose not to send.
msg6269 (view) Author: kowey Date: 2008-10-07.19:49:27
On Tue, Oct 07, 2008 at 19:39:07 -0000, Eric Kow wrote:
> Ah-hah! I think *now* I get what David is saying.  The trick is that the patches
> to be sent should /not/ be compared against the common slurpy; they should be
> compared against the common slurpy PLUS the patches we chose not to send.

No, sorry that's not right either.

First of all, the error at hand isn't even related to the patches we're
sending; it's just the inverse of our uncommon patches (all of them)
that we're trying to apply, so what we choose to send or not doesn't
enter into this question.

Second of all, the patches to be sent are commuted to the front of the
list anyway, so they should just apply cleanly to the common slurpy (not
that this is relevant any more).
msg6270 (view) Author: droundy Date: 2008-10-07.22:37:41
I've taken another look at the code, and I still don't see this bug in Sent.  A
test case would be *really* helpful here, even if it's just a large tarballed
repository with which one can trigger this.

David
msg6271 (view) Author: droundy Date: 2008-10-07.22:40:01
I should also mention that it's possible for this bug message to be triggered by
a corrupt repository, so if anyone can reproduce this, it's a good idea to run
darcs check --no-test on a clean copy of that repository.  I don't know what
could cause the repository corruption, but this little bit of code in Send
really does look innocent.

David
msg6273 (view) Author: kowey Date: 2008-10-08.08:25:07
Ok, so we've had two people independently study the code, who both believe there
is no bug in sent (at worst, we have poor handling of repository corruption),
and we have had success reproducing the original bug in any version of darcs
(this is from an IRC conversation with Trent).

Any objections if I mark this not-our-bug?  I'm setting this back to need-info
because we don't have a clear idea what to do next, but we know we need some
kind of test case.  Over to you, Thorkil.
msg6278 (view) Author: droundy Date: 2008-10-08.14:10:09
On Wed, Oct 08, 2008 at 08:25:09AM -0000, Eric Kow wrote:
> Ok, so we've had two people independently study the code, who both believe there
> is no bug in sent (at worst, we have poor handling of repository corruption),
> and we have had success reproducing the original bug in any version of darcs
> (this is from an IRC conversation with Trent).
> 
> Any objections if I mark this not-our-bug?  I'm setting this back to need-info
> because we don't have a clear idea what to do next, but we know we need some
> kind of test case.  Over to you, Thorkil.

I don't think not-our-bug is appropriate, as it almost certainly is
our bug.  It's just not in Sent.  Any time there's repository
corruption, we have to assume that it's our bug until we've proven
otherwise.  We could have an unreproducible flag of some sort, but I
think this would be a wrong use of not-our-bug.

I do agree that we should ignore this until we get more information.

David
msg6279 (view) Author: kowey Date: 2008-10-08.14:13:34
Right.  Our bug after all.
msg6284 (view) Author: twb Date: 2008-10-08.22:08:24
On Wed, Oct 08, 2008 at 08:25:09AM -0000, Eric Kow wrote:
> 
> Eric Kow <kowey@darcs.net> added the comment:
> 
> Ok, so we've had two people independently study the code, who both believe there
> is no bug in sent (at worst, we have poor handling of repository corruption),
> and we have had success reproducing the original bug in any version of darcs
> (this is from an IRC conversation with Trent).

I'd tag it 'unreproducible'.
msg8514 (view) Author: kowey Date: 2009-08-26.14:19:48
After all that batting-around we finally grew that 'presumed-dead' status we
were asking for ('unreproducible')
History
Date User Action Args
2008-09-23 09:03:05twbcreate
2008-09-24 02:00:43twbsetstatus: unread -> unknown
nosy: kowey, dagit, simon, twb
messages: + msg6127
title: darcs patch: Prevent installdocs from installing TeX intermediaries -> darcs send error w/ gpg and --edit in 2.0.3pre1
2008-09-24 02:20:12twbsetnosy: - twb
2008-09-24 02:23:26twbsetnosy: + twb
2008-09-28 19:22:55simonsetpriority: bug
nosy: + thorkilnaur
2008-09-30 16:16:20droundysetnosy: + droundy
messages: + msg6179
2008-10-01 09:09:26thorkilnaursetstatus: unknown -> needs-reproduction
nosy: + dmitry.kurochkin
topic: + Target-2.1
messages: + msg6182
2008-10-01 16:15:46thorkilnaursetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6187
2008-10-02 09:41:41koweysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
title: darcs send error w/ gpg and --edit in 2.0.3pre1 -> send => fromJust errorsrc/Darcs/Commands/Send.lhs:196 (2.0.3pre1)
2008-10-02 09:42:06koweysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
title: send => fromJust errorsrc/Darcs/Commands/Send.lhs:196 (2.0.3pre1) -> send => fromJust errorsrc/Darcs/Commands/Send.lhs:196 (2.0.3pre2)
2008-10-02 12:13:10koweysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6199
2008-10-07 15:43:35koweysettopic: + Regression
nosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6265
2008-10-07 19:25:47koweysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6267
2008-10-07 19:39:07koweysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6268
2008-10-07 19:49:29koweysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6269
2008-10-07 22:37:43droundysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6270
2008-10-07 22:40:03droundysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6271
2008-10-08 08:25:09koweysetpriority: bug -> not-our-bug
status: needs-reproduction -> waiting-for
messages: + msg6273
nosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
2008-10-08 14:10:12droundysetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6278
2008-10-08 14:13:36koweysetpriority: not-our-bug -> bug
nosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6279
2008-10-08 14:14:03koweysettopic: - Regression, Target-2.1
nosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
2008-10-08 22:08:27twbsetnosy: droundy, kowey, dagit, simon, twb, thorkilnaur, dmitry.kurochkin
messages: + msg6284
2009-08-06 18:00:59adminsetnosy: + markstos, jast, Serware, darcs-devel, zooko, mornfall, tommy, beschmi, - droundy, twb
2009-08-06 21:12:48adminsetnosy: - beschmi
2009-08-10 21:48:56adminsetnosy: + twb, - tommy, markstos, darcs-devel, zooko, jast, Serware, mornfall
2009-08-10 23:46:18adminsetnosy: - dagit
2009-08-25 17:25:21adminsetnosy: + darcs-devel, - simon
2009-08-26 14:19:50koweysetstatus: waiting-for -> given-up
nosy: kowey, darcs-devel, twb, thorkilnaur, dmitry.kurochkin
messages: + msg8514
2009-08-27 14:33:18adminsetnosy: kowey, darcs-devel, twb, thorkilnaur, dmitry.kurochkin