Hey there,

this should fix the cases where applying a bunch of patches including a rename
could lead to bits missing from pristine. The actual fix is in hashed-storage:

Sun Jun  6 16:21:25 CEST 2010  Petr Rockai <me@mornfall.net>
  * Track changed files across directory renames, in TreeMonad.
diff -rN -u -p old-hashed-storage/Storage/Hashed/Monad.hs new-hashed-storage/Storage/Hashed/Monad.hs
--- old-hashed-storage/Storage/Hashed/Monad.hs  2010-06-06 16:39:36.000000000 +0200
+++ new-hashed-storage/Storage/Hashed/Monad.hs  2010-06-06 16:39:36.000000000 +0200
@@ -117,6 +117,12 @@ modifyItem path item = do
                      , changed = (S.union paths (changed st))
                      , changesize = (changesize st + change) }

+renameChanged from to = modify $ \st -> st { changed = rename' $ changed st }
+  where rename' = S.fromList . map renameone . S.toList
+        renameone x | from `isPrefix` x = to `catPaths` relative from x
+                    | otherwise = x
+        relative (AnchoredPath from) (AnchoredPath x) = AnchoredPath $ drop (length from) x
+
 -- | Replace an item with a new version without modifying the content of the
 -- tree. This does not do any change tracking. Ought to be only used from a
 -- 'sync' implementation for a particular storage format. The presumed use-case
@@ -212,3 +218,4 @@ instance (Functor m, Monad m, MonadError
            unless (isNothing item) $ do
                   modifyItem from Nothing
                   modifyItem to item
+                  renameChanged from to

I agree it is a bit crude, but it should do for now. I will eventually refactor
the path handling code in h-s anyway.

3 patches for repository darcs-unstable@darcs.net:darcs:

Sun Jun  6 16:28:58 CEST 2010  Petr Rockai <me@mornfall.net>
  * Accept issue1860: (Un)applying move patches may lead to incomplete pristine.

Thu Jun  3 22:53:30 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1817: --external-merge broken, by bumping h-s dependency to 0.5.1.
  
  This h-s releases fixes a problem in index handling code where reading files
  from a Tree that was created from an index only worked in the same working
  directory in which the index had been opened. This was breaking
  externalResolution which called readBlob on index'd files (working) from a
  temporary directory.

Sun Jun  6 16:29:19 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1860: (Un)applying move patches may lead to incomplete pristine.
  
  Fixed in hashed-storage 0.5.2 -- directory renames in TreeIO have caused
  subsequent (disk) flush-es to be incomplete in some cases.

Attachments

• accept-issue1860_-_un_applying-move-patches-may-lead-to-incomplete-pristine_.dpatch
• unnamed

> Sun Jun  6 16:21:25 CEST 2010  Petr Rockai <me@mornfall.net>
> +renameChanged from to = modify $ \st -> st { changed = rename' $ changed st }
> +  where rename' = S.fromList . map renameone . S.toList
> +        renameone x | from `isPrefix` x = to `catPaths` relative from x
> +                    | otherwise = x
> +        relative (AnchoredPath from) (AnchoredPath x) = AnchoredPath $ drop (length from) x

Just a quick comment here (without having read the patch properly).

Do we have to worry about things like initial/trailing slashes in from
and x?

If I understand correctly AnchoredPath is for paths relative to the tree
root?

>             unless (isNothing item) $ do
>                    modifyItem from Nothing
>                    modifyItem to item
> +                  renameChanged from to

One would have to know more about hashed-storage to see if this makes
sense.
> 
> Sun Jun  6 16:28:58 CEST 2010  Petr Rockai <me@mornfall.net>
>   * Accept issue1860: (Un)applying move patches may lead to incomplete pristine.


Accept issue1860: (Un)applying move patches may lead to incomplete pristine.
----------------------------------------------------------------------------
I'll apply this one straightaway

> hunk ./tests/failing-issue1860-incomplete-pristine.sh 1
> +#!/usr/bin/env bash
> +## Copyright (C) 2010 Petr Rockai

So for some reason, our EXAMPLE.sh file has a request to name the issue
and give a synopsis at the top of the file.  I'm sure Trent had a good
reason for it, but no feelings either way.  The important thing is that
we get the job done.

(and the licensing stuff below helps).

> +. lib                           # Load some portability helpers.
> +rm -rf R S                      # Another script may have left a mess.
> +darcs init      --repo R        # Create our test repos.
> +
> +cd R
> +mkdir tools
> +darcs rec -lam "add tools"
> +mkdir tools/cgi
> +echo bar > tools/cgi/README
> +darcs rec -lam "add cgi"
> +darcs move tools contrib
> +darcs rec -lam "rename tools/ to contrib/"
> +rm -rf contrib/cgi
> +darcs rec -lam "drop cgi"
> +cd ..
> +
> +darcs get R S
> +cd S
> +darcs show pristine
> +darcs unpull --last 2 -a
> +darcs show pristine
> +cd ..

Looks good.  Maybe boiling it down further (anonymising the names,
patches etc) would have been nice, but it doesn't make a difference.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

On Sun, Jun 06, 2010 at 17:02:26 +0100, Eric Kow wrote:
> > Sun Jun  6 16:21:25 CEST 2010  Petr Rockai <me@mornfall.net>
> > +renameChanged from to = modify $ \st -> st { changed = rename' $ changed st }
> > +  where rename' = S.fromList . map renameone . S.toList

There's S.map if you're interested.

> > +        renameone x | from `isPrefix` x = to `catPaths` relative from x
> > +                    | otherwise = x
> > +        relative (AnchoredPath from) (AnchoredPath x) = AnchoredPath $ drop (length from) x

> Do we have to worry about things like initial/trailing slashes in from
> and x?

Also, will this do the right thing if from is something like "foo" and x
is something like "foobar"?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Eric Kow <kowey@darcs.net> writes:

> On Sun, Jun 06, 2010 at 17:02:26 +0100, Eric Kow wrote:
>> > Sun Jun  6 16:21:25 CEST 2010  Petr Rockai <me@mornfall.net>
>> > +renameChanged from to = modify $ \st -> st { changed = rename' $ changed st }
>> > +  where rename' = S.fromList . map renameone . S.toList
>
> There's S.map if you're interested.
>
>> > +        renameone x | from `isPrefix` x = to `catPaths` relative from x
>> > +                    | otherwise = x
>> > +        relative (AnchoredPath from) (AnchoredPath x) = AnchoredPath $ drop (length from) x
>
>> Do we have to worry about things like initial/trailing slashes in from
>> and x?
>
> Also, will this do the right thing if from is something like "foo" and x
> is something like "foobar"?
AnchoredPath is a list of path components, not of characters... all the
cases you mention should be covered quite well (no slash characters are
allowed in AnchoredPath ever and components cannot interfere in this way
either).

Yours,
   Petr.

On Sun, Jun 06, 2010 at 18:37:00 +0000, Petr Ročkai wrote:
> > Also, will this do the right thing if from is something like "foo" and x
> > is something like "foobar"?
> AnchoredPath is a list of path components, not of characters... all the
> cases you mention should be covered quite well (no slash characters are
> allowed in AnchoredPath ever and components cannot interfere in this way
> either).

Nice.  In she goes.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

This patch bundle (with 3 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-68f478f1468f4c1409ac69cf9001d7d16bf69dc4

This patch bundle (with 3 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-68f478f1468f4c1409ac69cf9001d7d16bf69dc4