gh <[email protected]> added the comment: 2010/11/9 Florent Becker <[email protected]>: > > Florent Becker <[email protected]> added the comment: > >> The good thing is that repositories with that _darcs/pristine.none >> file keep on being recognized as NoPristine repositories, so from the >> retro-compatibility point of view, I think this change is OK. > > Can you add a test that we keep this file when it exists?
I have tried writing such a test but when you think about the possible cases you get: * either test that "darcs repair" leaves pristine.none in a hashed repository, which is a useless test since hashed repos always have a hashed prsitine. * either test that "darcs repair" leaves pristine.none in an OF repository. I tried to do that but I realized that "darcs repair" never works in an OF repository without the _darcs/pristine/ directory, before or after this bundle. So "darcs repair" just fails in that case. In short, the fix for issue1977 in this bundle does not fix the case of OF repositories, but only for hashed repositories. OTOH it does not degrade the behaviour of darcs for the OF case. Since by looking at the code it is clear that no pristine.none or current.none file is removed, I suggest we can do without such a test, what do you think? >> data Pristine >> - = NoPristine !String >> + = NoPristine >> | PlainPristine !String >> | HashedPristine > > What would the String parameter represent before? It was either "pristine.none" or "current.none", according to the function identifyPristine before this bundle. It was also "aack?" when the function Darcs.Repository.Internal.maybeIdentifyRepository needed to return a dummy object of the type IdentifyRepo when successfully identifying a remote repository. > Ok, any reason not to use the NoPristine constructor? Now that it has > no argument, there is no reason to keep the nopristine constant around. Indeed, I'll send a patch removing this function. > hunk ./src/Darcs/Repository/Pristine.hs 116 >> bug "3 HashedPristine is not implemented yet." >> >> createPristineFromWorking :: Pristine -> IO () >> -createPristineFromWorking (NoPristine _) = return () >> +createPristineFromWorking NoPristine = return () >> createPristineFromWorking (PlainPristine n) = cloneTreeExcept > [darcsdir] "." n >> createPristineFromWorking HashedPristine = >> bug "HashedPristine is not implemented yet." > > ok (isn't the bug "HashedPristine is not implemented yet." a lie?) > Yes the bug strings are wrong: these cases can never be reacher in the current state of the code. Writing a patch for that. What a plate of spaghetti. >> easyCreatePartialsPristineDirectoryTree :: FilePathLike fp => [fp] -> > Pristine >> -> FilePath -> IO Bool >> -easyCreatePartialsPristineDirectoryTree _ (NoPristine _) _ = return > False >> +easyCreatePartialsPristineDirectoryTree _ NoPristine _ = return False >> easyCreatePartialsPristineDirectoryTree prefs (PlainPristine n) p >> = clonePartialsTree n p (map toFilePath prefs) >> return True >> easyCreatePartialsPristineDirectoryTree _ HashedPristine _ = > > A haddock for easy… would be welcome. OK I'll try. Guillaume __________________________________ Darcs bug tracker <[email protected]> <http://bugs.darcs.net/patch429> __________________________________ _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
