[darcs-users] [patch72] resolve issue1624 break global cache up into subdirectories

Luca Molteni luca_molteni at fastwebnet.it
Wed Dec 23 11:00:43 UTC 2009


Have you already applied this? I may have found a bug on it.

L.M.



2009/12/9 Luca Molteni <luca_molteni at fastwebnet.it>:
> Fixed those two whitespace things.
>
> Bye,
>
> L.M.
>
>
>
> 2009/11/29 Reinier Lamers <tux_rocker at reinier.de>:
>> Hi all,
>>
>> Op zondag 15 november 2009 17:17 schreef Luca Molteni:
>>> New submission from Luca Molteni <volothamp at gmail.com>:
>>
>> 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 <volothamp at gmail.com>**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 @cachelocation at .
>>>  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
>>
>>
>>
>>
>>
>>
>


More information about the darcs-users mailing list