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 <me@mornfall.net>**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 <me@mornfall.net>**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
|