On Mon, Jun 07, 2010 at 20:06:18 +0000, Petr Ročkai wrote: > Mon Jun 7 20:48:49 CEST 2010 Petr Rockai <m...@mornfall.net> > * Haddock merge2FL and fastRemoveFL in Patch.Depends.
> Mon Jun 7 20:50:41 CEST 2010 Petr Rockai <m...@mornfall.net> > * Extend the issue1014 test to check that named patches are not duplicated. Pushed in my first review from 2010-06-08 > Mon Jun 7 20:50:10 CEST 2010 Petr Rockai <m...@mornfall.net> > * Extend the get test to cover --context interaction with tags. Applied, thanks! > Mon Jun 7 20:49:34 CEST 2010 Petr Rockai <m...@mornfall.net> > * Use merge2FL instead of plain merge in tentativelMergePatches. Applied, thanks! > Mon Jun 7 21:59:12 CEST 2010 Petr Rockai <m...@mornfall.net> > * Make partitionFL do a 3-way split, and detect commutation bugs in Depends. More review later We also talked about this bundle a little bit on IRC http://irclog.perlgeek.de/darcs/2010-06-23#i_2471430 To sum, the context to this patch bundle is that the recent NewSet works introduces some silent-but-deadly situations: (A) We no longer notice the anomalous situation where two different patches have the same patchinfo. This sometimes arises when you split a CVS repository into two Darcs repositories and then try to merge them back together in Darcs, eg. with nofib and GHC (because CVS does not have atomic commits, but it lets you recycle the commit message across different files) (B) The issue1014 test no longer complains, but now instead of crashing it causes your history to have TWO instances of the same patch (turning your patchset into an unfortunate patchbag) The game here is to attempt to resolve (A) and try to prepare for a future resolution of (B); so two independent patches which can be applied independently of each other. Haddock merge2FL and fastRemoveFL in Patch.Depends. --------------------------------------------------- > +-- | Merge two FLs (say L and R), starting in a common context. The result > is a > +-- FL starting in the original end context of L, going to a new context that > is > +-- the result of applying all patches from R on top of patches from L. This > +-- version also correctly handles duplicate patches. While trying to review the patch below, I found myself having lots of little questions about how merge2FL relates to mergeFL, which Petr clarified. I'll be following up with a haddock patch of my own, which Petr has acked on IRC: http://hpaste.org/fastcgi/hpaste.fcgi/view?id=26477#a26477 Use merge2FL instead of plain merge in tentativelMergePatches. -------------------------------------------------------------- As above, this patch does not actually have any effect; it's merely rearranging the furniture in anticipation for future work on issue1014. > Ignore-this: 9fde612f399cab5553e9cf57e0764685 > hunk ./src/Darcs/Repository/Merge.hs 58 > tentativelyMergePatches_ mc r cmd opts usi themi = > do let us = mapFL_FL hopefully usi > them = mapFL_FL hopefully themi > - _ :/\: pc <- return $ merge (progressFL "Merging them" them :\/: > progressFL "Merging us" us) > + Sealed pc <- return $ merge2FL (progressFL "Merging them" usi) > (progressFL "Merging us" themi) We switch from merge to merge2FL. Notes with thanks to Petr: - this involves flipping the order of arguments simply because the two functions have different conventions - merge2FL should not be confused with mergeFL. Merge2FL does this extra work of knocking out patches that share a patchinfo, which will be important in the future when we try to fix issue1014 (did anyone else that the new post 2008 Darcs team has been starting to wade into the core yet?) - this actually has no effect because the patches with identical patchinfo will already been filtered out in findCommonAndUncommon But in future work, that may change... The rest of this patch is just using hopefully because we're now manipulating PatchInfoAnd (Hopefully captures the business of partial repositories, if I understand correctly, Kudos to whoever haddocked it! It made reading things a lot faster.) > - anonpend <- anonymous (fromPrims pend) > + anonpend <- n2pia `fmap` anonymous (fromPrims pend) > pend' :/\: pw <- return $ merge (pc :\/: anonpend :>: NilFL) > - let pwprim = joinPatches $ progressFL "Examining patches for conflicts" > $ mapFL_FL patchcontents pw > + let pwprim = joinPatches $ progressFL "Examining patches for conflicts" > $ > + mapFL_FL (patchcontents . hopefully) pw > - have_unrecorded_conflicts <- checkUnrecordedConflicts opts pc > + have_unrecorded_conflicts <- checkUnrecordedConflicts opts $ mapFL_FL > hopefully pc Extend the get test to cover --context interaction with tags. ------------------------------------------------------------- > +cd temp1 > +darcs tag -m tt > +echo x > x > +darcs rec -lam "x" x > +darcs changes --context > my_context > +cd .. > + > +rm -rf temp2 > +darcs get temp1 --context="${abs_to_context}" temp2 > +darcs changes --context --repo temp2 > repo2_context > +diff -u ${abs_to_context} repo2_context Here we test for the case where the context file contains a tag (but not as its most recent patch). Seems to make sense. Make partitionFL do a 3-way split, and detect commutation bugs in Depends. -------------------------------------------------------------------------- I hope to review this after lunch or failing that, tomorrow. -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list darcs-users@darcs.net http://lists.osuosl.org/mailman/listinfo/darcs-users
