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

@@ -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 <id@joeyh.name>  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

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

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 []]
+						return [mkExported cur []]
-		Export.closeDb db `after` fillexport r ea db exported
+		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

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

Logs/Export.hs

@@ -1,11 +1,21 @@
 {- git-annex export log
  -
- - Copyright 2017 Joey Hess <id@joeyh.name>
+ - Copyright 2017-2019 Joey Hess <id@joeyh.name>
  -
  - 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

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