On Sat, Apr 26, 2008 at 4:22 AM, David Roundy <[EMAIL PROTECTED]> wrote:
> > This looks like a wrong change to me. It may not currently introduce a > bug, but your concerns are correct. At a minimum it introduces a latent > bug that could bite our users if we ever add the --summary flag to any > command that affects our repository. It also changes the behavior of > darcs > whatsnew to be less accurate. I should have been more clear, but I was rushing when I wrote the email description. I really had intended this to bring out the problem for discussion and not seriously intended this change to be accepted. I do appreciate that you commented on it though because your feedback has been useful. > > > > slurp_pending :: RepoPatch p => Repository p -> IO Slurpy > > -slurp_pending repo = do > > +slurp_pending repo@(Repo _ opts _ _) = do > > cur <- slurp_recorded repo > > hunk ./src/Darcs/Repository/Internal.lhs 234 > > - pend <- read_pending repo > > + pend <- if Summary `elem` opts > > + then return ( NilFL :: FL Prim) > > + else read_pending repo > > case apply_to_slurpy pend cur of > > Just pendcur -> return pendcur > > If we ever add the --summary flag to (for instance) record, push or pull, > this will corrupt repositories. I could easily imagine wanting to see a > summary of each patch that I'm prompted to pull, as if I had hit 'x'. So > no, this is definitely not the way to go. > > The real fix for Issue814 is to ignore any changes to newly added files > when doing whatsnew -s, and this change needs to happen in diff. The diff that causes the file to be read into memory happens in recur_diff, in the 3rd case where, s == s'. I changed this case so that in summary mode it skips doing the gendiff of s and s', this makes 'whatsnew -s' essentially instantanious for this case. Obviously this is the wrong way to fix the problem because my way would cause all diffs of existing files to be skipped when in summary mode. What we want to do is ignore the diff of files that are new. So, that makes me think to fix this problem we'd have to change what we report in the slurpies during the 'whatsnew' case. Changing the slurpy behavior is something I'm not prepared to dig into at this time. I think I should put this away and work on other things for the time being. I expected this would be low hanging fruit, but I see now the problem is more fundamental. > Gwern's > idea of making is_funky faster is always good (since it speeds up many > darcs commands, if only a little), but I don't think it touches the real > problem, which is that we shouldn't be opening these files at all. Right, and actually I've been working with Gwern over IRC about optimizing whatsnew and specifically the binary detection. He's noticed that co_slurpy is strict in the IO which is interesting. I think he's looking at getting mmap working for address spaces that are greater than 32bits. If you look in fpstring.c you'll see that my_mmap takes an int where it should work with size_t, but I don't think that's the only problem. I think some of the FFI stuff needs changing too. By the way, what does the "co" in co_slurpy mean? Thanks, Jason
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
