assistant: In direct mode, objects are now only dropped when all associated files are unwanted. This avoids a repreated drop/get loop of a file that has a copy in an archive directory, and a copy not in an archive directory. (Indirect mode still has some buggy behavior in this area, since it does not keep track of associated files.) Closes: #712060

Annex/Content/Direct.hs

@@ -7,6 +7,7 @@
 
 module Annex.Content.Direct (
 	associatedFiles,
+	associatedFilesRelative,
 	removeAssociatedFile,
 	removeAssociatedFileUnchecked,
 	addAssociatedFile,

Assistant/Drop.hs

@@ -18,6 +18,7 @@ import Command
 import Annex.Wanted
 import Annex.Exception
 import Config
+import Annex.Content.Direct
 
 import qualified Data.Set as S
 
@@ -35,20 +36,26 @@ handleDrops reason fromhere key f knownpresentremote = do
 {- The UUIDs are ones where the content is believed to be present.
  - The Remote list can include other remotes that do not have the content;
  - only ones that match the UUIDs will be dropped from.
- - If allows to drop fromhere, that drop will be tried first. -}
+ - If allowed to drop fromhere, that drop will be tried first.
+ -
+ - In direct mode, all associated files are checked, and only if all
+ - of them are unwanted are they dropped.
+ -}
 handleDropsFrom :: [UUID] -> [Remote] -> Reason -> Bool -> Key -> AssociatedFile -> Maybe Remote -> Assistant ()
 handleDropsFrom _ _ _ _ _ Nothing _ = noop
-handleDropsFrom locs rs reason fromhere key (Just f) knownpresentremote
+handleDropsFrom locs rs reason fromhere key (Just afile) knownpresentremote = do
-	| fromhere = do
+	fs <- liftAnnex $ ifM isDirect
-		n <- getcopies
+		( associatedFilesRelative key
-		if checkcopies n Nothing
+		, return [afile]
-			then go rs =<< dropl n
+		)
-			else go rs n
+	n <- getcopies fs
-	| otherwise = go rs =<< getcopies
+	if fromhere && checkcopies n Nothing
+		then go fs rs =<< dropl fs n
+		else go fs rs n
   where
-	getcopies = liftAnnex $ do
+	getcopies fs = liftAnnex $ do
 		(untrusted, have) <- trustPartition UnTrusted locs
-		numcopies <- getNumCopies =<< numCopies f
+		numcopies <- maximum <$> mapM (getNumCopies <=< numCopies) fs
 		return (length have, numcopies, S.fromList untrusted)
 
 	{- Check that we have enough copies still to drop the content.
@@ -66,20 +73,20 @@ handleDropsFrom locs rs reason fromhere key (Just f) knownpresentremote
 		| S.member u untrusted = v
 		| otherwise = decrcopies v Nothing
 
-	go [] _ = noop
+	go _ [] _ = noop
-	go (r:rest) n
+	go fs (r:rest) n
-		| uuid r `S.notMember` slocs = go rest n
+		| uuid r `S.notMember` slocs = go fs rest n
 		| checkcopies n (Just $ Remote.uuid r) =
-			dropr r n >>= go rest
+			dropr fs r n >>= go fs rest
 		| otherwise = noop
 
-	checkdrop n@(have, numcopies, _untrusted) u a =
+	checkdrop fs n@(have, numcopies, _untrusted) u a =
-		ifM (liftAnnex $ wantDrop True u (Just f))
+		ifM (liftAnnex $ allM (wantDrop True u . Just) fs)
 			( ifM (liftAnnex $ safely $ doCommand $ a (Just numcopies))
 				( do
 					debug
 						[ "dropped"
-						, f
+						, afile
 						, "(from " ++ maybe "here" show u ++ ")"
 						, "(copies now " ++ show (have - 1) ++ ")"
 						, ": " ++ reason
@@ -90,11 +97,11 @@ handleDropsFrom locs rs reason fromhere key (Just f) knownpresentremote
 			, return n
 			)
 
-	dropl n = checkdrop n Nothing $ \numcopies ->
+	dropl fs n = checkdrop fs n Nothing $ \numcopies ->
-		Command.Drop.startLocal f numcopies key knownpresentremote
+		Command.Drop.startLocal afile numcopies key knownpresentremote
 
-	dropr r n  = checkdrop n (Just $ Remote.uuid r) $ \numcopies ->
+	dropr fs r n  = checkdrop fs n (Just $ Remote.uuid r) $ \numcopies ->
-		Command.Drop.startRemote f numcopies key r
+		Command.Drop.startRemote afile numcopies key r
 
 	safely a = either (const False) id <$> tryAnnex a
 

Utility/Monad.hs

@@ -27,6 +27,10 @@ getM p (x:xs) = maybe (getM p xs) (return . Just) =<< p x
 anyM :: Monad m => (a -> m Bool) -> [a] -> m Bool
 anyM p = liftM isJust . firstM p
 
+allM :: Monad m => (a -> m Bool) -> [a] -> m Bool
+allM _ [] = return True
+allM p (x:xs) = p x <&&> allM p xs
+
 {- Runs an action on values from a list until it succeeds. -}
 untilTrue :: Monad m => [a] -> (a -> m Bool) -> m Bool
 untilTrue = flip anyM

debian/changelog

@@ -21,6 +21,12 @@ git-annex (4.20130602) UNRELEASED; urgency=low
   * Android: Fix use of cp command to not try to use features present
     only on build system.
   * Windows: Fix hang when adding several files at once.
+  * assistant: In direct mode, objects are now only dropped when all
+    associated files are unwanted. This avoids a repreated drop/get loop
+    of a file that has a copy in an archive directory, and a copy not in an
+    archive directory. (Indirect mode still has some buggy behavior in this
+    area, since it does not keep track of associated files.)
+    Closes: #712060
 
  -- Joey Hess <joeyh@debian.org>  Mon, 10 Jun 2013 12:52:44 -0400
 

doc/bugs/Handling_of_files_inside_and_outside_archive_directory_at_the_same_time.mdwn

@@ -7,3 +7,11 @@ Karsten
 
 [[!tag /design/assistant]]
 [[!meta title="assistant preferred content handling of files inside and outside archive directory at the same time"]]
+
+> Update: Current status is this is fixed for direct mode.
+> 
+> In indirect mode, the startup scan will still download and then drop
+> content if a file outside and inside the archive directory has the
+> same content. It doesn't loop like it did in direct mode, only
+> happens once (or once per duplicate file, really). Is still potentially
+> annoying and a bug. --[[Joey]]