[darcs-users] darcs patch: Refactor copyInventory to not use identi... (and 1 more)

[Eric Kow]

On Tue, Sep 16, 2008 at 14:12:16 -0700, Dmitry Kurochkin wrote:
> This is my attempt to  fixissue1039. The patches introduce a 'repo-id'
> parameter that is created during init and stored in format file. During
> get source repo id is copied to the local repo. During push/pull/send
> we compare repo ids and ask for confirmation if they do not match.

Cool! I've had a quick look.  I think we should wait until Dmitry
comments further on doing this with repoformat or not.

> - older darcs versions are not able to get repositories with 'repo-id'.
>   Perhaps repo-id should be stored not in the format file.

I agree.  I would rather this be stored somewhere else like
_darcs/repo-id and this file (like format) have the right not to exist.

> - tests do not pass. I suppose it hangs on prompt input.
>   How is this handled in other places?

Ah... so why is it hanging now, because it thinks repositories are
unrelated when they really are.  We could introduce a flag to make darcs
ignore repository ancestry, or just audit the test suite and answer yes
in the right places.


Refactor copyInventory to not use identifyRepositoryFor.
--------------------------------------------------------
This patch threads a new 'repo' (torepository?) argument through
from copyRepository to copyInventory.

> -copyRepository fromrepository@(Repo _ opts rf _)
> +copyRepository repo fromrepository@(Repo _ opts rf _)

I wonder if we should flip these arguments, fromrepository torepository.

> -copyInventory :: forall p C(r u t). RepoPatch p => Repository p C(r u t) -> IO ()
> -copyInventory fromrepo@(Repo fromdir opts rf (DarcsRepository _ cremote)) = do
> -  repo@(Repo todir xx rf2 (DarcsRepository yy c)) <- identifyRepositoryFor fromrepo "."
> +copyInventory :: forall p C(r u t). RepoPatch p => Repository p C(r u t) -> Repository p C(r u t) -> IO ()
> +copyInventory repo@(Repo todir xx rf2 (DarcsRepository yy c)) fromrepo@(Repo fromdir opts rf (DarcsRepository _ cremote)) = do

Here is the main change.  copyInventory now accepts the 'to' repository
as an argument instead of trying to identifyRepositoryFor "."

Resolve issue1039: detect seemingly unrelated repositories.
-----------------------------------------------------------
As above, I think we should not be doing this in the repository format.
The format exists to prevent older darcs from trying to read from and
write to a repository format it does not understand.  In this case,
older darcs should understand the repositories just fine, repo-id or no
repo-id.  Repo-id checking should just be a bonus.

By the way, it would be nice if there was some way to generate a
repository id in a pre-existing repository.  Maybe it could be really
simple, like telling people to echo some unique string to
_darcs/repo-id.

Furthermore, it *might* be nice if when a repository does not have an
id, and it tries to pull from one that does and the user proceeds, to
set the id of the repo we are pulling to (maybe with yet another
prompt... which might annoy everybody that's doing darcs-all pull in the
GHC repo)

> +copyRepoId :: RepoFormat -> RepoFormat -> IO ()
> +copyRepoId (RF ks) rf =
> +  writeRepoFormat (RF $ removeRepoId ks ++ repoid) df
> +    where removeRepoId = delete [] . map (filter (not.(isPrefixOf tok).unpackPS))
> +          tok = unpackPS $ rp2ps (RepoId "")
> +          repoid = case get_repoid rf of
> +                     Just s -> [[rp2ps (RepoId s)]]
> +                     Nothing -> []

I'm having trouble understanding this code, but I suspect it's moot
if we do not use the repo format for this.

> +get_repoid :: RepoFormat -> Maybe String
> +get_repoid (RF ks) = rid $ filter (isPrefixOf tok) (map unpackPS (concat ks))
> +    where tok = unpackPS $ rp2ps (RepoId "")
> +          rid []    = Nothing
> +          rid (a:_) = Just $ drop 8 a

The 8 here corresponds to "repo-id ", so maybe it should be explicit
just to be safe?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20080917/5788346c/attachment.pgp