Reinier Lamers <[email protected]> added the comment: Hi all,
This patch looks okay. However, the test succeeds even when I haven't applied your fix. The context includes both T and a then. > [Accept issue1909: unrecord -O in a tagged repo makes a bad bundle. > Petr Rockai <[email protected]>**20100807171245 > > Ignore-this: d8b2d7563cd4612814a209a515358cb4 > > ] addfile ./tests/failing-issue1909-unrecord-O-misses-tag.sh > hunk ./tests/failing-issue1909-unrecord-O-misses-tag.sh 1 > +#!/usr/bin/env bash > +## issue1909: unrecord -O in tagged repo makes a busted bundle > + > +. lib > + > +rm -rf R > +mkdir R > +darcs init --repo R > +echo a > R/a > +darcs rec -lam a --repo R --ignore-times > +darcs tag -m T --repo R > +echo b > R/a > +darcs rec -lam b --repo R --ignore-times > +echo c > R/a > +darcs rec -lam c --repo R --ignore-times > + > +darcs unpull -p c -a --repo R -O > +cat c.dpatch > +grep '^\[b' c.dpatch > +grep TAG c.dpatch So here we hae a test that checks if both b and the tag T are mentioned in a bundle that contains c which was unrecorded from a repo with a history a-T-b- c. > [Resolve issue1909: generate correct context in unpull -O. > Petr Rockai <[email protected]>**20100807171552 > > Ignore-this: 8d66f660e691ffe76a8da1eab9e5dcc9 > > ] move ./tests/failing-issue1909-unrecord-O-misses-tag.sh > ./tests/issue1909-unrecord-O-misses-tag.sh hunk > ./src/Darcs/Commands/Unrecord.lhs 62 > > import Darcs.SelectChanges ( selectChanges > > , WhichChanges(..) > , selectionContext, runSelection ) > > -import Darcs.Patch.Bundle ( makeBundle, patchFilename ) > +import Darcs.Patch.Bundle ( makeBundle, patchFilename, contextPatches ) Here we import the contextPatches function that Petr made in the 1873 resolution. It returns a list of the patches that form the context. > hunk ./src/Darcs/Commands/Unrecord.lhs 337 > > return () > > putStrLn $ "Finished " ++ presentParticiple cmdname ++ "." > > -matchingHead :: Patchy p => [DarcsFlag] -> PatchSet p C(Origin r) > +matchingHead :: RepoPatch p => [DarcsFlag] -> PatchSet p C(Origin r) > > -> FlippedSeal (RL (PatchInfoAnd p)) C(r) > This makes the type a bit more specific: the function no longer works upon anything that's like a patch (Patchy), but only on RepoPatch'es. We have to do this because we're going to use contextPatches which only works on RepoPatch'es. > hunk ./src/Darcs/Commands/Unrecord.lhs 339 > -matchingHead opts (PatchSet x _) > - | or (mapRL (matchAPatchread opts) x) = FlippedSeal x > +matchingHead opts set@(PatchSet x _) > + | or (mapRL (matchAPatchread opts) x) = contextPatches set > matchingHead opts (PatchSet x (Tagged t _ ps :<: ts)) > > = (x +<+) `mapFlipped` matchingHead opts (PatchSet (t:<:ps) ts) > > matchingHead _ (PatchSet _ NilRL) = FlippedSeal NilRL And here we switch to using contextPatch. This matchingHead function appears to return an RL (in a FlippedSeal) that goes back from the top of the repo history to the latest tag such that there is at least one patch matching the DarcsFlag's in that RL. So it seems the most important difference is that the result of matchingHead includes the tag at its end, whereas previously this tag was left out. For the context of a bundle this is desired, and for patch selection it seems desired too. Reinier __________________________________ Darcs bug tracker <[email protected]> <http://bugs.darcs.net/patch333> __________________________________ _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
