[darcs-users] [darcs-devel] [patch236] In replacePristine, cope also with Trees... (and 2 more) [status=accepted-pending-tests]

Eric Kow kowey at darcs.net
Fri May 7 11:58:03 UTC 2010


On Thu, May 06, 2010 at 15:27:10 +0000, Petr Ročkai wrote:
> this resolves the immediate problem reported as issue1837. A regression test
> would be welcome but I don't have time to work on that. The first two patches
> are required, and can be reasonably cherry-picked for branch-2.4 as well.

I confirm this applies on branch-2.4 (needs a refactor patch from David M which
seems harmless enough).

I'm not going to push this just yet... some things to figure out with the tests.

> <mornfall> kowey: (I have fixed the --partial bug, *but* the whole way it works
>          is pretty dangerous...)
> <mornfall> kowey: I.e. if *something* writes into the working copy while the
>          get is running, it will corrupt pristine, hashed or not (since *this*
>          variant of get will work in working and then copy the result over to
>          pristine, instead of the other way around)
> <mornfall> Most get variants work the other way around (pristine first), which
>          is safe.
> <mornfall> (Bloody mess.)

This is a comment on the pre-existing setup, not Petr's changes

We talked about it a bit.  When fetching from old-fashioned
repositories, we apply patches to the working dir (checkpoint or
otherwise) and then copy from working to pristine.  [In the darcs 1
client, things were even worse; we'd just copying the pristine, I think,
which hid a lot of errors that only resurfaced when darcs 2 client
became more thorough].  This ought to change for the future.

> Thu May  6 17:15:24 CEST 2010  Petr Rockai <me at mornfall.net>
>   * In replacePristine, cope also with Trees that have no hashes in them.
> 
> Thu May  6 17:19:43 CEST 2010  Petr Rockai <me at mornfall.net>
>   * Resolve issue1837: Add readWorking and use it in pristineFromWorking.
> 
> Thu May  6 17:20:35 CEST 2010  Petr Rockai <me at mornfall.net>
>   * Also use readWorking in setScriptsExecutable (minor refactor).


In replacePristine, cope also with Trees that have no hashes in them.
---------------------------------------------------------------------
> hunk ./src/Darcs/Repository.hs 337
>      where replace HashedPristine =
>                do let t = darcsdir </> "hashed_inventory"
>                   i <- gzReadFilePS t
> -                 root <- writeDarcsHashed tree $ darcsdir </> "pristine.hashed"
> +                 tree' <- darcsAddMissingHashes tree
> +                 root <- writeDarcsHashed tree' $ darcsdir </> "pristine.hashed"

I'm guessing this computes the hash for all files in the tree we don't
yet know the hash for.

Worth haddocking?

  darcsAddMissingHashes :: (Monad m, Functor m) => Tree m -> m (Tree m)
  darcsAddMissingHashes = updateTree update
      where update (SubTree t) = return . SubTree $ t { treeHash = darcsTreeHash t }
            update (File blob@(Blob con NoHash)) =
                do hash <- sha256 <$> readBlob blob
                   return $ File (Blob con hash)
            update (Stub _ NoHash) = fail "NoHash Stub encountered in darcsAddMissingHashes"
            update x = return x

I gather this is necessary because we want to account for the tree coming from
an unhashed source (ie. working).

It doesn't sound like it could hurt in any case.  Could there be some sort of
performance issue involved?

Resolve issue1837: Add readWorking and use it in pristineFromWorking.
---------------------------------------------------------------------
> hunk ./src/Darcs/Repository.hs 353
>  pristineFromWorking :: RepoPatch p => Repository p C(r u t) -> IO ()
>  pristineFromWorking repo@(Repo dir _ rf _)
>      | formatHas HashedInventory rf =
> -        withCurrentDirectory dir $ readUnrecorded repo >>= replacePristine repo
> +        withCurrentDirectory dir $ readWorking >>= replacePristine repo

This would normally alarm me, but I guess it's in a function called
pristineFromWorking (the idea of which is a bit yuck-inducing)
  
> +-- | Obtains a Tree corresponding to the working copy of the
> +-- repository. NB. Almost always, using readUnrecorded is the right
> +-- choice. This function is only useful in not-completely-constructed
> +-- repositories.
> +readWorking :: IO (Tree IO)
> +readWorking = expand =<< (nodarcs `fmap` readPlainTree ".")
> +  where nodarcs = Tree.filter (\(AnchoredPath (Name x:_)) _ -> x /= BSC.pack "_darcs")

Yay haddock. Seems fine, maybe I'm missing something.
We could use the new darcsdir.

Also use readWorking in setScriptsExecutable (minor refactor).
--------------------------------------------------------------
> -    let nodarcs = Tree.filter (\(AnchoredPath (Name x:_)) _ -> x /= BC.pack "_darcs")
> -    tree <- expand =<< (nodarcs `fmap` readPlainTree ".")
> +    tree <- readWorking

Yeah

-- 
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: 198 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20100507/5a4f8967/attachment.pgp>


More information about the darcs-users mailing list