fix annex.largefiles largerthan/smallerthan bug

Fix bug in handling of annex.largefiles that use largerthan/smallerthan.
When adding a modified file, it incorrectly used the file size of the old
version of the file, not the current size.

That was the only largefiles limit that didn't directly look at the file on
disk already. Added a new type to keep straight the two different ways such
a limit can be matched. I kind of wanted to extend MatchingFile or FileInfo
to indicate that the matcher is supposed to operate on files from disk or
annex, but it turned out to be too complex to implement it that way.

This also changes the LimitAnnexFiles case when lookupFileKey does not find
a key. It used to fall back to statting the file, now it always returns
False. I doubt the old code could really get to that point, but if it
somehow does, it's better for preferred content matching to be consistent.

Annex/FileMatcher.hs

@@ -113,14 +113,14 @@ tokenizeMatcher = filter (not . null) . concatMap splitparens . words
   where
 	splitparens = segmentDelim (`elem` "()")
 
-commonKeylessTokens :: [ParseToken (MatchFiles Annex)]
-commonKeylessTokens =
+commonKeylessTokens :: LimitBy -> [ParseToken (MatchFiles Annex)]
+commonKeylessTokens lb =
 	[ SimpleToken "anything" (simply limitAnything)
 	, SimpleToken "nothing" (simply limitNothing)
 	, ValueToken "include" (usev limitInclude)
 	, ValueToken "exclude" (usev limitExclude)
-	, ValueToken "largerthan" (usev $ limitSize (>))
-	, ValueToken "smallerthan" (usev $ limitSize (<))
+	, ValueToken "largerthan" (usev $ limitSize lb (>))
+	, ValueToken "smallerthan" (usev $ limitSize lb (<))
 	]
 
 commonKeyedTokens :: [ParseToken (MatchFiles Annex)]
@@ -147,7 +147,7 @@ preferredContentKeylessTokens pcd =
 	[ SimpleToken "standard" (call $ matchStandard pcd)
 	, SimpleToken "groupwanted" (call $ matchGroupWanted pcd)
 	, SimpleToken "inpreferreddir" (simply $ limitInDir preferreddir)
-	] ++ commonKeylessTokens
+	] ++ commonKeylessTokens LimitAnnexFiles
   where
 	preferreddir = fromMaybe "public" $
 		M.lookup "preferreddir" =<< (`M.lookup` configMap pcd) =<< repoUUID pcd
@@ -182,7 +182,9 @@ mkLargeFilesParser = do
 	let mimer n = ValueToken n $ 
 		const $ Left $ "\""++n++"\" not supported; not built with MagicMime support"
 #endif
-	let parse = parseToken $ commonKeyedTokens ++ commonKeylessTokens ++
+	let parse = parseToken $
+		commonKeyedTokens ++
+		commonKeylessTokens LimitDiskFiles ++
 #ifdef WITH_MAGICMIME
 		[ mimer "mimetype" $
 			matchMagic "mimetype" getMagicMimeType providedMimeType

CHANGELOG

@@ -11,6 +11,9 @@ git-annex (7.20190913) UNRELEASED; urgency=medium
   * enable-tor: Run kdesu with -c option.
   * enable-tor: Use pkexec to run command as root when gksu and kdesu are not
     available.
+  * Fix bug in handling of annex.largefiles that use largerthan/smallerthan.
+    When adding a modified file, it incorrectly used the file size of the
+    old version of the file, not the current size.
 
  -- Joey Hess <id@joeyh.name>  Thu, 19 Sep 2019 11:11:19 -0400
 

Limit.hs

@@ -38,6 +38,10 @@ import Data.Time.Clock.POSIX
 import qualified Data.Set as S
 import qualified Data.Map as M
 
+{- Some limits can look at the current status of files on
+ - disk, or in the annex. This allows controlling which happens. -}
+data LimitBy = LimitDiskFiles | LimitAnnexFiles
+
 {- Checks if there are user-specified limits. -}
 limited :: Annex Bool
 limited = (not . Utility.Matcher.isEmpty) <$> getMatcher'
@@ -302,26 +306,28 @@ limitSecureHash _ = checkKey $ pure . cryptographicallySecure . keyVariety
 
 {- Adds a limit to skip files that are too large or too small -}
 addLargerThan :: String -> Annex ()
-addLargerThan = addLimit . limitSize (>)
+addLargerThan = addLimit . limitSize LimitAnnexFiles (>)
 
 addSmallerThan :: String -> Annex ()
-addSmallerThan = addLimit . limitSize (<)
+addSmallerThan = addLimit . limitSize LimitAnnexFiles (<)
 
-limitSize :: (Maybe Integer -> Maybe Integer -> Bool) -> MkLimit Annex
-limitSize vs s = case readSize dataUnits s of
+limitSize :: LimitBy -> (Maybe Integer -> Maybe Integer -> Bool) -> MkLimit Annex
+limitSize lb vs s = case readSize dataUnits s of
 	Nothing -> Left "bad size"
 	Just sz -> Right $ go sz
   where
-	go sz _ (MatchingFile fi) = lookupFileKey fi >>= check fi sz
+	go sz _ (MatchingFile fi) = case lb of
+		LimitAnnexFiles -> lookupFileKey fi >>= \case
+			Just key -> checkkey sz key
+			Nothing -> return False
+		LimitDiskFiles -> do
+			filesize <- liftIO $ catchMaybeIO $ getFileSize (currFile fi)
+			return $ filesize `vs` Just sz
 	go sz _ (MatchingKey key _) = checkkey sz key
 	go sz _ (MatchingInfo p) =
 		getInfo (providedFileSize p) 
 			>>= \sz' -> return (Just sz' `vs` Just sz)
 	checkkey sz key = return $ keySize key `vs` Just sz
-	check _ sz (Just key) = checkkey sz key
-	check fi sz Nothing = do
-		filesize <- liftIO $ catchMaybeIO $ getFileSize (currFile fi)
-		return $ filesize `vs` Just sz
 
 addMetaData :: String -> Annex ()
 addMetaData = addLimit . limitMetaData

doc/bugs/add_of_modified_file_applies_annex.largefiles_to_size_of_old_key.mdwn

@@ -39,3 +39,5 @@ or by --largerthan.
 
 But, for matching largefiles, it needs to look at the actual file on disk,
 not an old key.
+
+> [[fixed|done]] --[[Joey]]