Have you already applied this? I may have found a bug on it. L.M.
2009/12/9 Luca Molteni <[email protected]>: > Fixed those two whitespace things. > > Bye, > > L.M. > > > > 2009/11/29 Reinier Lamers <[email protected]>: >> Hi all, >> >> Op zondag 15 november 2009 17:17 schreef Luca Molteni: >>> New submission from Luca Molteni <[email protected]>: >> >> Looks like it's about time for a review. I have a lot of questions about it. >> In a large part, that is because the existing cache code seems a bit dense >> to read through. >> >>>[issue1624 break global cache up into subdirectories >>>Luca Molteni <[email protected]>**20091115160511 >>> Ignore-this: ae4555fd3172a53549d90e42993dbce4 >>>] hunk ./src/Darcs/Repository/Cache.hs 23 >>> import System.Directory ( removeFile, doesFileExist, getDirectoryContents ) >>> import System.Posix.Files ( linkCount, getSymbolicLinkStatus ) >>> import System.IO ( hPutStrLn, stderr ) >>>+import System.FilePath (takeDirectory) >> >> Nitpick: as you can see, most of darcs source leaves spaces between the >> names of the imported functions and the parenthesis. >> >>> hunk ./src/Darcs/Repository/Cache.hs 113 >>> -- | @hashedFilePath cachelocation subdir hash@ returns the physical >>> filename of >>> -- hash @hash@ in the @subdir@ section of @cachelocat...@. >>> hashedFilePath :: CacheLoc -> HashedDir -> String -> String >>> -hashedFilePath (Cache Directory _ d) s f = d ++ "/" ++ (hashedDir s) ++ >>> "/" ++ f >>> +hashedFilePath (Cache Directory _ d) s f = d ++ "/" ++ (hashedDir s) ++ >>> "/" ++ (hashedBucket f) ++ "/" ++ f >>> + where hashedBucket f = take 2 $ case dropWhile (/='-') f of >>> + [] -> f >>> + s -> drop 1 s >> >> I initally assumed that f was a filename because of its name, but it is a >> hash. But that is not your mistake. >> >> Where does the dash ('-') in the hash come from? In what situation won't it >> be present, invoking the [] case in hashedBucket? >> >> What happens with old caches that still use the older names? Aren't they >> made useless by this change? >> >>> hunk ./src/Darcs/Repository/Cache.hs 146 >>> copyFileUsingCache oos (Ca cache) subdir f = >>> do debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir >>> subdir)++"/"++f >>> Just stickItHere <- cacheLoc cache >>> - createDirectoryIfMissing False (reverse $ dropWhile (/='/') $ >>> reverse stickItHere) >>> + createBucket stickItHere >>> sfuc cache stickItHere >>> `catchall` return () >>> where cacheLoc [] = return Nothing >> >> I'm willing to believe that this change does the right thing, but the >> copyFileUsingCache function is hermetic poetry to me. >> >>> hunk ./src/Darcs/Repository/Cache.hs 168 >>> >>> data FromWhere = LocalOnly | Anywhere deriving ( Eq ) >>> >>> +createBucket :: String -> IO () >>> +createBucket s = createDirectoryIfMissing True (takeDirectory s) >>> + >> >> Why do you change the first argument of createDirectoryIfMissing to True? >> >>> hunk ./src/Darcs/Repository/Cache.hs 206 >>> return (fn c, x) >>> `catchall` do (fname,x) <- ffuc cs >>> do createCache c subdir >>> + createBucket (fn c) >>> createLink fname (fn c) >>> return (fn c, x) >>> `catchall` >> >> Why is this necessary? >> >>> hunk ./src/Darcs/Repository/Cache.hs 247 >>> where hash = cacheHash ps >>> wfuc (c:cs) | not $ writable c = wfuc cs >>> | otherwise = do createCache c subdir >>> + createBucket (fn c) >>> write compr (fn c) ps -- FIXME: >>> create links in caches >>> return hash >>> wfuc [] = debugFail $ "No location to write file `" ++ >>> (hashedDir subdir) ++"/"++hash ++ "'" >> >> And this? >> >>> hunk ./src/Darcs/Repository/Cache.hs 252 >>> fn c = hashedFilePath c subdir hash >>> + >> >> This looks like a spurious whitespace change. They may cause unnecessary >> conflicts later on, hence they are not so popular. >> >> Bye, >> Reinier >> >> >> >> >> >> > _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
