darcs

Patch 153 In check, compare index path list against working, not...

Title In check, compare index path list against working, not...
Superseder Nosy List darcs-users, kowey, mornfall
Related Issues
Status accepted Assigned To
Milestone

Created on 2010-02-03.11:03:20 by mornfall, last changed 2011-05-10.20:07:00 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
in-check_-compare-index-path-list-against-working_-not-pristine_.dpatch mornfall, 2010-02-03.11:03:19 text/x-darcs-patch
unnamed mornfall, 2010-02-03.11:03:19 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg9941 (view) Author: mornfall Date: 2010-02-03.11:03:19
Hi,

this, coupled with new hashed-storage (0.4.6) should kick some "darcs check"
issues back into obedience (hash mismatches, "missing items in index").

Yours,
   Petr.

1 patch for repository darcs-unstable@darcs.net:darcs:

Wed Feb  3 11:27:17 CET 2010  Petr Rockai <me@mornfall.net>
  * In check, compare index path list against working, not pristine.
Attachments
msg9945 (view) Author: kowey Date: 2010-02-03.17:41:49
On Wed, Feb 03, 2010 at 11:03:20 +0000, Petr Ročkai wrote:
> this, coupled with new hashed-storage (0.4.6) should kick some "darcs check"
> issues back into obedience (hash mismatches, "missing items in index").

I was confused by this at first because I thought that darcs
check/repair was only something that compared pristine (cache) against
the patch set (canonical).

Petr reminded me over IRC that with the introduction of an index, we've
added a new behaviour to darcs check/repair which is to ALSO compare the
index (cache) against the working dir (canonical)

This patch seems to make sense in that light, so I'm pushing the patch.

Also, Ganesh has pointed out that checking hashed-storage patches is in a sense
useless unless we also enforce stricter dependencies.  While we both agree that
there's no need to go that far, I think it may make sense to make some effort
at this until we hit some sort of rough sense of maturity, I guess when we've
been using hashed-storage long enough.  I realise it's a bit of a
double-standard wrt other libraries, but extra scrutiny never hurts (except in
opportunity cost terms), and hashed-storage is pretty special-purpose for now...

Wed Feb  3 10:27:52 GMT 2010  Petr Rockai <me@mornfall.net>
  * Remove the empty-dir hack in favour of a proper (and correct) fix.
  
  In an index update, the changes propagate from the leaves (usually files) to
  the root. Directories are only updated in case a child node of them has been
  updated. However, leaf (empty) directories had been neglected and could lead to
  blank index positions.
    hunk ./Storage/Hashed/Index.hs 273
    -       let we_changed = or [ changed x | (_, x) <- inferiors ]
    +       let we_changed = or [ changed x | (_, x) <- inferiors ] || nullleaf
    +           nullleaf = null inferiors && oldhash == nullsha
    +           nullsha = SHA256 (BS.replicate 32 0)

This seems fairly straightforward.  I'm reading this to mean, "we should update
the index entry for this directory if any of its contents changed, or if it was
previously marked as empty"

    hunk ./Storage/Hashed/Index.hs 115
    -emptyDirHash = sha256 BL.empty
    -

    hunk ./Storage/Hashed/Index.hs 367
    -                  when (null $ listImmediate s) $ updateItem i 0 emptyDirHash

I'm not sure what this does, but I'll trust that this was the empty dir hack
and that it's safe to remove.


In check, compare index path list against working, not pristine.
----------------------------------------------------------------
> Petr Rockai <me@mornfall.net>**20100203102717
>  Ignore-this: 4f6fe81c86677964eb15088e4be090f8
> ] hunk ./src/Darcs/Repository/Repair.hs 167
>    working <- expand =<< restrict pristine <$> readPlainTree "."
>    working_hashed <- darcsUpdateHashes working
>    let index_paths = [ p | (p, _) <- list index ]
> -      pristine_paths = [ p | (p, _) <- list pristine ]
> -      index_extra = index_paths \\ pristine_paths
> -      pristine_extra = pristine_paths \\ index_paths
> +      working_paths = [ p | (p, _) <- list working ]
> +      index_extra = index_paths \\ working_paths
> +      working_extra = working_paths \\ index_paths
>        gethashes p (Just i1) (Just i2) = (p, itemHash i1, itemHash i2)
>        gethashes p (Just i1) Nothing   = (p, itemHash i1, NoHash)
>        gethashes p   Nothing (Just i2) = (p,      NoHash, itemHash i2)
> hunk ./src/Darcs/Repository/Repair.hs 183
>                                    | (p, h1, h2) <- mismatches ]
>    unless (quiet || null index_extra) $
>           putStrLn $ "Extra items in index!\n" ++ format index_extra
> -  unless (quiet || null pristine_extra) $
> -         putStrLn $ "Missing items in index!\n" ++ format pristine_extra
> +  unless (quiet || null working_extra) $
> +         putStrLn $ "Missing items in index!\n" ++ format working_extra
>    unless (quiet || null mismatches) $
>           putStrLn $ "Hash mismatch(es)!\n" ++ mismatches_disp
> hunk ./src/Darcs/Repository/Repair.hs 187
> -  return $ null index_extra && null pristine_extra && null mismatches
> +  return $ null index_extra && null working_extra && null mismatches



-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
msg9946 (view) Author: darcswatch Date: 2010-02-03.18:05:42
This patch bundle (with 1 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-6979899f19c68747f35e982900e7e4786e299015
msg14264 (view) Author: darcswatch Date: 2011-05-10.20:07:00
This patch bundle (with 1 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-6979899f19c68747f35e982900e7e4786e299015
History
Date User Action Args
2010-02-03 11:03:20mornfallcreate
2010-02-03 11:04:24darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-6979899f19c68747f35e982900e7e4786e299015
2010-02-03 17:41:50koweysetnosy: + kowey
messages: + msg9945
2010-02-03 18:05:44darcswatchsetstatus: needs-review -> accepted
messages: + msg9946
2011-05-10 20:07:00darcswatchsetmessages: + msg14264