[darcs-devel] darcs patch: Make read/write_pending polymorphic. (and 5 more)

[David Roundy]

On Sun, Jul 10, 2005 at 04:40:02AM +0200, Juliusz Chroboczek wrote:
> Sun Jul 10 03:25:15 CEST 2005  Juliusz Chroboczek <jch at pps.jussieu.fr>
>   * Make read/write_pending polymorphic.
> 
> Sun Jul 10 03:48:14 CEST 2005  Juliusz Chroboczek <jch at pps.jussieu.fr>
>   * Add ``lax'' argument to applyToGitSlurpy.
>   When lax is true, we apply merger_equivalent to mergers.
> 
> Sun Jul 10 03:52:21 CEST 2005  Juliusz Chroboczek <jch at pps.jussieu.fr>
>   * Import GitRepo from darcs-git.
>   This version has write support and support for reverse-engineering
>   Darcs merges from Git merges.
> 
> Sun Jul 10 04:14:26 CEST 2005  Juliusz Chroboczek <jch at pps.jussieu.fr>
>   * Make sync_repo polymorphic.
> 
> Sun Jul 10 04:15:43 CEST 2005  Juliusz Chroboczek <jch at pps.jussieu.fr>
>   * Make writePatch and updateInventory polymorphic.
> 
> Sun Jul 10 04:38:02 CEST 2005  Juliusz Chroboczek <jch at pps.jussieu.fr>
>   * Make withRepoLock polymorphic.

Great! I just have one issue...

> +sync_repo :: Repository -> IO ()
> +sync_repo (Repo _ _ (DarcsRepository p)) = DarcsRepo.sync_repo p
> +-- this should probably update the Git cache
> +sync_repo (Repo _ _ GitRepository) = return ()
...
> +-- the second result is an opaque token that should be passed to
> +-- updateInventory
> +writePatch :: Repository -> [DarcsFlag] -> Patch -> IO (Patch, String)
> +writePatch (Repo _ _ (DarcsRepository _)) opts patch =
> +    do fp <- DarcsRepo.write_patch opts patch
> +       patch' <- gzReadPatchFileLazily fp
> +       return (patch', fp)
> +writePatch (Repo _ _ GitRepository) _ p =
> +    do cd <- getCurrentDirectory
> +       GitRepo.writePatch cd p
...
> +-- this should be called with signals blocked
> +updateInventory :: Repository -> [(Patch, String)] -> IO ()
> +updateInventory (Repo _ _ (DarcsRepository _)) l =
> +    sequence_ $
> +        map (\(p,_) ->
> +             DarcsRepo.add_to_inventory "." (fromJust (patch2patchinfo p)))
> +            l
> +updateInventory (Repo _ _ GitRepository) l = GitRepo.updateInventory l

I'd prefer for these generic functions not to ignore the "repo" data member
that indicates the repository location itself.  Your functions are safe in
the current code, but will fail if we write code like:

  repository <- identifyRepository "."
  setCurrentDirectory "_darcs"
  sync_repo repository

This isn't really a likely scenario, but best to code defensively.  We know
where the Repository is, so it would be nice to make use of that.  This may
allow us to write simpler client pull/apply/etc code in the future, since
we won't have to so often worry about which directory we're in.

> +withRepoLock :: (Repository -> IO a) -> IO a

I like this one! Takes two common idioms and combine them into one.  :)
-- 
David Roundy
http://www.darcs.net