darcs

Patch 801 Resolve issue1921: return Nothing in splitOnTag if the tag isn't in the patchset

Title Resolve issue1921: return Nothing in splitOnTag if the tag isn't in the patchset
Superseder Nosy List galbolle, owst
Related Issues
Status accepted Assigned To galbolle
Milestone

Created on 2012-04-09.01:34:25 by owst, last changed 2012-05-11.19:25:02 by gh.

Files
File name Status Uploaded Type Edit Remove
patch-preview.txt owst, 2012-04-09.01:34:25 text/x-darcs-patch
tidy_style-patch_depends.dpatch owst, 2012-04-09.01:34:25 application/x-darcs-patch
unnamed owst, 2012-04-09.01:34:25
See mailing list archives for discussion on individual patches.
Messages
msg15544 (view) Author: owst Date: 2012-04-09.01:34:25
Patches that accept, and resolve issue 1921.

An initial tidy-up, followed by a few more refactorings, and finally fix the
problem by allowing splitOnTag to signify when the tag isn't present in the
patchset.
10 patches for repository http://darcs.net:

Thu Apr  5 01:54:57 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Tidy/style Patch.Depends

Thu Apr  5 16:56:33 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Accept issue1921: attempting to apply a patch bundle based on missing tag dies.

Sun Apr  8 18:49:09 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * A few more operator spacing tidy-ups in Patch.Depends

Mon Apr  9 00:08:15 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Rename and refactor simpleTag in Patch.Depends, to better express intent

Mon Apr  9 00:49:01 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Remove unused getPatchesInTag function from Patch.Depends

Mon Apr  9 02:03:00 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Tweak a few identifiers and implementation of helpers for getTagsRight

Mon Apr  9 02:06:37 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Tweak issue1921 test to check for graceful failure

Mon Apr  9 02:10:53 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Resolve issue1921: return Nothing in splitOnTag if the tag isn't in the patchset

Mon Apr  9 02:15:12 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Fixup a couple of occurences of with_partial_intersection in Patch.Depends
  It's now taggedIntersection

Mon Apr  9 02:25:15 BST 2012  Owen Stephens <darcs@owenstephens.co.uk>
  * Add some Haddock and tweak some identifier names in Patch.Depends
Attachments
msg15545 (view) Author: owst Date: 2012-04-09.01:36:09
Self-screening, but I'd appreciate a quick review of this. (any insight
into the TODO comment on splitOnTag would be great!)
msg15643 (view) Author: galbolle Date: 2012-05-04.07:55:04
This depends on patch 770
msg15644 (view) Author: galbolle Date: 2012-05-04.07:55:59
no, it doesn't, sorry for being stupid
msg15645 (view) Author: galbolle Date: 2012-05-04.10:00:39
Applying, thanks

Tidy/style Patch.Depends
------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120405005457

Style patch indeed, nothing to see.


Accept issue1921: attempting to apply a patch bundle based on missing
tag dies.
-------------------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120405155633

addfile ./tests/issue1921-missing-tag-in-apply-bundle.sh

hunk ./tests/issue1921-missing-tag-in-apply-bundle.sh 1

> +. lib
> +
> +rm -rf R S T patch.dpatch
> +
> +darcs init --repo R
> +
> +cd R
> +
> +# Setup a repo with a tagged patch, and another patch ontop, so we
have a split
> +# inventory
> +touch file1
> +darcs rec -alm 'Add file1'
> +darcs tag -m 'file1 tag'
> +touch file2
> +darcs rec -alm 'Add file2'
> +
> +# Take a copy of the repo at this point
> +darcs get . ../S
> +
> +# Add the tag which we will fail on
> +darcs tag -m 'file2 tag'
> +
> +# Take a copy with the tag
> +darcs get . ../T
> +
> +# Add our patch which will depend only on the last tag.
> +echo 'file1' > file1
> +darcs rec -am 'file1 content'
> +
> +# Create a patch bundle with the new patch (by sending against the
repo we
> +# copied, with the last tag)
> +darcs send ../T -a -o ../patch.dpatch --no-edit-description
> +
> +cd ../S
> +# Try to apply to the patch which depends on the missing tag.
> +darcs apply ../patch.dpatch

Shouldn't this be added as failing-issue1921-….sh?

A few more operator spacing tidy-ups in Patch.Depends
-----------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120408174909

Ok

Remove unused getPatchesInTag function from Patch.Depends
---------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120408234901

Ok

Tweak a few identifiers and implementation of helpers for getTagsRight
----------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409010300

hunk ./src/Darcs/Patch/Depends.hs 42 and 44

[imports]

hunk ./src/Darcs/Patch/Depends.hs 172

hunk ./src/Darcs/Patch/Depends.hs 188

hunk ./src/Darcs/Patch/Depends.hs 194
>          | fst hp `elem` ds =
> -            case snd hp of
> -                Just ds' -> drop_tags_r (ds' ++ delete (fst hp) ds) pps
> -                Nothing -> drop_tags_r (delete (fst hp) ds) pps
> +            let extraDeps = fromMaybe [] $ snd hp in
> +            drop_tags_r (extraDeps ++ (delete (fst hp) ds)) pps
>          | otherwise = hp : drop_tags_r ds pps
>  
>  infoAndDeps :: PatchInfoAnd p wX wY -> (PatchInfo, Maybe [PatchInfo])

morally auto-hlint

replace ./src/Darcs/Patch/Depends.hs [A-Za-z_0-9] drop_tags_r dropDepsIn


Tweak issue1921 test to check for graceful failure
--------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409010637

hunk ./tests/issue1921-missing-tag-in-apply-bundle.sh 62
>  darcs send ../T -a -o ../patch.dpatch --no-edit-description
>  
>  cd ../S
> -# Try to apply to the patch which depends on the missing tag.
> -darcs apply ../patch.dpatch
> +
> +# Try to apply to the patch which depends on the missing tag (we
expect darcs
> +# to fail gracefully here)
> +not darcs apply ../patch.dpatch &> apply_output.txt
> +
> +# A best-attempt at ensuring darcs warns about the missing tag:
> +grep "tagged file2 tag" apply_output.txt
> +grep "FATAL: Cannot apply this bundle. We are missing the above
patches." apply_output.txt

Still failing (which incidentally is good for our tracking of bugs and
failing tests)

Resolve issue1921: return Nothing in splitOnTag if the tag isn't in the
patchset
--------------------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409011053


hunk ./src/Darcs/Commands/Optimize.hs 21
>  {-# LANGUAGE CPP #-}
>  
>  module Darcs.Commands.Optimize ( optimize ) where
> +
> +#include "impossible.h"
> +
>  import Control.Applicative ( (<$>) )
>  import Control.Exception ( finally )
>  import Control.Monad ( when, unless )

hunk ./src/Darcs/Commands/Optimize.hs 275
>  chooseOrder ps = case filter isTag $ mapRL info $ newset2RL ps of
>                    [] -> ps
>                    (lt:_) -> case splitOnTag lt ps of
> -                            PatchSet xs ts :> r -> PatchSet (r+<+xs) ts
> +                            Just (PatchSet xs ts :> r) ->
> +                                PatchSet (r+<+xs) ts
> +                            _ -> impossible
>  
>  optimizeUpgradeFormat :: IO ()
>  optimizeUpgradeFormat = do

hunk ./src/Darcs/Patch/Depends.hs 88
>  taggedIntersection bbb (PatchSet a (Tagged ta _ _ :<: _))
>      | Just (PatchSet b t) <- maybeSplitSetOnTag (info ta) bbb =
>      Fork t b (unsafeCoercePStart a)
> -taggedIntersection aaa (PatchSet b (Tagged tb _ pb :<: tbs)) =
> +taggedIntersection aaa ccc@(PatchSet b (Tagged tb _ pb :<: tbs)) =
>      case hopefullyM tb of
>          Just _ -> taggedIntersection aaa (PatchSet (b +<+ tb :<: pb) tbs)
>          Nothing -> case splitOnTag (info tb) aaa of

hunk ./src/Darcs/Patch/Depends.hs 92
> -                       PatchSet NilRL com :> us ->
> +                       Just (PatchSet NilRL com :> us) ->
>                             Fork com us (unsafeCoercePStart b)

hunk ./src/Darcs/Patch/Depends.hs 94
> -                       _ -> impossible
> +                       Just _ -> impossible
> +                       Nothing -> Fork NilRL (newset2RL aaa)
(newset2RL ccc)
>  
>  -- |'maybeSplitSetOnTag' takes a tag's 'PatchInfo', @t0@, and a
'PatchSet' and
>  -- attempts to find @t0@ in one of the 'Tagged's in the PatchSet. If
the tag is

hunk ./src/Darcs/Patch/Depends.hs 120
>          then if getTagsRight patchset == [info hp]
>                   -- special case to avoid looking at redundant patches
>                   then flipSeal NilRL
> -                 else case splitOnTag t patchset of _ :> e -> flipSeal e
> +                 else case splitOnTag t patchset of
> +                     Just (_ :> e) -> flipSeal e
> +                     _ -> impossible
>          else case getPatchesBeyondTag t (PatchSet ps ts) of
>                   FlippedSeal xxs -> FlippedSeal (hp :<: xxs)
>  getPatchesBeyondTag t (PatchSet NilRL NilRL) =

hunk ./src/Darcs/Patch/Depends.hs 132
>      getPatchesBeyondTag t0 (PatchSet (t :<: ps) ts)
>  
>  splitOnTag :: RepoPatch p => PatchInfo -> PatchSet p wStart wX
> -           -> (PatchSet p :> RL (PatchInfoAnd p)) wStart wX
> -splitOnTag t (PatchSet ps (Tagged hp x ps2 :<: ts)) | info hp == t =
> -    PatchSet NilRL (Tagged hp x ps2 :<: ts) :> ps
> +           -> Maybe ((PatchSet p :> RL (PatchInfoAnd p)) wStart wX)
> +splitOnTag t (PatchSet ps ts@(Tagged hp _ _ :<: _)) | info hp == t =
> +    Just $ PatchSet NilRL ts :> ps
>  splitOnTag t patchset@(PatchSet (hp :<: ps) ts) | info hp == t =
>      if getTagsRight patchset == [info hp]

hunk ./src/Darcs/Patch/Depends.hs 137
> -        then PatchSet NilRL (Tagged hp Nothing ps :<: ts) :> NilRL
> +        then Just $ PatchSet NilRL (Tagged hp Nothing ps :<: ts) :> NilRL
>          else case partitionRL ((`notElem` (t : ds)) . info) (hp :<:
ps) of
>              (x :<: a) :> b ->
>                  if getTagsRight (PatchSet (x :<: a) ts) == [t]

hunk ./src/Darcs/Patch/Depends.hs 141
> -                    then PatchSet NilRL (Tagged x Nothing a :<: ts) :> b
> -                    else case splitOnTag t $ eatOne $ PatchSet (x :<:
a) ts of
> -                        xx :> yy -> xx :> (b +<+ yy)
> +                    then Just $ PatchSet NilRL (Tagged x Nothing a
:<: ts) :> b
> +                    else do
> +                        let eatenOne = eatOne $ PatchSet (x :<: a) ts
> +                        xx :> yy <- splitOnTag t eatenOne
> +                        return $ xx :> (b +<+ yy)
>              _ -> impossible
>    where
>      ds = getdeps (hopefully hp)

hunk ./src/Darcs/Patch/Depends.hs 153
>      eatOne (PatchSet ps1 (Tagged x _ ps2 :<: ts')) =
>          PatchSet (ps1 +<+ x :<: ps2) ts'
>      eatOne _ = bug "a stubborn case in splitOnTag (theoretically
possible)"
> -splitOnTag t (PatchSet (p :<: ps) ts) = case splitOnTag t (PatchSet
ps ts) of
> -                                            ns :> x -> ns :> (p :<: x)
> +splitOnTag t (PatchSet (p :<: ps) ts) = do
> +    ns :> x <- splitOnTag t (PatchSet ps ts)
> +    return $ ns :> (p :<: x)
>  splitOnTag t0 (PatchSet NilRL (Tagged t _ ps :<: ts)) =
>      splitOnTag t0 (PatchSet (t :<: ps) ts)

hunk ./src/Darcs/Patch/Depends.hs 158
> -splitOnTag t0 (PatchSet NilRL NilRL) =
> -    bug $ "tag\n" ++ renderString (humanFriendly t0)
> -          ++ "\nis not in the patchset in splitOnTag."
> -
> +splitOnTag _ (PatchSet NilRL NilRL) = Nothing
>  
>  -- | @getTagsRight ps@ returns the 'PatchInfo' for all the patches in
>  --   @ps@ that are not depended on by anything else *through explicit

The whole patch is just propagating that everywhere, fine

Fixup a couple of occurences of with_partial_intersection in Patch.Depends
--------------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409011512

hunk ./src/Darcs/Patch/Depends.hs 66
>  import Printer ( renderString, vcat )
>  
>  {-|
> -with_partial_intersection takes two 'PatchSet's and splits them into
a /common/
> +taggedIntersection takes two 'PatchSet's and splits them into a /common/
>  intersection portion and two sets of patches.  The intersection, however,
>  is only lazily determined, so there is no guarantee that all intersecting
>  patches will be included in the intersection 'PatchSet'.  This is a
pretty

hunk ./src/Darcs/Patch/Depends.hs 73
>  efficient function, because it makes use of the already-broken-up
nature of
>  'PatchSet's.
>  
> -Note that the first argument to with_partial_intersection should be
> +Note that the first argument to taggedIntersection should be
>  the repository that is more cheaply accessed (i.e. local), as

hunk ./src/Darcs/Patch/Depends.hs 75
> -with_partial_intersection does its best to reduce the number of
> +taggedIntersection does its best to reduce the number of
>  inventories that are accessed from its rightmost argument.
>  -}
>  taggedIntersection :: forall p wStart wX wY. RepoPatch p =>

Ok


Add some Haddock and tweak some identifier names in Patch.Depends
-----------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409012515

Ok

hunk ./src/Darcs/Patch/Depends.hs 83
>                        Fork (RL (Tagged p))
>                             (RL (PatchInfoAnd p))
>                             (RL (PatchInfoAnd p)) wStart wX wY
> -taggedIntersection (PatchSet ps1 NilRL) s = Fork NilRL ps1 (newset2RL s)
> -taggedIntersection s (PatchSet ps2 NilRL) = Fork NilRL (newset2RL s) ps2
> -taggedIntersection bbb (PatchSet a (Tagged ta _ _ :<: _))
> -    | Just (PatchSet b t) <- maybeSplitSetOnTag (info ta) bbb =
> -    Fork t b (unsafeCoercePStart a)
> -taggedIntersection aaa ccc@(PatchSet b (Tagged tb _ pb :<: tbs)) =
> -    case hopefullyM tb of
> -        Just _ -> taggedIntersection aaa (PatchSet (b +<+ tb :<: pb) tbs)
> -        Nothing -> case splitOnTag (info tb) aaa of
> +taggedIntersection (PatchSet ps1 NilRL) s2 = Fork NilRL ps1
(newset2RL s2)
> +taggedIntersection s1 (PatchSet ps2 NilRL) = Fork NilRL (newset2RL
s1) ps2
> +taggedIntersection s1 (PatchSet ps2 (Tagged t _ _ :<: _))
> +    | Just (PatchSet ps1 ts1) <- maybeSplitSetOnTag (info t) s1 =
> +    Fork ts1 ps1 (unsafeCoercePStart ps2)
> +taggedIntersection s1 s2@(PatchSet ps2 (Tagged t _ p :<: ts2)) =
> +    case hopefullyM t of
> +        Just _ -> taggedIntersection s1 (PatchSet (ps2 +<+ t :<: p) ts2)
> +        Nothing -> case splitOnTag (info t) s1 of
>                         Just (PatchSet NilRL com :> us) ->

hunk ./src/Darcs/Patch/Depends.hs 93
> -                           Fork com us (unsafeCoercePStart b)
> +                           Fork com us (unsafeCoercePStart ps2)
>                         Just _ -> impossible

hunk ./src/Darcs/Patch/Depends.hs 95
> -                       Nothing -> Fork NilRL (newset2RL aaa)
(newset2RL ccc)
> +                       Nothing -> Fork NilRL (newset2RL s1)
(newset2RL s2)
>  
>  -- |'maybeSplitSetOnTag' takes a tag's 'PatchInfo', @t0@, and a
'PatchSet' and
>  -- attempts to find @t0@ in one of the 'Tagged's in the PatchSet. If
the tag is

hunk ./src/Darcs/Patch/Depends.hs 131
>  getPatchesBeyondTag t0 (PatchSet NilRL (Tagged t _ ps :<: ts)) =
>      getPatchesBeyondTag t0 (PatchSet (t :<: ps) ts)
>  
> +-- |splitOnTag takes a tag's 'PatchInfo', and a 'PatchSet', and
attempts to
> +-- find the tag in the PatchSet, returning a pair: the PatchSet "up
to" the
> +-- tag, and a RL of patches after the tag; If the tag is not in the
PatchSet,
> +-- we return Nothing.
>  splitOnTag :: RepoPatch p => PatchInfo -> PatchSet p wStart wX
>             -> Maybe ((PatchSet p :> RL (PatchInfoAnd p)) wStart wX)

hunk ./src/Darcs/Patch/Depends.hs 137
> +-- If the tag we are looking for is the first tag of the patchset,
return the
> +-- patchset upto the tag, and the patches after it.
>  splitOnTag t (PatchSet ps ts@(Tagged hp _ _ :<: _)) | info hp == t =
>      Just $ PatchSet NilRL ts :> ps

hunk ./src/Darcs/Patch/Depends.hs 141
> +-- If the tag is the most recent patch in the set, we check if the
patch is the
> +-- only non-depended-on patch in the set; creating a new Tagged out
of the
> +-- patches and tag, and adding it to the patchset, if this is the case.
> +-- Otherwise, we partition the patches into the tag/dependencies (and
their
> +-- implicit-dependencies) and then...
> +-- TODO: finish describing this case.
>  splitOnTag t patchset@(PatchSet (hp :<: ps) ts) | info hp == t =
>      if getTagsRight patchset == [info hp]
>          then Just $ PatchSet NilRL (Tagged hp Nothing ps :<: ts) :> NilRL

hunk ./src/Darcs/Patch/Depends.hs 165
>      eatOne (PatchSet ps1 (Tagged x _ ps2 :<: ts')) =
>          PatchSet (ps1 +<+ x :<: ps2) ts'
>      eatOne _ = bug "a stubborn case in splitOnTag (theoretically
possible)"
> +-- We drop the leading patch, to try and find a non-Tagged tag.
>  splitOnTag t (PatchSet (p :<: ps) ts) = do
>      ns :> x <- splitOnTag t (PatchSet ps ts)
>      return $ ns :> (p :<: x)

hunk ./src/Darcs/Patch/Depends.hs 169
> +-- If there are no patches left, we "unfold" the first Tagged, and
try again.
>  splitOnTag t0 (PatchSet NilRL (Tagged t _ ps :<: ts)) =
>      splitOnTag t0 (PatchSet (t :<: ps) ts)

hunk ./src/Darcs/Patch/Depends.hs 172
> +-- If we've checked all the patches, but haven't found the tag,
return Nothing.
>  splitOnTag _ (PatchSet NilRL NilRL) = Nothing
>  
>  -- | @getTagsRight ps@ returns the 'PatchInfo' for all the patches in
msg15646 (view) Author: galbolle Date: 2012-05-04.10:00:58
Applying, thanks

Tidy/style Patch.Depends
------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120405005457

Style patch indeed, nothing to see.


Accept issue1921: attempting to apply a patch bundle based on missing
tag dies.
-------------------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120405155633

addfile ./tests/issue1921-missing-tag-in-apply-bundle.sh

hunk ./tests/issue1921-missing-tag-in-apply-bundle.sh 1

> +. lib
> +
> +rm -rf R S T patch.dpatch
> +
> +darcs init --repo R
> +
> +cd R
> +
> +# Setup a repo with a tagged patch, and another patch ontop, so we
have a split
> +# inventory
> +touch file1
> +darcs rec -alm 'Add file1'
> +darcs tag -m 'file1 tag'
> +touch file2
> +darcs rec -alm 'Add file2'
> +
> +# Take a copy of the repo at this point
> +darcs get . ../S
> +
> +# Add the tag which we will fail on
> +darcs tag -m 'file2 tag'
> +
> +# Take a copy with the tag
> +darcs get . ../T
> +
> +# Add our patch which will depend only on the last tag.
> +echo 'file1' > file1
> +darcs rec -am 'file1 content'
> +
> +# Create a patch bundle with the new patch (by sending against the
repo we
> +# copied, with the last tag)
> +darcs send ../T -a -o ../patch.dpatch --no-edit-description
> +
> +cd ../S
> +# Try to apply to the patch which depends on the missing tag.
> +darcs apply ../patch.dpatch

Shouldn't this be added as failing-issue1921-….sh?

A few more operator spacing tidy-ups in Patch.Depends
-----------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120408174909

Ok

Remove unused getPatchesInTag function from Patch.Depends
---------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120408234901

Ok

Tweak a few identifiers and implementation of helpers for getTagsRight
----------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409010300

hunk ./src/Darcs/Patch/Depends.hs 42 and 44

[imports]

hunk ./src/Darcs/Patch/Depends.hs 172

hunk ./src/Darcs/Patch/Depends.hs 188

hunk ./src/Darcs/Patch/Depends.hs 194
>          | fst hp `elem` ds =
> -            case snd hp of
> -                Just ds' -> drop_tags_r (ds' ++ delete (fst hp) ds) pps
> -                Nothing -> drop_tags_r (delete (fst hp) ds) pps
> +            let extraDeps = fromMaybe [] $ snd hp in
> +            drop_tags_r (extraDeps ++ (delete (fst hp) ds)) pps
>          | otherwise = hp : drop_tags_r ds pps
>  
>  infoAndDeps :: PatchInfoAnd p wX wY -> (PatchInfo, Maybe [PatchInfo])

morally auto-hlint

replace ./src/Darcs/Patch/Depends.hs [A-Za-z_0-9] drop_tags_r dropDepsIn


Tweak issue1921 test to check for graceful failure
--------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409010637

hunk ./tests/issue1921-missing-tag-in-apply-bundle.sh 62
>  darcs send ../T -a -o ../patch.dpatch --no-edit-description
>  
>  cd ../S
> -# Try to apply to the patch which depends on the missing tag.
> -darcs apply ../patch.dpatch
> +
> +# Try to apply to the patch which depends on the missing tag (we
expect darcs
> +# to fail gracefully here)
> +not darcs apply ../patch.dpatch &> apply_output.txt
> +
> +# A best-attempt at ensuring darcs warns about the missing tag:
> +grep "tagged file2 tag" apply_output.txt
> +grep "FATAL: Cannot apply this bundle. We are missing the above
patches." apply_output.txt

Still failing (which incidentally is good for our tracking of bugs and
failing tests)

Resolve issue1921: return Nothing in splitOnTag if the tag isn't in the
patchset
--------------------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409011053


hunk ./src/Darcs/Commands/Optimize.hs 21
>  {-# LANGUAGE CPP #-}
>  
>  module Darcs.Commands.Optimize ( optimize ) where
> +
> +#include "impossible.h"
> +
>  import Control.Applicative ( (<$>) )
>  import Control.Exception ( finally )
>  import Control.Monad ( when, unless )

hunk ./src/Darcs/Commands/Optimize.hs 275
>  chooseOrder ps = case filter isTag $ mapRL info $ newset2RL ps of
>                    [] -> ps
>                    (lt:_) -> case splitOnTag lt ps of
> -                            PatchSet xs ts :> r -> PatchSet (r+<+xs) ts
> +                            Just (PatchSet xs ts :> r) ->
> +                                PatchSet (r+<+xs) ts
> +                            _ -> impossible
>  
>  optimizeUpgradeFormat :: IO ()
>  optimizeUpgradeFormat = do

hunk ./src/Darcs/Patch/Depends.hs 88
>  taggedIntersection bbb (PatchSet a (Tagged ta _ _ :<: _))
>      | Just (PatchSet b t) <- maybeSplitSetOnTag (info ta) bbb =
>      Fork t b (unsafeCoercePStart a)
> -taggedIntersection aaa (PatchSet b (Tagged tb _ pb :<: tbs)) =
> +taggedIntersection aaa ccc@(PatchSet b (Tagged tb _ pb :<: tbs)) =
>      case hopefullyM tb of
>          Just _ -> taggedIntersection aaa (PatchSet (b +<+ tb :<: pb) tbs)
>          Nothing -> case splitOnTag (info tb) aaa of

hunk ./src/Darcs/Patch/Depends.hs 92
> -                       PatchSet NilRL com :> us ->
> +                       Just (PatchSet NilRL com :> us) ->
>                             Fork com us (unsafeCoercePStart b)

hunk ./src/Darcs/Patch/Depends.hs 94
> -                       _ -> impossible
> +                       Just _ -> impossible
> +                       Nothing -> Fork NilRL (newset2RL aaa)
(newset2RL ccc)
>  
>  -- |'maybeSplitSetOnTag' takes a tag's 'PatchInfo', @t0@, and a
'PatchSet' and
>  -- attempts to find @t0@ in one of the 'Tagged's in the PatchSet. If
the tag is

hunk ./src/Darcs/Patch/Depends.hs 120
>          then if getTagsRight patchset == [info hp]
>                   -- special case to avoid looking at redundant patches
>                   then flipSeal NilRL
> -                 else case splitOnTag t patchset of _ :> e -> flipSeal e
> +                 else case splitOnTag t patchset of
> +                     Just (_ :> e) -> flipSeal e
> +                     _ -> impossible
>          else case getPatchesBeyondTag t (PatchSet ps ts) of
>                   FlippedSeal xxs -> FlippedSeal (hp :<: xxs)
>  getPatchesBeyondTag t (PatchSet NilRL NilRL) =

hunk ./src/Darcs/Patch/Depends.hs 132
>      getPatchesBeyondTag t0 (PatchSet (t :<: ps) ts)
>  
>  splitOnTag :: RepoPatch p => PatchInfo -> PatchSet p wStart wX
> -           -> (PatchSet p :> RL (PatchInfoAnd p)) wStart wX
> -splitOnTag t (PatchSet ps (Tagged hp x ps2 :<: ts)) | info hp == t =
> -    PatchSet NilRL (Tagged hp x ps2 :<: ts) :> ps
> +           -> Maybe ((PatchSet p :> RL (PatchInfoAnd p)) wStart wX)
> +splitOnTag t (PatchSet ps ts@(Tagged hp _ _ :<: _)) | info hp == t =
> +    Just $ PatchSet NilRL ts :> ps
>  splitOnTag t patchset@(PatchSet (hp :<: ps) ts) | info hp == t =
>      if getTagsRight patchset == [info hp]

hunk ./src/Darcs/Patch/Depends.hs 137
> -        then PatchSet NilRL (Tagged hp Nothing ps :<: ts) :> NilRL
> +        then Just $ PatchSet NilRL (Tagged hp Nothing ps :<: ts) :> NilRL
>          else case partitionRL ((`notElem` (t : ds)) . info) (hp :<:
ps) of
>              (x :<: a) :> b ->
>                  if getTagsRight (PatchSet (x :<: a) ts) == [t]

hunk ./src/Darcs/Patch/Depends.hs 141
> -                    then PatchSet NilRL (Tagged x Nothing a :<: ts) :> b
> -                    else case splitOnTag t $ eatOne $ PatchSet (x :<:
a) ts of
> -                        xx :> yy -> xx :> (b +<+ yy)
> +                    then Just $ PatchSet NilRL (Tagged x Nothing a
:<: ts) :> b
> +                    else do
> +                        let eatenOne = eatOne $ PatchSet (x :<: a) ts
> +                        xx :> yy <- splitOnTag t eatenOne
> +                        return $ xx :> (b +<+ yy)
>              _ -> impossible
>    where
>      ds = getdeps (hopefully hp)

hunk ./src/Darcs/Patch/Depends.hs 153
>      eatOne (PatchSet ps1 (Tagged x _ ps2 :<: ts')) =
>          PatchSet (ps1 +<+ x :<: ps2) ts'
>      eatOne _ = bug "a stubborn case in splitOnTag (theoretically
possible)"
> -splitOnTag t (PatchSet (p :<: ps) ts) = case splitOnTag t (PatchSet
ps ts) of
> -                                            ns :> x -> ns :> (p :<: x)
> +splitOnTag t (PatchSet (p :<: ps) ts) = do
> +    ns :> x <- splitOnTag t (PatchSet ps ts)
> +    return $ ns :> (p :<: x)
>  splitOnTag t0 (PatchSet NilRL (Tagged t _ ps :<: ts)) =
>      splitOnTag t0 (PatchSet (t :<: ps) ts)

hunk ./src/Darcs/Patch/Depends.hs 158
> -splitOnTag t0 (PatchSet NilRL NilRL) =
> -    bug $ "tag\n" ++ renderString (humanFriendly t0)
> -          ++ "\nis not in the patchset in splitOnTag."
> -
> +splitOnTag _ (PatchSet NilRL NilRL) = Nothing
>  
>  -- | @getTagsRight ps@ returns the 'PatchInfo' for all the patches in
>  --   @ps@ that are not depended on by anything else *through explicit

The whole patch is just propagating that everywhere, fine

Fixup a couple of occurences of with_partial_intersection in Patch.Depends
--------------------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409011512

hunk ./src/Darcs/Patch/Depends.hs 66
>  import Printer ( renderString, vcat )
>  
>  {-|
> -with_partial_intersection takes two 'PatchSet's and splits them into
a /common/
> +taggedIntersection takes two 'PatchSet's and splits them into a /common/
>  intersection portion and two sets of patches.  The intersection, however,
>  is only lazily determined, so there is no guarantee that all intersecting
>  patches will be included in the intersection 'PatchSet'.  This is a
pretty

hunk ./src/Darcs/Patch/Depends.hs 73
>  efficient function, because it makes use of the already-broken-up
nature of
>  'PatchSet's.
>  
> -Note that the first argument to with_partial_intersection should be
> +Note that the first argument to taggedIntersection should be
>  the repository that is more cheaply accessed (i.e. local), as

hunk ./src/Darcs/Patch/Depends.hs 75
> -with_partial_intersection does its best to reduce the number of
> +taggedIntersection does its best to reduce the number of
>  inventories that are accessed from its rightmost argument.
>  -}
>  taggedIntersection :: forall p wStart wX wY. RepoPatch p =>

Ok


Add some Haddock and tweak some identifier names in Patch.Depends
-----------------------------------------------------------------
Owen Stephens <darcs@owenstephens.co.uk>**20120409012515

Ok

hunk ./src/Darcs/Patch/Depends.hs 83
>                        Fork (RL (Tagged p))
>                             (RL (PatchInfoAnd p))
>                             (RL (PatchInfoAnd p)) wStart wX wY
> -taggedIntersection (PatchSet ps1 NilRL) s = Fork NilRL ps1 (newset2RL s)
> -taggedIntersection s (PatchSet ps2 NilRL) = Fork NilRL (newset2RL s) ps2
> -taggedIntersection bbb (PatchSet a (Tagged ta _ _ :<: _))
> -    | Just (PatchSet b t) <- maybeSplitSetOnTag (info ta) bbb =
> -    Fork t b (unsafeCoercePStart a)
> -taggedIntersection aaa ccc@(PatchSet b (Tagged tb _ pb :<: tbs)) =
> -    case hopefullyM tb of
> -        Just _ -> taggedIntersection aaa (PatchSet (b +<+ tb :<: pb) tbs)
> -        Nothing -> case splitOnTag (info tb) aaa of
> +taggedIntersection (PatchSet ps1 NilRL) s2 = Fork NilRL ps1
(newset2RL s2)
> +taggedIntersection s1 (PatchSet ps2 NilRL) = Fork NilRL (newset2RL
s1) ps2
> +taggedIntersection s1 (PatchSet ps2 (Tagged t _ _ :<: _))
> +    | Just (PatchSet ps1 ts1) <- maybeSplitSetOnTag (info t) s1 =
> +    Fork ts1 ps1 (unsafeCoercePStart ps2)
> +taggedIntersection s1 s2@(PatchSet ps2 (Tagged t _ p :<: ts2)) =
> +    case hopefullyM t of
> +        Just _ -> taggedIntersection s1 (PatchSet (ps2 +<+ t :<: p) ts2)
> +        Nothing -> case splitOnTag (info t) s1 of
>                         Just (PatchSet NilRL com :> us) ->

hunk ./src/Darcs/Patch/Depends.hs 93
> -                           Fork com us (unsafeCoercePStart b)
> +                           Fork com us (unsafeCoercePStart ps2)
>                         Just _ -> impossible

hunk ./src/Darcs/Patch/Depends.hs 95
> -                       Nothing -> Fork NilRL (newset2RL aaa)
(newset2RL ccc)
> +                       Nothing -> Fork NilRL (newset2RL s1)
(newset2RL s2)
>  
>  -- |'maybeSplitSetOnTag' takes a tag's 'PatchInfo', @t0@, and a
'PatchSet' and
>  -- attempts to find @t0@ in one of the 'Tagged's in the PatchSet. If
the tag is

hunk ./src/Darcs/Patch/Depends.hs 131
>  getPatchesBeyondTag t0 (PatchSet NilRL (Tagged t _ ps :<: ts)) =
>      getPatchesBeyondTag t0 (PatchSet (t :<: ps) ts)
>  
> +-- |splitOnTag takes a tag's 'PatchInfo', and a 'PatchSet', and
attempts to
> +-- find the tag in the PatchSet, returning a pair: the PatchSet "up
to" the
> +-- tag, and a RL of patches after the tag; If the tag is not in the
PatchSet,
> +-- we return Nothing.
>  splitOnTag :: RepoPatch p => PatchInfo -> PatchSet p wStart wX
>             -> Maybe ((PatchSet p :> RL (PatchInfoAnd p)) wStart wX)

hunk ./src/Darcs/Patch/Depends.hs 137
> +-- If the tag we are looking for is the first tag of the patchset,
return the
> +-- patchset upto the tag, and the patches after it.
>  splitOnTag t (PatchSet ps ts@(Tagged hp _ _ :<: _)) | info hp == t =
>      Just $ PatchSet NilRL ts :> ps

hunk ./src/Darcs/Patch/Depends.hs 141
> +-- If the tag is the most recent patch in the set, we check if the
patch is the
> +-- only non-depended-on patch in the set; creating a new Tagged out
of the
> +-- patches and tag, and adding it to the patchset, if this is the case.
> +-- Otherwise, we partition the patches into the tag/dependencies (and
their
> +-- implicit-dependencies) and then...
> +-- TODO: finish describing this case.
>  splitOnTag t patchset@(PatchSet (hp :<: ps) ts) | info hp == t =
>      if getTagsRight patchset == [info hp]
>          then Just $ PatchSet NilRL (Tagged hp Nothing ps :<: ts) :> NilRL

hunk ./src/Darcs/Patch/Depends.hs 165
>      eatOne (PatchSet ps1 (Tagged x _ ps2 :<: ts')) =
>          PatchSet (ps1 +<+ x :<: ps2) ts'
>      eatOne _ = bug "a stubborn case in splitOnTag (theoretically
possible)"
> +-- We drop the leading patch, to try and find a non-Tagged tag.
>  splitOnTag t (PatchSet (p :<: ps) ts) = do
>      ns :> x <- splitOnTag t (PatchSet ps ts)
>      return $ ns :> (p :<: x)

hunk ./src/Darcs/Patch/Depends.hs 169
> +-- If there are no patches left, we "unfold" the first Tagged, and
try again.
>  splitOnTag t0 (PatchSet NilRL (Tagged t _ ps :<: ts)) =
>      splitOnTag t0 (PatchSet (t :<: ps) ts)

hunk ./src/Darcs/Patch/Depends.hs 172
> +-- If we've checked all the patches, but haven't found the tag,
return Nothing.
>  splitOnTag _ (PatchSet NilRL NilRL) = Nothing
>  
>  -- | @getTagsRight ps@ returns the 'PatchInfo' for all the patches in
msg15647 (view) Author: galbolle Date: 2012-05-04.11:42:38
In fact, this depends on patch445.
msg15667 (view) Author: gh Date: 2012-05-11.19:25:02
Dependencies have been pushed; tests pass; pushing this bundle then.
History
Date User Action Args
2012-04-09 01:34:25owstcreate
2012-04-09 01:36:09owstsetstatus: needs-screening -> needs-review
messages: + msg15545
2012-04-09 15:05:06mndrixsettitle: Tidy/style Patch.Depends (and 9 more) -> Resolve issue1921: return Nothing in splitOnTag if the tag isn't in the patchset
2012-05-04 07:37:19galbollesetassignedto: galbolle
nosy: + galbolle
2012-05-04 07:55:05galbollesetmessages: + msg15643
2012-05-04 07:55:59galbollesetmessages: + msg15644
2012-05-04 10:00:43galbollesetmessages: + msg15645
2012-05-04 10:00:58galbollesetstatus: needs-review -> accepted
messages: + msg15646
2012-05-04 11:42:38galbollesetstatus: accepted -> accepted-pending-tests
messages: + msg15647
2012-05-11 19:25:02ghsetstatus: accepted-pending-tests -> accepted
messages: + msg15667