[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


Hi,

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 
is:

$ 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.

Ganesh


More information about the darcs-users mailing list