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

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

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

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
 

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 ()

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