[darcs-users] [patch374] ADV: the initial pile
Eric Kow
kowey at darcs.net
Thu Sep 2 12:28:33 UTC 2010
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 <me at mornfall.net>
> * Update haddock.
>
> Wed Aug 11 17:39:29 CEST 2010 Petr Rockai <me at mornfall.net>
> * First stab at a hashed-storage 0.6 port.
>
> Wed Aug 11 21:25:55 CEST 2010 Petr Rockai <me at mornfall.net>
> * Move the preferences system into IO where it belongs.
>
> Wed Aug 11 21:45:04 CEST 2010 Petr Rockai <me at mornfall.net>
> * Make FileName an alias to Relative (from Hashed.Storage.Path).
>
> Wed Aug 11 22:12:49 CEST 2010 Petr Rockai <me at mornfall.net>
> * Fix annotate that got broken due to path format change.
>
> Thu Aug 12 00:02:43 CEST 2010 Petr Rockai <me at mornfall.net>
> * Replace FilePath with FileName in SelectChanges and ChooseTouching.
>
> Thu Aug 12 00:09:46 CEST 2010 Petr Rockai <me at mornfall.net>
> * Make SubPath just another alias for Relative.
>
> Thu Aug 12 00:16:21 CEST 2010 Petr Rockai <me at 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 <me at mornfall.net>
> * Merge Darcs.Patch.FileName into Darcs.Path.
>
> Thu Aug 12 00:47:39 CEST 2010 Petr Rockai <me at mornfall.net>
> * Remove the now-redundant sp2fn.
>
> Thu Aug 12 01:05:34 CEST 2010 Petr Rockai <me at mornfall.net>
> * Fix announceFiles in WhatsNew (abolish unsafePathFrom*).
>
> Thu Aug 12 01:06:04 CEST 2010 Petr Rockai <me at mornfall.net>
> * Restore the ".." check in isMaliciousPath.
>
> Thu Aug 12 01:29:40 CEST 2010 Petr Rockai <me at 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 <me at 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:kowey at jabber.fr (Jabber or Google Talk only)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20100902/667d0d14/attachment.pgp>
More information about the darcs-users
mailing list