From ad1d422dd72218e8847e9e6ec01d2048858345fc Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 30 Jan 2019 12:36:30 -0400 Subject: [PATCH] fix false positive in export conflict detection Like the earlier fixed one in Command.Export, it occurred when the same tree was exported by multiple clones. Previous fix was incomplete since several other places looked at the list of exported trees to detect when there was an export conflict. Added a single unified function to avoid missing any places it needed to be fixed. This commit was sponsored by mo on Patreon. --- CHANGELOG | 8 ++++++++ Command/Export.hs | 6 +++--- Command/Sync.hs | 8 ++++---- Database/Export.hs | 2 +- Logs/Export.hs | 34 +++++++++++++++++++++++++++++----- Remote/Helper/Export.hs | 2 +- 6 files changed, 46 insertions(+), 14 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 260967f97b..83043451b8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,11 @@ +git-annex (7.20190130) UNRELEASED; urgency=medium + + * Fix false positive in export conflict detection, that occurred + when the same tree was exported by multiple clones. Previous fix was + incomplete. + + -- Joey Hess Wed, 30 Jan 2019 12:30:22 -0400 + git-annex (7.20190129) upstream; urgency=medium * initremote S3: When configured with versioning=yes, either ask the user diff --git a/Command/Export.hs b/Command/Export.hs index dda726b89b..9d99ef19dc 100644 --- a/Command/Export.hs +++ b/Command/Export.hs @@ -103,7 +103,7 @@ changeExport r ea db new = do startRecoverIncomplete r ea db (Git.DiffTree.srcsha diff) (Git.DiffTree.file diff) - forM_ (concatMap incompleteExportedTreeish old) $ \incomplete -> + forM_ (incompleteExportedTreeishes old) $ \incomplete -> mapdiff recover incomplete new -- Diff the old and new trees, and delete or rename to new name all @@ -113,7 +113,7 @@ changeExport r ea db new = do -- When there was an export conflict, this resolves it. -- -- The ExportTree is also updated here to reflect the new tree. - case nub (map exportedTreeish old) of + case exportedTreeishes old of [] -> updateExportTree db emptyTree new [oldtreesha] -> do diffmap <- mkDiffMap oldtreesha new db @@ -158,7 +158,7 @@ changeExport r ea db new = do c <- Annex.getState Annex.errcounter when (c == 0) $ do recordExport (uuid r) $ ExportChange - { oldTreeish = map exportedTreeish old + { oldTreeish = exportedTreeishes old , newTreeish = new } where diff --git a/Command/Sync.hs b/Command/Sync.hs index f223afac18..8e13da299d 100644 --- a/Command/Sync.hs +++ b/Command/Sync.hs @@ -700,8 +700,8 @@ seekExportContent rs (currbranch, _) = or <$> forM rs go Nothing -> nontracking r Just cur -> do Command.Export.changeExport r ea db cur - return [Exported cur []] - Export.closeDb db `after` fillexport r ea db exported + return [mkExported cur []] + Export.closeDb db `after` fillexport r ea db (exportedTreeishes exported) nontracking r = do exported <- getExport (Remote.uuid r) @@ -709,7 +709,7 @@ seekExportContent rs (currbranch, _) = or <$> forM rs go return exported warnnontracking r exported currb = inRepo (Git.Ref.tree currb) >>= \case - Just currt | not (any (\ex -> exportedTreeish ex == currt) exported) -> + Just currt | not (any (== currt) (exportedTreeishes exported)) -> showLongNote $ unwords [ "Not updating export to " ++ Remote.name r , "to reflect changes to the tree, because export" @@ -721,7 +721,7 @@ seekExportContent rs (currbranch, _) = or <$> forM rs go fillexport _ _ _ [] = return False - fillexport r ea db (Exported { exportedTreeish = t }:[]) = + fillexport r ea db (t:[]) = Command.Export.fillExport r ea db t fillexport r _ _ _ = do warnExportConflict r diff --git a/Database/Export.hs b/Database/Export.hs index 9786196b46..ab36f96a01 100644 --- a/Database/Export.hs +++ b/Database/Export.hs @@ -216,7 +216,7 @@ updateExportTreeFromLog db@(ExportHandle _ u) = old <- liftIO $ fromMaybe emptyTree <$> getExportTreeCurrent db l <- Log.getExport u - case map Log.exportedTreeish l of + case Log.exportedTreeishes l of [] -> return ExportUpdateSuccess (new:[]) | new /= old -> do updateExportTree db old new diff --git a/Logs/Export.hs b/Logs/Export.hs index 57fa0f565f..74f24412ef 100644 --- a/Logs/Export.hs +++ b/Logs/Export.hs @@ -1,11 +1,21 @@ {- git-annex export log - - - Copyright 2017 Joey Hess + - Copyright 2017-2019 Joey Hess - - Licensed under the GNU GPL version 3 or higher. -} -module Logs.Export where +module Logs.Export ( + Exported, + mkExported, + ExportParticipants, + ExportChange(..), + getExport, + exportedTreeishes, + incompleteExportedTreeishes, + recordExport, + recordExportBeginning, +) where import qualified Data.Map as M @@ -23,12 +33,29 @@ import qualified Data.Attoparsec.ByteString.Lazy as A import qualified Data.Attoparsec.ByteString.Char8 as A8 import Data.ByteString.Builder +-- This constuctor is not itself exported to other modules, to enforce +-- consistent use of exportedTreeishes. data Exported = Exported { exportedTreeish :: Git.Ref , incompleteExportedTreeish :: [Git.Ref] } deriving (Eq, Show) +mkExported :: Git.Ref -> [Git.Ref] -> Exported +mkExported = Exported + +-- | Get the list of exported treeishes. +-- +-- If the list contains multiple items, there was an export conflict, +-- and different trees were exported to the same special remote. +exportedTreeishes :: [Exported] -> [Git.Ref] +exportedTreeishes = nub . map exportedTreeish + +-- | Treeishes that started to be exported, but were not finished. +incompleteExportedTreeishes :: [Exported] -> [Git.Ref] +incompleteExportedTreeishes = concatMap incompleteExportedTreeish + + data ExportParticipants = ExportParticipants { exportFrom :: UUID , exportTo :: UUID @@ -41,9 +68,6 @@ data ExportChange = ExportChange } -- | Get what's been exported to a special remote. --- --- If the list contains multiple items, there was an export conflict, --- and different trees were exported to the same special remote. getExport :: UUID -> Annex [Exported] getExport remoteuuid = nub . mapMaybe get . M.toList . simpleMap . parseExportLog diff --git a/Remote/Helper/Export.hs b/Remote/Helper/Export.hs index 86f43d2fe5..abe617a971 100644 --- a/Remote/Helper/Export.hs +++ b/Remote/Helper/Export.hs @@ -188,7 +188,7 @@ adjustExportable r = case M.lookup "exporttree" (config r) of , checkPresentCheap = False , mkUnavailable = return Nothing , getInfo = do - ts <- map (fromRef . exportedTreeish) + ts <- map fromRef . exportedTreeishes <$> getExport (uuid r) is <- getInfo r return (is++[("export", "yes"), ("exportedtree", unwords ts)])