darcs

Patch 333 Accept issue1909: unrecord -O in a tagge... (and 1 more)

Title Accept issue1909: unrecord -O in a tagge... (and 1 more)
Superseder Nosy List kowey, mornfall, tux_rocker
Related Issues
Status accepted Assigned To tux_rocker
Milestone

Created on 2010-08-07.17:13:29 by mornfall, last changed 2011-05-10.22:07:43 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
accept-issue1909_-unrecord-_o-in-a-tagged-repo-makes-a-bad-bundle_.dpatch mornfall, 2010-08-07.17:13:28 text/x-darcs-patch
unnamed mornfall, 2010-08-07.17:13:28
See mailing list archives for discussion on individual patches.
Messages
msg12030 (view) Author: mornfall Date: 2010-08-07.17:13:28
Bleh bleh. Those pesky contexts...

2 patches for repository http://darcs.net/:

Sat Aug  7 19:12:45 CEST 2010  Petr Rockai <me@mornfall.net>
  * Accept issue1909: unrecord -O in a tagged repo makes a bad bundle.

Sat Aug  7 19:15:52 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1909: generate correct context in unpull -O.
Attachments
msg12068 (view) Author: kowey Date: 2010-08-09.09:37:11
If you wouldn't mind having a look, Reinier (I'm a little reluctant to 
send patches your way for fear of overloading with your RM duties.  Is 
this OK?  It being release critical helps, I hope...)
msg12094 (view) Author: tux_rocker Date: 2010-08-10.19:00:50
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
msg12141 (view) Author: mornfall Date: 2010-08-12.14:37:51
Reinier, please re-check. It *is* failing for me without the appropriate 
Resolve xxx: patch. You need to run it explicitly if it's prefixed with 
failing-.

$ cabal test tests/failing-issue1909-unrecord-O-misses-tag.sh
(etc etc)

grep TAG c.dpatch
+ grep TAG c.dpatch

Some tests failed:
failing-issue1909-unrecord-O-misses-tag.sh
msg12144 (view) Author: darcswatch Date: 2010-08-12.19:22:27
This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-f7ee3609f2d86f7270fe0e1bd5d7591d66829c9f
msg12145 (view) Author: tux_rocker Date: 2010-08-12.19:56:38
> Petr Ročkai <me@mornfall.net> added the comment:
> Reinier, please re-check. It *is* failing for me without the appropriate 
> Resolve xxx: patch. You need to run it explicitly if it's prefixed with 
> failing-.
> 
> $ cabal test tests/failing-issue1909-unrecord-O-misses-tag.sh
> (etc etc)
> 
> grep TAG c.dpatch
> + grep TAG c.dpatch
> 
> Some tests failed:
> failing-issue1909-unrecord-O-misses-tag.sh

Turns out it passed only for old-fashioned repositories. Somehow I 
misinterpreted that. Apologies for the inconvenience...

Applied, thanks!
msg14407 (view) Author: darcswatch Date: 2011-05-10.22:07:43
This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-f7ee3609f2d86f7270fe0e1bd5d7591d66829c9f
History
Date User Action Args
2010-08-07 17:13:29mornfallcreate
2010-08-07 17:14:27darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-f7ee3609f2d86f7270fe0e1bd5d7591d66829c9f
2010-08-09 09:37:11koweysetassignedto: tux_rocker
messages: + msg12068
nosy: + kowey, tux_rocker
2010-08-10 19:00:51tux_rockersetmessages: + msg12094
2010-08-10 19:15:59tux_rockersetstatus: needs-review -> followup-requested
2010-08-12 14:37:51mornfallsetstatus: followup-requested -> review-in-progress
messages: + msg12141
2010-08-12 19:22:27darcswatchsetstatus: review-in-progress -> accepted
messages: + msg12144
2010-08-12 19:56:38tux_rockersetmessages: + msg12145
2011-05-10 22:07:43darcswatchsetmessages: + msg14407