init: fix data loss bug

Fix bug that lost modifications to unlocked files when init is re-ran in an
already initialized repo.

In retrospect needing scanUnlockedFiles False in the direct mode upgrade
path was a good hint that it was unsafe when used with True.

However, this bug did not affect upgrade from v5. In such an upgrade, an
unlocked file that is modified is left as-is. The only place
scanUnlockedFiles True did overwrite modified unlocked files is during an
git-annex init of a repo that was already initialized by git-annex.

(I also tried a scenario where the repo had not been initialized by
git-annex yet, but was cloned from a v7 repo with an unlocked file, and the
pointer file replaced with some other content, and the data loss did not
occur in that situation.)

Since the fixed scanUnlockedFiles avoids overwriting non-pointer files,
it should be safe to run in any situation, so there's no need any longer
for the parameter.

diff --git a/Annex/Init.hs b/Annex/Init.hs
index e590620cb51fcd415df7a8678855faa6b1d7cf0c..1bc82710f4d8cad5029078f399bb200f08ebb55c 100644 (file)
--- a/Annex/Init.hs
+++ b/Annex/Init.hs
@@ -109,7 +109,7 @@ initialize' mversion = checkCanInitialize  $ do
                setVersion (fromMaybe defaultVersion mversion)
        configureSmudgeFilter
        showSideAction "scanning for unlocked files"
-       scanUnlockedFiles True
+       scanUnlockedFiles
        unlessM isBareRepo $ do
                hookWrite postCheckoutHook
                hookWrite postMergeHook

diff --git a/Annex/WorkTree.hs b/Annex/WorkTree.hs
index eb64e0cbb17a6711b2ca45a291008a9f48287d4c..cf40f527bf0ee544ccadb0492dc183d729182f15 100644 (file)
--- a/Annex/WorkTree.hs
+++ b/Annex/WorkTree.hs
@@ -70,11 +70,12 @@ ifAnnexed file yes no = maybe no yes =<< lookupFile file
  - when initializing/upgrading a v6+ mode repository.
  -
  - Also, the content for the unlocked file may already be present as
- - an annex object. If so, make the unlocked file use that content
- - when replacefiles is True.
+ - an annex object. If so, populate the pointer file with it.
+ - But if worktree file does not have a pointer file's content, it is left
+ - as-is.
  -}
-scanUnlockedFiles :: Bool -> Annex ()
-scanUnlockedFiles replacefiles = whenM (inRepo Git.Ref.headExists) $ do
+scanUnlockedFiles :: Annex ()
+scanUnlockedFiles = whenM (inRepo Git.Ref.headExists) $ do
        Database.Keys.runWriter $
                liftIO . Database.Keys.SQL.dropAllAssociatedFiles
        (l, cleanup) <- inRepo $ Git.LsTree.lsTree Git.LsTree.LsTreeRecursive Git.Ref.headRef
@@ -92,15 +93,18 @@ scanUnlockedFiles replacefiles = whenM (inRepo Git.Ref.headExists) $ do
                let tf = Git.LsTree.file i
                Database.Keys.runWriter $
                        liftIO . Database.Keys.SQL.addAssociatedFileFast (toIKey k) tf
-               whenM (pure replacefiles <&&> inAnnex k) $ do
+               whenM (inAnnex k) $ do
                        f <- fromRepo $ fromTopFilePath tf
-                       destmode <- liftIO $ catchMaybeIO $ fileMode <$> getFileStatus f
-                       ic <- replaceFile f $ \tmp ->
-                               linkFromAnnex k tmp destmode >>= \case
-                                       LinkAnnexOk -> 
-                                               withTSDelta (liftIO . genInodeCache tmp)
-                                       LinkAnnexNoop -> return Nothing
-                                       LinkAnnexFailed -> liftIO $ do
-                                               writePointerFile tmp k destmode
-                                               return Nothing
-                       maybe noop (restagePointerFile (Restage True) f) ic
+                       liftIO (isPointerFile f) >>= \case
+                               Just k' | k' == k -> do
+                                       destmode <- liftIO $ catchMaybeIO $ fileMode <$> getFileStatus f
+                                       ic <- replaceFile f $ \tmp ->
+                                               linkFromAnnex k tmp destmode >>= \case
+                                                       LinkAnnexOk -> 
+                                                               withTSDelta (liftIO . genInodeCache tmp)
+                                                       LinkAnnexNoop -> return Nothing
+                                                       LinkAnnexFailed -> liftIO $ do
+                                                               writePointerFile tmp k destmode
+                                                               return Nothing
+                                       maybe noop (restagePointerFile (Restage True) f) ic
+                               _ -> noop

diff --git a/CHANGELOG b/CHANGELOG
index 22c69752e13d4a958b2d72c978c182f501ac7d26..344e2ff2994b8933f95e5517b316b6c1265fe200 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,7 @@
 git-annex (7.20191025) UNRELEASED; urgency=medium
 
+  * init: Fix bug that lost modifications to unlocked files when init is
+    re-ran in an already initialized repo.
   * benchmark: Add --databases to benchmark sqlite databases.
 
  -- Joey Hess <id@joeyh.name>  Tue, 29 Oct 2019 15:13:03 -0400

diff --git a/Upgrade/V5.hs b/Upgrade/V5.hs
index 5c8ed0a1bd9dc7217cb9bcf098f60ab2041d7037..5d331c87871df96c922d277aa93064d7c19e02eb 100644 (file)
--- a/Upgrade/V5.hs
+++ b/Upgrade/V5.hs
@@ -42,13 +42,10 @@ upgrade automatic = flip catchNonAsync (const $ return False) $ do
                ( do
                        checkGitVersionForDirectUpgrade
                        convertDirect
-                       -- Worktree files are already populated, so don't
-                       -- have this try to populate them again.
-                       scanUnlockedFiles False
                , do
                        checkGitVersionForIndirectUpgrade
-                       scanUnlockedFiles True
                )
+       scanUnlockedFiles
        configureSmudgeFilter
        -- Inode sentinal file was only used in direct mode and when
        -- locking down files as they were added. In v6, it's used more