[darcs-users] [patch257] Resolve issue 1503

Adolfo Builes builes.adolfo at googlemail.com
Thu Jun 3 06:26:46 UTC 2010


I have fixed some things now, and it looks better

------------------------------
>
> -------------------------
> > +    pullingFrom <- mapM (fixUrl opts) repos
> > +    withRepoLock opts $- \initialRepository ->
> > +      modifyCache initialRepository pullingFrom  >>= \repository ->
> > +      fetchPatches opts' repos "pull" repository >>= applyPatches opts'
> repository
>
> OK, we still have the two repositories here but at least it's clearer
> which one you should be using at all times.
>
> You could get rid of the extra argument by reversing the definition of
> modifyCache so it lends itself more to partial application.
>

I tried before with partial application, but  I was getting each time
"Inferred type is less polymorphic than expected Quantified type variable
`p' escapes", I read a workaround for it in a Oleg's paper, but that will
mean creating a newtype and ... , I think is not work for this case, I used
the lambda instead, I came back to use the do notation, I tried to do it
with binding but the compiler was complaining about a type which was
infering incorrectly, using do notation did the trick.


> +-- | Add locals repositories to cache, ensuring that locals are tried
> before any remote one.
> > +modifyCache ::  forall p C(r u). RepoPatch p => Repository p C(r u r) ->
> > +                        [String] -> IO (Repository p C(r u r))
> > +modifyCache (Repo dir opts rf(DarcsRepository pristine (Ca ccache)))
> repos =
>
> I think modifyCache would be more useful as a higher order function
> (otherwise, I would be not so inclined to have it in the
> Darcs.Repository hierarchy).
>
> Rather than being specifically about adding the local repositories to
> the cache, it could just be about applying some (Cache -> Cache) on the
> repository's cache.
>
>
I modify "modifyCache" now it is located in Repository.InternalTypes, there
is a similar function there which acts over ther cache, so that's why I
decided to put there, also, mornfal noticed :

*18:23 mornfall: InternalTypes is a misnomer already -- but yes, it will do
> for now.*
>

So, I will write these down for a future refactor.

> +  do
> > +    let cache = Ca (addLocals ccache repos)
> > +    return (Repo dir opts rf (DarcsRepository pristine cache))
> > +    where
> > +      addLocals = foldr (\a b -> if isFile a
> > +                                 then Cache DarcsCache.Repo NotWritable
> a : b
> > +                                 else b)
>

Now the addLocal method looks is clearer, also I moved back to Pull.hs, and
makes use of the function "modifyCache", it looks like :

addLocal repo repos = modifyCache repo $ \ (Ca cache) ->
                                                  Ca $ [Cache
DarcsCache.Repo NotWritable r | r <- repos, isFile r ] ++ cache


> +compareByLocality (Cache _ _  x) (Cache _ _ y)
> > +  | isFile x && (isUrl y || isSsh y)  = LT
> > +  | (isUrl x || isSsh x) && isFile y = GT
> > +  | otherwise = EQ
>
> Every top level function should have a type signature.
>
>
Sorry, I didn't added it :).. now it has its signature


>  copyFileUsingCache :: OrOnlySpeculate -> Cache -> HashedDir -> String ->
> IO ()
> > -copyFileUsingCache oos (Ca cache) subdir f =
> > -    do debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir
> subdir)++"/"++f
> > +copyFileUsingCache oos (Ca unsortedCache) subdir f =
> > +    do let (Ca cache) = Ca (sortBy compareByLocality unsortedCache)
> > +       debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir
> subdir)++"/"++f
>
> Interesting.  One question: does this sorting belong here?  What if in
> the future we want to sort by proximity to the repository directory?
> How would we manage that?  What's the right attitude to adopt here?  One
> possible answer, for example, is YAGNI.  Is that the right one?
>
> I have lifted all that sorting from there, now it is done when it is as it
is populated ( thanks Petr :))


>  fetchFileUsingCachePrivate :: FromWhere -> Cache -> HashedDir -> String
> -> IO (String, B.ByteString)
> > -fetchFileUsingCachePrivate fromWhere (Ca cache) subdir f =
> > +fetchFileUsingCachePrivate fromWhere (Ca unsortedCache) subdir f =
> >      do when (fromWhere == Anywhere) $ copyFileUsingCache ActuallyCopy
> (Ca cache) subdir f
> >         ffuc cache
> >      `catchall` debugFail ("Couldn't fetch `"++f++"'\nin subdir
> "++(hashedDir subdir)++
> > hunk ./src/Darcs/Repository/Cache.
>>
>> hs 265
>> >
>> >            ffuc [] = debugFail $ "No sources from which to fetch file
>> `"++f++"'\n"++ show (Ca cache)
>> >
>> > +          Ca cache = Ca (sortBy compareByLocality unsortedCache)
>>
>> What's the difference between these two functions?
>
>
 I don't have a clear answer for that question, I was writing something but
then I realize it was a supposition I was doing, I hope to have a proper
answer by the weekend :).

--
Adolfo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20100603/10bb3077/attachment.html>


More information about the darcs-users mailing list