On Mon, Aug 30, 2010 at 08:43:30 +0000, Petr Ročkai wrote: > 61 patches for repository http://darcs.net:
Continuing to chip away at this bundle and trying to learn how to review more efficiently. > Sat Jul 17 10:40:48 CEST 2010 Petr Rockai <m...@mornfall.net> > * Update haddock. > > Wed Aug 11 17:39:29 CEST 2010 Petr Rockai <m...@mornfall.net> > * First stab at a hashed-storage 0.6 port. > > Wed Aug 11 21:25:55 CEST 2010 Petr Rockai <m...@mornfall.net> > * Move the preferences system into IO where it belongs. > > Wed Aug 11 21:45:04 CEST 2010 Petr Rockai <m...@mornfall.net> > * Make FileName an alias to Relative (from Hashed.Storage.Path). > > Wed Aug 11 22:12:49 CEST 2010 Petr Rockai <m...@mornfall.net> > * Fix annotate that got broken due to path format change. > > Thu Aug 12 00:02:43 CEST 2010 Petr Rockai <m...@mornfall.net> > * Replace FilePath with FileName in SelectChanges and ChooseTouching. > > Thu Aug 12 00:09:46 CEST 2010 Petr Rockai <m...@mornfall.net> > * Make SubPath just another alias for Relative. > > Thu Aug 12 00:16:21 CEST 2010 Petr Rockai <m...@mornfall.net> > * Introduce a new Darcs.Path module to centralise path handling. Covered in last review. > Thu Aug 12 00:36:15 CEST 2010 Petr Rockai <m...@mornfall.net> > * Merge Darcs.Patch.FileName into Darcs.Path. > > Thu Aug 12 00:47:39 CEST 2010 Petr Rockai <m...@mornfall.net> > * Remove the now-redundant sp2fn. > > Thu Aug 12 01:05:34 CEST 2010 Petr Rockai <m...@mornfall.net> > * Fix announceFiles in WhatsNew (abolish unsafePathFrom*). > > Thu Aug 12 01:06:04 CEST 2010 Petr Rockai <m...@mornfall.net> > * Restore the ".." check in isMaliciousPath. > > Thu Aug 12 01:29:40 CEST 2010 Petr Rockai <m...@mornfall.net> > * Fix a subtle bug in onlyHunks with rather curious side-effects. I stopped here. > Thu Aug 12 01:30:27 CEST 2010 Petr Rockai <m...@mornfall.net> > * Merge Darcs.RepoPath into Darcs.Path. More to go! Merge Darcs.Patch.FileName into Darcs.Path. ------------------------------------------- > - then do has_target <- treeHasDir cur (fn2fp $ superName $ fp2fn new) > + then do has_target <- treeHasDir cur (fn2fp $ parent $ fp2fn new) Getting some sort of notion that we a get little renames like this > - result <- foldM removeOnePath > (ftf,recorded,unrecorded, []) $ > - map pathFromSubPath files > + result <- foldM removeOnePath > (ftf,recorded,unrecorded, []) files And less need for conversions like this. (Presumably because we're accepting more FileName and fewer FilePath) > hunk ./src/Darcs/Patch/Named.lhs 42 > -import Darcs.Witnesses.Eq ( EqCheck(..) ) > -import Darcs.Witnesses.Ordered ( FL(..), RL(..), mapFL, concatFL > - , mapFL_FL ) > -import Darcs.Patch.Prim ( Prim(..), FromPrim(..), Effect(effect, effectRL), > nFn ) > -#include "impossible.h" > - > -data Patch C(x y) where > - PP :: Prim C(x y) -> Patch C(x y) > - ComP :: FL Patch C(x y) -> Patch C(x y) > - Merger :: Patch C(x y) > - -> RL Patch C(x b) > - -> Patch C(c b) > - -> Patch C(c d) > - -> Patch C(x y) > - Regrem :: Patch C(x y) > - -> RL Patch C(x b) > - -> Patch C(c b) > - -> Patch C(c a) > - -> Patch C(y x) > - > -instance FromPrim Patch where > - fromPrim = PP > +import Darcs.Witnesses.Ordered ( concatFL ) > +import Darcs.Patch.Prim ( Effect(effect, effectRL), nFn ) > hunk ./src/Darcs/Patch/Named.lhs 42 > -import Darcs.Witnesses.Ordered ( concatFL ) > hunk ./src/Darcs/Patch/Named.lhs 40 > -import Darcs.Patch.Info ( PatchInfo, patchinfo, makeFilename ) > -import Darcs.Patch.Patchy ( Patchy ) > -import Darcs.Patch.Prim ( Effect(effect, effectRL), nFn ) > +import Darcs.Patch.Info ( PatchInfo, patchinfo, makeFilename, invertName, > idpatchinfo ) > +import Darcs.Patch.Patchy ( Patchy, Commute(..), Invert(..) ) > +import Darcs.Patch.Prim ( Effect(effect, effectRL), nFn, Conflict(..) ) > +import Darcs.Witnesses.Eq ( MyEq(..) ) > +import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), (:/\:)(..) ) > hunk ./src/Darcs/Patch/Named.lhs 41 > -import Darcs.Patch.Patchy ( Patchy, Commute(..), Invert(..) ) > +import Darcs.Patch.Patchy ( Patchy, Commute(..), Merge(..), > PatchInspect(..), Invert(..) ) > ] > : > hunk ./src/Darcs/Patch/Named.lhs 45 > -import Darcs.Patch.Prim ( Prim(..), FromPrim(..), Effect(effect, effectRL), > nFn ) > +import Darcs.Patch.Prim ( Prim(..), FromPrim(..), Effect(effect, effectRL) ) > hunk ./src/Darcs/Patch/FileName.hs 1 Module is nuked. I'm assuming that the code moved in Darcs.Path is only hunk-moved. I'll just restate my reservations about the future stability of Darcs.Path, that we'll need to make it sure that should Darcs.Path change in the future our behaviour with respect to pre-existing patches does not also change. Our of the challenges in the Darcs hacking context is that correctness for Darcs is partly defined by historical precedent, ie. "does not behave like it used to" can be considered a bug. :-/ [I guess folks working on browser etc have similar problems, cf. quirks mode, just with somewhat lower stakes] > -pathFromSubPath :: SubPath -> Relative > -pathFromSubPath x = y -- trace ("pathFromSubPath: " ++ show x ++ " -> " ++ > show y) y No longer needed because they are the same. Remove the now-redundant sp2fn. ------------------------------- Skipping this and just assuming that it's a natural consequence of the new consolidated FileName == SubPath == Storage.Hashed.Relative Fix announceFiles in WhatsNew (abolish unsafePathFrom*). -------------------------------------------------------- > - let paths = map toFilePath files > - check = virtualTreeIO (mapM exists $ map unsafePathFromString > paths) > + let check = virtualTreeIO (mapM exists files) Seems nice, but it's not clear to me what the fix actually is. Restore the ".." check in isMaliciousPath. ------------------------------------------ > +isMaliciousPath = forbidden [ "_darcs", ".." ] > + where forbidden bad (directory -> dir :/: rest) = dir `elem` bad || > forbidden bad rest > + forbidden _ _ = False This is one of those functions where you don't need to check old behaviour, just the new one. forbidden: Nice use of English for clarity. Unless I misunderstand the (foo -> pattern) syntax, this is just recursively walking the path elements and checking to see if any of the elements are forbidden. Why have explicit recursion when we can just convert to a list and use any? Fix a subtle bug in onlyHunks with rather curious side-effects. --------------------------------------------------------------- > onlyHunks :: [Sealed (FL Prim C(x))] -> Bool > onlyHunks [] = False > -onlyHunks pss = fn2fp f /= "" && all oh pss > +onlyHunks pss = fn2fp f /= "." && all oh pss > where f = getAFilename pss > oh :: Sealed (FL Prim C(x)) -> Bool > oh (Sealed (p:>:ps)) = primIsHunk p && So we have a problem with assumptions about the representation of "." Perhaps we need to introduce some sort of "isRelativeRoot" function and abolish explicit checking on "."/"" instead? -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> For a faster response, try +44 (0)1273 64 2905 or xmpp:ko...@jabber.fr (Jabber or Google Talk only)
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list darcs-users@darcs.net http://lists.osuosl.org/mailman/listinfo/darcs-users