wferi@n0:~/nfsroot$ darcs --version
2.4.1 (release)
wferi@n0:~/nfsroot$ darcs revert -a ./root/etc/rcS.d ./root/etc/init.d/clgr
Reverting changes in "root/etc/init.d/clgr" "root/etc/rcS.d"..

darcs: bug at src/Darcs/Repository/Internal.hs:339 compiled Apr 14 2010
12:05:41
There was an attempt to write an invalid pending!
If possible, please send the contents of _darcs/patches/pending_buggy
along with a bug report.
See http://wiki.darcs.net/index.html/BugTrackerHowto for help on bug
reporting.

Attachments

• pending_buggy

Pending patch bugs should be taken *very* seriously (they can create
havoc further down the line in the event that darcs fails to notice them
on record time).

Ferenc, do you have any way of reproducing the error?

A good move would be for us to study the pending patch and figure out
what is wrong with it (which may give us an idea how it got created). 
The revert is a good clue.  The adddirs and moves may be worth noting.

Ferenc: if you need a work around, I think you can (after making a
backup for forensics) delete the pending patch and your index file.

Eric Kow <bugs@darcs.net> writes:

> Eric Kow <kowey@darcs.net> added the comment:
>
> Pending patch bugs should be taken *very* seriously (they can create
> havoc further down the line in the event that darcs fails to notice them
> on record time).
>
> Ferenc, do you have any way of reproducing the error?

I copied the repository, it reproduces the error message at the given
command.  I didn't try to do it from darcs init, though.

> A good move would be for us to study the pending patch and figure out
> what is wrong with it (which may give us an idea how it got created). 
> The revert is a good clue.  The adddirs and moves may be worth noting.
>
> Ferenc: if you need a work around, I think you can (after making a
> backup for forensics) delete the pending patch and your index file.

I didn't get stuck, did some records instead and the error disappeared.
Your starting paragraph gives me strange feelings, though...
-- 
Regards,
Feri.

On Tue, Apr 27, 2010 at 19:13:08 +0200, Ferenc Wagner wrote:
> I didn't get stuck, did some records instead and the error disappeared.
> Your starting paragraph gives me strange feelings, though...

I'd do a darcs check to be sure.

I suspect Darcs didn't try to use the buggy pending from revert.
Since you have a copy of the repository, you can look at the
pending files and compare.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

_darcs/patches/pending is not touched by the revert command.

Attachments

• pending

darcs revert -a ./root/etc/rcS.d succeeds, but after that darcs revert
-a ./root/etc/init.d/clgr gives the same error as the original command.

I lost control here. Why does darcs think I don't want to revert after
all?  Output:

$ ./revert_test1
/tmp/tmp.WSJJvaMHnA
Recording changes in "a/b/c":

Finished recording patch 'levels'
Reverting changes in "a/b/c"..

If you don't want to revert after all, that's fine with me!
Finished reverting.

On the other hand, darcs revert -a a/b works and reverts a/b/c as well.
Somebody is confused by the nesting here...

Attachments

• revert_test1

On Tue, Apr 27, 2010 at 18:21:26 +0000, Ferenc Wágner wrote:
> I lost control here. Why does darcs think I don't want to revert after
> all?  Output:

It could be darcs revert being confused by a/b/c not being in the
working directory.  Maybe darcs remove without actually removing
the file will have a different effect

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

I attach a script (pending_bug), which reproduces the error under darcs
2.4.1.

Attachments

• pending_bug

I can't really do more about this, I'm afraid.

Test case submitted (patch233).  Beautiful and minimal.  

We now need somebody to stare at the Darcs code and figure out what the
heck is happening.  I recommend we do this by the 2010-10 hacking sprint
at the latest.

I have trouble debugging this because there are no Show or Show2
instances for the datatypes used in the SelectChanges code, and the
witnesses stuff makes it hard to make them.

mornfall already fixed it before I had even finished telling him about
the issue on IRC. So assigning to him.

The following patch updated issue issue1825 with status=resolved;resolvedin=2.6.0 HEAD

* Resolve issue1825: do not omit important prims in unrecordedChanges w/ files. 
Ignore-this: 6948f29fe09882ec719269b21fc50ae7

This is a rather heavy-handed fix. We stop filtering the pending patch in
unrecordedChanges -- overall, this is an optimisation, since we can now avoid
reading the pending patch twice, and in most cases we also avoid filtering that
part of the patch twice. However, in exchange, whatsnew now has to filter the
patch (this was not required previously). This may be slightly less efficient
for diffs with very many changed files: however, it is unlikely to be a
bottleneck, since computing and even formatting the patches for display is much
more expensive.

Historical note (I'm trawling through pending bugs trying to figure out 
how stuff works): Petr's patch292 was reviewed by Ganesh (thanks for 
that!).  Part of the reason I'm doing this is because I suspect that 
this change has something to do with issue2212

The basic story as I understand it is that we would sometimes get 
invalid pending (can't be applied to pristine) when passing a set of 
files to restrict operations like revert.  The reason seems to be that 
we do a diff between (pristine + pending) vs working and return the 
resulting (pending +>+ diff).

If pending is restricted to only the patches that touch a subset of 
files, but the diff is not; the patch can be invalid.  The diff would be 
in the wrong context as it may contain says hunks for files we weren't 
looking at.

I think since the time of this patch, we've started to use the 
unrecordedChanges function in more places like darcs add (or was it 
always there?).  Now our struggle is that the pending patch contains 
“too much” stuff.  See issue2212

Feh, I forgot to the important bit, which is that the fix is essentially 
to move the filtering of patches (to see if they touch the file in 
question) out of `unrecordedChanges`.  The `revert` command already does 
it own touch filtering; however other commands which use this function 
would then need to do filtering of their own, as a post-processing step.  
Only whatsnew was updated accordingly.