[darcs-users] darcs patch: Resolve issue1584: Provide optimize --up... (and 1 more)

Ganesh Sittampalam ganesh at earth.li
Sun Sep 20 15:20:14 UTC 2009


This looks pretty good, I have a few comments which could either be 
address by amend-record or an extra patch as you prefer.

On Thu, 3 Sep 2009, Eric Kow wrote:

> Initially the tests were failing because I had not created a hash for 
> the empty pristine (yay tests!) and also because my (somewhat random) 
> use of the NoUpdateWorking flag was causing darcs to delete the 
> tentative pending patch.
> Fri Sep  4 08:15:00 CEST 2009  Eric Kow <kowey at darcs.net>
>  * Resolve issue1584: Provide optimize --upgrade command.
>  This can be used for an inplace upgrade to the latest hashed format.
>  Right now it only handles old-fashioned => hashed, but there may be
>  future format changes for which this can be reused.

Should print out a message after doing the upgrade. Otherwise all you see 

$ darcs optimize --upgrade
Checking repository in case of corruption
The repository is consistent.

Also missing some punctuation there.

You don't seem to clean out old inventories, though you do clean out old 
patches. Likewise checkpoints.

It fails on --partial repos without giving a useful message. In theory we 
could convert those to lazy repos I guess but that seems like extra effort 
unless there is significant user demand.

> Wed Sep  2 08:00:31 CEST 2009  Eric Kow <kowey at darcs.net>
>  * Test for issue1584, the optimize --upgrade feature.

This test just checks that the upgrade repo ends up in hashed but not 
darcs-2 format and that it still has something that the old repo had.

> +import Darcs.Arguments ( DarcsFlag( UpgradeFormat, UseHashedInventory,

UseHashedInventory is imported because we will need to pass it along to 
the repository building code.

> +-- imports for optimize --upgrade; to be tidied

Do they actually need tidying? I don't see any warnings.

> +The \verb|--upgrade| option for \verb!darcs optimize' performs an inplace
> +upgrade of your repository to the lastest \emph{compatible} format. 
> Right this

This should read "Right now".

> +             case state of
> +               RepositoryConsistent -> do
> +                 putStrLn "The repository is consistent."
> +                 actuallyUpgradeFormat repository
> +               _repoIsBroken ->
> +                 putStrLn "Corruption detected! Please run darcs repair 
> first"
> +

A cunning way of indicating what a catch-all case covers.

> +actuallyUpgradeFormat :: RepoPatch p => Repository p C(r u t) -> IO ()
> +actuallyUpgradeFormat repository = do
> +  -- convert patches/inventory
> +  patches <- read_repo repository
> +  let k = "Hashing patch"
> +  beginTedious k
> +  tediousSize k (lengthRL $ concatRL patches)
> +  let patches' = mapRL_RL (mapRL_RL (progress k)) patches
> +  cache <- getCaches [] "."
> +  let compr = compression [] -- default compression?

Yes, this is default compression. I think you should remove the '?'.

> +  -- convert pristine by applying patches
> +  -- I'm not sure if the best way to convert pristine is to copy it or
> +  -- to apply the patches.  I believe the apply method is more reliable

I tend to agree since it'll remove any pristine corruption.

> +  let patchesToApply = progressFL "Applying patch" $ reverseRL $ 
> concatRL 
> $ patches'
> +  createDirectoryIfMissing False $ darcsdir </> hashedDir 
> HashedPristineDir
> +  writeFile (darcsdir </> hashedDir HashedPristineDir </> sha1PS 
> B.empty)

Empty repos use a SHA1 rather than a SHA256 for no apparent reason. This 
just continues that tradition - which I think darcs-hs will fix.


More information about the darcs-users mailing list