From 3066bdb1fb60e80f40b5badc150fb6eb51a922bb Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 30 Sep 2019 17:12:47 -0400 Subject: [PATCH] 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 | 14 ++++++----- CHANGELOG | 3 +++ Limit.hs | 24 ++++++++++++------- ...s_annex.largefiles_to_size_of_old_key.mdwn | 2 ++ 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/Annex/FileMatcher.hs b/Annex/FileMatcher.hs index 6358049309..b41a4a421f 100644 --- a/Annex/FileMatcher.hs +++ b/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 diff --git a/CHANGELOG b/CHANGELOG index 2e69a73b01..1fae747ccc 100644 --- a/CHANGELOG +++ b/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 Thu, 19 Sep 2019 11:11:19 -0400 diff --git a/Limit.hs b/Limit.hs index cc381a021a..5ac0fe636a 100644 --- a/Limit.hs +++ b/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 diff --git a/doc/bugs/add_of_modified_file_applies_annex.largefiles_to_size_of_old_key.mdwn b/doc/bugs/add_of_modified_file_applies_annex.largefiles_to_size_of_old_key.mdwn index 1ff69454a9..be03a8a5ea 100644 --- a/doc/bugs/add_of_modified_file_applies_annex.largefiles_to_size_of_old_key.mdwn +++ b/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]]