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
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
