[darcs-users] darcs patch: resolve issue1131
Dmitry Kurochkin
dmitry.kurochkin at gmail.com
Thu Oct 9 10:45:34 UTC 2008
On Wed, Oct 8, 2008 at 7:55 PM, Eric Kow <kowey at darcs.net> wrote:
> On Wed, Oct 08, 2008 at 15:55:36 +0400, Dmitry Kurochkin wrote:
>> I am not able to reproduce the issue. So it is not tested. But it is
>> pretty simple so I do not think it introduces any new bugs (and
>> hopefully fixes one :)).
>
> Secondary review. I think I'm happy to have this in stable.
>
> Resolve issue1131: accept download requests for different files.
> ----------------------------------------------------------------
>> +data UrlState = UrlState { inProgress :: Map String ( FilePath
>> + , [FilePath]
>> + , Cachable
>> + , (MVar String) )
>
> Before we mapped from a URL to the FilePath it should be saved as when
> downloaded. Now we map to that and a list of possible FilePaths in
> case we ask for it with different local names. (We still don't know
> why it would do this yet). In principle, I suppose we could just
> replace FilePath, [FilePath] by [FilePath]
I wanted to put it into a single list first. But in this case we have to:
1. original file is first, append extra files to tail.
2. original file is last, append extra files to head.
I did not want to append from tail or keep original file last. So I
decided to store original file separately, afterall it is treated
specially.
>
>> hunk ./src/URL.hs 150
>> case Map.lookup u p of
>
> Are we already in the process of or planning to download this file?
> If not, stick it in the queuue (put new_nst), otherwise...
>
> BEFORE
>> - Just (f', c', _) ->
>> - if f == f' && c == c'
>> - then if u `elemQ` w && priority r == High
>> - then do put $ st { waitToStart = pushQ u (deleteQ u w) }
>> dbg $ "Moving "++u++" to head of download queue."
>> - else dbg "Ignoring UrlRequest of file that's already queued."
>> - else bug $ "URL.urlThread: same URLs with different parameters "++u++" "++f++ show (f,f',c,c')
>
> AFTER
>> + Just (f', fs', c', v) ->
>> + if c /= c'
>> + then bug $ "URL.urlThread: same URLs with different parameters "++u++" "++show (c,c')
>> + else do when (u `elemQ` w && priority r == High) $ do
>> + modify (\s -> s { waitToStart = pushQ u (deleteQ u w) })
>> dbg $ "Moving "++u++" to head of download queue."
>> + if f `notElem` (f':fs')
>> + then let new_p = Map.insert u (f', f:fs', c', v) p
>> + in do modify (\s -> s { inProgress = new_p })
>> + dbg "Adding new file to existing UrlRequest."
>> + else dbg "Ignoring UrlRequest of file that's already queued."
>
> Are you happy with this summary, Dmitry?
>
> 'muzak' here is my shorthand for 'print the ignoring urlrequest message'
>
> -------------------------------------------+--------+--------------------
> | BEFORE | AFTER
> ---------+--------------+------------------+--------+--------------------
> filename | cacheability | waiting+priority | hurry | hurry; muzak
> filename | cacheability | | muzak | muzak
> filename | | waiting+priority | bug! | bug!
> filename | | | bug! | bug!
> ---------+--------------+------------------+--------+--------------------
> | cacheability | waiting+priority | bug! | hurry; extra name
> | cacheability | | bug! | extra name
> | | waiting+priority | bug! | bug!
> | | | bug! | bug!
> ---------+--------------+------------------+--------+--------------------
> | | waiting+priority | bug! | bug!
> ---------+--------------+------------------+--------+--------------------
I am not sure what the last row is for - it is the same as two row
above. Otherwise this looks correct.
Thanks for the review!
Regards,
Dmitry
>
>> hunk ./src/URL.hs 224
>> + mapM_ (copyFile f) fs
>
> When we download a file, make sure that it gets copied to all its new
> names.
>
> Pitfall check
> -------------
>> - put $ st { waitToStart = pushQ u (deleteQ u w) }
>> + modify (\s -> s { waitToStart = pushQ u (deleteQ u w) })
>
> * Possibility that the current state is not the same st?
> Safe - verified that there are no intervening puts or modifies
> * Interaction between f' and fs
> Safe - we always check f' along with fs
> * Copying over the file we just downloaded
> Safe - f' doesn't go in fs (unless there is some kind of crazy link)
>
> --
> Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
> PGP Key ID: 08AC04F9
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFI7NfxBUrOwgisBPkRAiyXAKCTwghNdy6plakB/52lfJiAvFzbowCeM249
> qNWdNqI7ZzMhtpzTQ9GYmBw=
> =Gjdj
> -----END PGP SIGNATURE-----
>
>
More information about the darcs-users
mailing list