fix problem populating pointer files

This is a result of an audit of every use of getInodeCaches,
to find places that misbehave when the annex object is not in the inode
cache, despite pointer files for the same key being in the inode cache.

Unfortunately, that is the case for objects that were in v7 repos that
upgraded to v8. Added a note about this gotcha to getInodeCaches.

Database.Keys.reconcileStaged, then annex.thin is set, would fail to
populate pointer files in this situation. Changed it to check if the
annex object is unmodified the same way inAnnex does, falling back to a
checksum if the inode cache is not recorded.

Sponsored-by: Dartmouth College's Datalad project

diff --git a/Annex/Content/Presence.hs b/Annex/Content/Presence.hs
index bfcdec694cc77da33b00a291c37362062d66780f..15110213b1831bfd15347b56571561e9620bb1b1 100644 (file)
--- a/Annex/Content/Presence.hs
+++ b/Annex/Content/Presence.hs
@@ -20,12 +20,11 @@ module Annex.Content.Presence (
        contentLockFile,
 ) where
 
+import Annex.Content.Presence.LowLevel
 import Annex.Common
 import qualified Annex
-import Annex.Verify
 import Annex.LockPool
 import qualified Database.Keys
-import Types.Remote
 import Annex.InodeSentinal
 import Utility.InodeCache
 import qualified Utility.RawFilePath as R
@@ -144,23 +143,7 @@ isUnmodified key f =
                Nothing -> return False
 
 isUnmodified' :: Key -> RawFilePath -> InodeCache -> [InodeCache] -> Annex Bool
-isUnmodified' key f fc ic = isUnmodifiedCheap'' fc ic <||> expensivecheck
-  where
-       expensivecheck = ifM (verifyKeyContent RetrievalAllKeysSecure AlwaysVerify UnVerified key f)
-               ( do
-                       -- The file could have been modified while it was
-                       -- being verified. Detect that.
-                       ifM (geti >>= maybe (return False) (compareInodeCaches fc))
-                               ( do
-                                       -- Update the InodeCache to avoid
-                                       -- performing this expensive check again.
-                                       Database.Keys.addInodeCaches key [fc]
-                                       return True
-                               , return False
-                               )
-               , return False
-               )
-       geti = withTSDelta (liftIO . genInodeCache f)
+isUnmodified' = isUnmodifiedLowLevel Database.Keys.addInodeCaches
 
 {- Cheap check if a file contains the unmodified content of the key,
  - only checking the InodeCache of the key.
@@ -174,8 +157,5 @@ isUnmodifiedCheap key f = maybe (return False) (isUnmodifiedCheap' key)
        =<< withTSDelta (liftIO . genInodeCache f)
 
 isUnmodifiedCheap' :: Key -> InodeCache -> Annex Bool
-isUnmodifiedCheap' key fc = isUnmodifiedCheap'' fc
+isUnmodifiedCheap' key fc = isUnmodifiedCheapLowLevel fc
        =<< Database.Keys.getInodeCaches key
-
-isUnmodifiedCheap'' :: InodeCache -> [InodeCache] -> Annex Bool
-isUnmodifiedCheap'' fc ic = anyM (compareInodeCaches fc) ic

diff --git a/Annex/Content/Presence/LowLevel.hs b/Annex/Content/Presence/LowLevel.hs
new file mode 100644 (file)
index 0000000..5d6bb42
--- /dev/null
+++ b/Annex/Content/Presence/LowLevel.hs
@@ -0,0 +1,36 @@
+{- git-annex object content presence, low-level functions
+ -
+ - Copyright 2010-2021 Joey Hess <id@joeyh.name>
+ -
+ - Licensed under the GNU AGPL version 3 or higher.
+ -}
+
+module Annex.Content.Presence.LowLevel where
+
+import Annex.Common
+import Annex.Verify
+import Types.Remote
+import Annex.InodeSentinal
+import Utility.InodeCache
+
+isUnmodifiedLowLevel :: (Key -> [InodeCache] -> Annex ()) -> Key -> RawFilePath -> InodeCache -> [InodeCache] -> Annex Bool
+isUnmodifiedLowLevel addinodecaches key f fc ic = isUnmodifiedCheapLowLevel fc ic <||> expensivecheck
+  where
+       expensivecheck = ifM (verifyKeyContent RetrievalAllKeysSecure AlwaysVerify UnVerified key f)
+               ( do
+                       -- The file could have been modified while it was
+                       -- being verified. Detect that.
+                       ifM (geti >>= maybe (return False) (compareInodeCaches fc))
+                               ( do
+                                       -- Update the InodeCache to avoid
+                                       -- performing this expensive check again.
+                                       addinodecaches key [fc]
+                                       return True
+                               , return False
+                               )
+               , return False
+               )
+       geti = withTSDelta (liftIO . genInodeCache f)
+
+isUnmodifiedCheapLowLevel :: InodeCache -> [InodeCache] -> Annex Bool
+isUnmodifiedCheapLowLevel fc ic = anyM (compareInodeCaches fc) ic

diff --git a/CHANGELOG b/CHANGELOG
index 3facd20fa15f1601339703157dca5c2acacadb6d..62083aa26c0ca264040e25beca0013f9cc156996 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,6 +19,8 @@ git-annex (8.20210715) UNRELEASED; urgency=medium
   * Fix bug that caused some transfers to incorrectly fail with
     "content changed while it was being sent", when the content was not
     changed.
+  * Fix bug that could prevent pointer files from being populated,
+    in a repository that was upgraded from v7.
 
  -- Joey Hess <id@joeyh.name>  Wed, 14 Jul 2021 14:26:36 -0400
 

diff --git a/Database/Keys.hs b/Database/Keys.hs
index 83a3e7a49e5ecc89c19cccc1995849e55a8a0fde..d7bd6195043118fe36f5432e5145f52f46f35f4d 100644 (file)
--- a/Database/Keys.hs
+++ b/Database/Keys.hs
@@ -36,6 +36,7 @@ import Annex.Common hiding (delete)
 import qualified Annex
 import Annex.LockFile
 import Annex.Content.PointerFile
+import Annex.Content.Presence.LowLevel
 import Annex.Link
 import Utility.InodeCache
 import Annex.InodeSentinal
@@ -182,7 +183,14 @@ addInodeCaches :: Key -> [InodeCache] -> Annex ()
 addInodeCaches k is = runWriterIO $ SQL.addInodeCaches k is
 
 {- A key may have multiple InodeCaches; one for the annex object, and one
- - for each pointer file that is a copy of it. -}
+ - for each pointer file that is a copy of it.
+ -
+ - When there are no pointer files, the annex object typically does not
+ - have its InodeCache recorded either, so the list will be empty.
+ -
+ - Note that, in repos upgraded from v7, there may be InodeCaches recorded
+ - for pointer files, but none recorded for the annex object.
+ -}
 getInodeCaches :: Key -> Annex [InodeCache]
 getInodeCaches = runReaderIO . SQL.getInodeCaches
 
@@ -375,12 +383,12 @@ reconcileStaged qh = do
        reconcilepointerfile file key = do
                ics <- liftIO $ SQL.getInodeCaches key (SQL.ReadHandle qh)
                obj <- calcRepo (gitAnnexLocation key)
-               objic <- withTSDelta (liftIO . genInodeCache obj)
-               -- Like inAnnex, check the annex object's inode cache
+               mobjic <- withTSDelta (liftIO . genInodeCache obj)
+               -- Like inAnnex, check the annex object is unmodified
                -- when annex.thin is set.
                keypopulated <- ifM (annexThin <$> Annex.getGitConfig)
-                       ( maybe (pure False) (`elemInodeCaches` ics) objic
-                       , pure (isJust objic)
+                       ( maybe (pure False) (\objic -> isUnmodifiedLowLevel addInodeCaches key obj objic ics) mobjic
+                       , pure (isJust mobjic)
                        )
                p <- fromRepo $ fromTopFilePath file
                filepopulated <- sameInodeCache p ics
@@ -390,7 +398,7 @@ reconcileStaged qh = do
                                        Nothing -> return ()
                                        Just ic -> liftIO $
                                                SQL.addInodeCaches key
-                                                       (catMaybes [Just ic, objic])
+                                                       (catMaybes [Just ic, mobjic])
                                                        (SQL.WriteHandle qh)
                        (False, True) -> depopulatePointerFile key p
                        _ -> return ()

diff --git a/git-annex.cabal b/git-annex.cabal
index 6b5684ea533e70e7838966ac012401da671720c5..94c7e38595e0be04f49dcf35769b094b1b970f18 100644 (file)
--- a/git-annex.cabal
+++ b/git-annex.cabal
@@ -630,6 +630,7 @@ Executable git-annex
     Annex.Concurrent.Utility
     Annex.Content
     Annex.Content.Presence
+    Annex.Content.Presence.LowLevel
     Annex.Content.LowLevel
     Annex.Content.PointerFile
     Annex.CopyFile