darcs

Issue 535 darcs record; darcs send; darcs unrecord --last=1" fails in (partial) ghc head repo

Title darcs record; darcs send; darcs unrecord --last=1" fails in (partial) ghc head repo
Priority bug Status duplicate
Milestone Resolved in
Superseder darcs fails to unrecord a patch I just recorded (failed to read patch in get_extra) (1.0.8)
View: 317
Nosy List claus.reinke, cvs-ghc, darcs-devel, dmitry.kurochkin, kowey, thorkilnaur, tommy
Assigned To
Topics

Created on 2007-09-17.21:35:10 by claus.reinke, last changed 2009-08-27.13:51:57 by admin.

Messages
msg2129 (view) Author: claus.reinke Date: 2007-09-17.21:35:10
context:

    - i'm working on patches for ghc head
    - to send the patches for review, i'm supposed to use 'darcs send',
        which means i need to 'darcs record' them
    - the patches evolve in discussion until they get accepted,
        so i can't keep the interim versions of the patches recorded

the issue:

i thought that using

    darcs record; darcs send; darcs unrecord --last=1

would do what i need: being able to send a patch without keeping
it recorded in my local repo. but it fails:

    $ darcs unrecord --last=1

    Mon Sep 17 11:41:51 GMT Daylight Time 2007  claus.reinke@talk21.com
      * ghci :browse! :{:} :set :show languages/packages
    ..
    Shall I unrecord this patch? (1/1)  [ynWvpxqadjk], or ? for help:
    realdarcs.exe: failed to read patch in get_extra:
    Fri Dec 15 21:44:30 GMT Standard Time 2006  Ian Lynagh <igloo@earth.li>
      * Free more things that we allocate
    Perhaps this is a 'partial' repository?

    $ darcs -v
    1.0.9 (release)

    $ which darcs
    /cygdrive/c/darcs/darcsdir-cygwin/darcs

i have no idea whyever darcs would need to look beyond the last
patch if that is all i want to unrecord, so why is partial/complete
repo even an issue here? looks like a bug to me.

and is there a workaround that could help me avoid throwing away 
my repo and waiting a few days while pulling the huge complete repo 
over my slow connection?

is darcs really not useable with partial repos? that would be a pain.
i know that several ghc developers have already given up on partial
repos (there's even a good old-fashioned tarball to start from...).

if partial repos are unusable at the moment, what is the target date 
for fixing this issue?

thanks,
claus
msg2132 (view) Author: claus.reinke Date: 2007-09-18.18:58:35
>    darcs record; darcs send; darcs unrecord --last=1
>
>i have no idea whyever darcs would need to look beyond the last
>patch if that is all i want to unrecord, so why is partial/complete
>repo even an issue here? looks like a bug to me.
>
>and is there a workaround that could help me..?

i was lucky to have a recent backup of the repo but, without
wanting to put down darcs (i really like how easy it is to set up
and get started, or how i can put up a repo on simple http, so i
was -initially, at least- delighted to see it more or less scale up
to repos the size of ghc's), this doesn't look good (at all!), and
gives me a very insecure feeling when using darcs:

i shouldn't need to depend on backups, the revision control
system's main purpose is to keep my old versions safe and
accessible, and undoing the last record seems to be a very
basic feature of such a system.

having made a backup of _darcs before my next record/send,
i could check what darcs was doing and found what i expected:

    $ diff -qr _darcs ../_darcs/
    Only in ../_darcs/: .inventory.swp
    Files _darcs/inventory and ../_darcs/inventory differ
    Only in ../_darcs/patches: 20070918015956-969c0-7366c0b86461d3320d02e68a97981e9a7dbc3165.gz
    Only in ../_darcs/prefs: author
    Files _darcs/pristine/compiler/basicTypes/RdrName.lhs and
../_darcs/pristine/compiler/basicTypes/RdrName.lhs differ
    Files _darcs/pristine/compiler/ghci/InteractiveUI.hs and
../_darcs/pristine/compiler/ghci/InteractiveUI.hs differ
    Files _darcs/pristine/compiler/main/DynFlags.hs and ../_darcs/pristine/compiler/main/DynFlags.hs
differ
    Files _darcs/pristine/compiler/main/GHC.hs and ../_darcs/pristine/compiler/main/GHC.hs differ
    Files _darcs/pristine/docs/users_guide/ghci.xml and ../_darcs/pristine/docs/users_guide/ghci.xml
differ

the only differences were the extra patch summary at the end of
_darcs/inventory, the added patch file, and the patches recorded
into _darcs/pristine.

i still can see no reason why 'darcs unrecord --last=1' would
have to look beyond that: just unapply the last patch, to restore
_darcs/pristine, then remove the patch and patch summary.

to confirm, something like this appears to do what i expected
to achieve using 'darcs unrecord --last=1':

    darcs diff -u --last=1 >../diff-last.log
    (..remove unrecorded hunks from diff-last.log ..)
    patch -u -R -p1 -d _darcs/pristine <../diff-last.log

followed by removing the patch from _darcs/patches and
the summary from _darcs/inventory (the patch being the
newest, or the one refered to in the summary).

at least, the repo checks out as consistent, and 'darcs
whatsnew' lists the changes that used to be recorded
in the patch i removed.

are there reasons that 'darcs unrecord --last=1' doesn't
do this, or is it a bug? if it is not a bug, could a darcs
command be added to automate this essential function?

thanks,
claus
msg2140 (view) Author: droundy Date: 2007-09-21.16:59:11
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).  :(  The code, unfortunately, is hard to fix.

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.

David
msg2143 (view) Author: claus.reinke Date: 2007-09-22.15:32:06
>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
History
Date User Action Args
2007-09-17 21:35:11claus.reinkecreate
2007-09-18 18:58:36claus.reinkesetstatus: unread -> unknown
messages: + msg2132
2007-09-21 16:59:12droundysetmessages: + msg2140
2007-09-21 17:00:21droundylinkissue536 superseder
2007-09-21 17:01:46droundysetstatus: unknown -> duplicate
superseder: + darcs fails to unrecord a patch I just recorded (failed to read patch in get_extra) (1.0.8)
2007-09-22 15:32:07claus.reinkesetmessages: + msg2143
2009-08-06 17:45:47adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, dagit, mornfall, simon, thorkilnaur, - droundy, cvs-ghc, claus.reinke
2009-08-06 20:41:30adminsetnosy: - beschmi
2009-08-10 22:08:21adminsetnosy: + claus.reinke, cvs-ghc, - markstos, darcs-devel, zooko, jast, dagit, Serware, mornfall
2009-08-25 17:57:36adminsetnosy: + darcs-devel, - simon
2009-08-27 13:51:57adminsetnosy: tommy, kowey, darcs-devel, cvs-ghc, thorkilnaur, claus.reinke, dmitry.kurochkin