[issue535] darcs record; darcs send; darcs unrecord --last=1" fails in (partial) ghc head repo

[Claus Reinke]

>This is definitely a bug, and I believe it's also been reported before.  But
>it's a tricky bug to fix (--partial repository bugs generally are).  We (well,
>*I*, to be honest) never intended for --partial repositories to be used with
>development repositories, so a number of commands just aren't supported 
>(unless you're lucky).  

i've now followed some of the links on the tracker - if anything, i'm
even more disillusioned with darcs than before. the best thing would 
be to make this a priority over new features.

as that hasn't happened, the only honest thing seems to clearly
advertize --partial as a partially implemented feature, listing
explicitly those commands that have been looked over and are
known to work, and those commands for which bugs are known.

but as darcs is being used for huge repos, not having --partial 
would be a great loss, i think.

>:(  The code, unfortunately, is hard to fix.

that sounds strange - if, as you say, the issue is not inherent in
the patch model, some function is asking for more information
than it should need. and, as the comments in tracker and source
confirm, that is not good for non--partial repos, either.

>It used to be that unrecord was *always* unrecord --last=1, but we 
>made unrecord fancier, and this bug is one of the side-effects.

so, undoing that patch would be a first fix option!-)

until this bug is fixed, it might be useful to check and
advertize the workaround i mentioned (get darcs diff -u,
and apply it reversed to _darcs/pristine, then delete patch
and summary). as i haven't seen any other workaround 
proposed on the tracker, it might even be worth turning
this into a script/command that is able to figure out 
whether or not it is safely applicable.

a real fix would, of course, be best. and since this is a 
duplicate of an urgent ticket that has been tracked for
nearly a year (!), i'm surprised that not every darcs
hacker is on it (this looks a lot more basic than the 
other major issue: no redesign required, many more
users affected).

taking a peek into the sources, and backtracking from
functions that could generate the error message, the trouble 
seems to be the call to get_common_and_uncommon in 
get_last_patches. there are no comments on either, but 
there are comments for gcau, and it seems odd to me:

are you really taking allpatches and allpatches-{first},
and then try to find the patches not common to both
by eliminating all patches that are common to both,
all through the complete repo history?

[and what would one have to do to get into the other
 branch of the conditional in unrecord_cmd, which
 does a simple 'head allpatches'?]

there's a comment for gcau (copied to other functions
as well), saying that it shouldn't read more than needed,
but gcau can't tell how much that might be, right? and
since get_last_patches asks for all non-common patches,
gcau has to keep looking until it can be sure there are no
more of those.

whereas get_last_patches knows exactly that it is only 
interested in the most recent patches here.

is there any reason not to make use of that knowledge?
or am i completely on the wrong track?

other things to consider: 

- missingPatchError always claims that the issue is in
    get_extra, but it is called from other locations as well?

- if gcau runs over the end of a partial repo, doesn't
    that mean that the missing patches have to be the 
    same for its two parameter patch sets? it isn't as if
    the repo could be --partial to two separate complete
    repos, is it? so gcau should be able to return something
    like a partial result, with the caller able to decide
    whether the missing bits are likely to be common or not.

claus