metadata --batch: Fix bug when conflicting metadata changes were made in the same...

1 microsecond delay is ugly.. but, maintaining an queue of a list of timestamps
and taking a new one from the queue each time around, or maintaining a timestamp
counter, would probably be slower.

diff --git a/CHANGELOG b/CHANGELOG
index 6978142805039691784e7506f88006bfd8399752..839af6559a186795fd690257466ceb64cfc1fa5b 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,8 @@
 git-annex (6.20161211) UNRELEASED; urgency=medium
 
   * Debian: Build webapp on armel.
+  * metadata --batch: Fix bug when conflicting metadata changes were
+    made in the same batch run.
 
  -- Joey Hess <id@joeyh.name>  Sun, 11 Dec 2016 21:29:51 -0400
 

diff --git a/Command/MetaData.hs b/Command/MetaData.hs
index 04d859e4cf25163c0e3d4fc3ca4b934b8b1865f1..ebb9d0f1745153e56573b133307a0036a3cb946d 100644 (file)
--- a/Command/MetaData.hs
+++ b/Command/MetaData.hs
@@ -20,6 +20,7 @@ import qualified Data.Text as T
 import qualified Data.ByteString.Lazy.UTF8 as BU
 import Data.Time.Clock.POSIX
 import Data.Aeson
+import Control.Concurrent
 
 cmd :: Command
 cmd = withGlobalOptions ([jsonOption] ++ annexedMatchingOptions) $ 
@@ -65,23 +66,22 @@ optParser desc = MetaDataOptions
                        )
 
 seek :: MetaDataOptions -> CommandSeek
-seek o = do
-       now <- liftIO getPOSIXTime
-       case batchOption o of
-               NoBatch -> do
-                       let seeker = case getSet o of
-                               Get _ -> withFilesInGit
-                               GetAll -> withFilesInGit
-                               Set _ -> withFilesInGitNonRecursive
-                                       "Not recursively setting metadata. Use --force to do that."
-                       withKeyOptions (keyOptions o) False
-                               (startKeys now o)
-                               (seeker $ whenAnnexed $ start now o)
-                               (forFiles o)
-               Batch -> withMessageState $ \s -> case outputType s of
-                       JSONOutput _ -> batchInput parseJSONInput $
-                               commandAction . startBatch now
-                       _ -> giveup "--batch is currently only supported in --json mode"
+seek o = case batchOption o of
+       NoBatch -> do
+               now <- liftIO getPOSIXTime
+               let seeker = case getSet o of
+                       Get _ -> withFilesInGit
+                       GetAll -> withFilesInGit
+                       Set _ -> withFilesInGitNonRecursive
+                               "Not recursively setting metadata. Use --force to do that."
+               withKeyOptions (keyOptions o) False
+                       (startKeys now o)
+                       (seeker $ whenAnnexed $ start now o)
+                       (forFiles o)
+       Batch -> withMessageState $ \s -> case outputType s of
+               JSONOutput _ -> batchInput parseJSONInput $
+                       commandAction . startBatch
+               _ -> giveup "--batch is currently only supported in --json mode"
 
 start :: POSIXTime -> MetaDataOptions -> FilePath -> Key -> CommandStart
 start now o file k = startKeys now o k (mkActionItem afile)
@@ -150,8 +150,8 @@ parseJSONInput i = do
                (Nothing, Just f) -> Right (Left f, m)
                (Nothing, Nothing) -> Left "JSON input is missing either file or key"
 
-startBatch :: POSIXTime -> (Either FilePath Key, MetaData) -> CommandStart
-startBatch now (i, (MetaData m)) = case i of
+startBatch :: (Either FilePath Key, MetaData) -> CommandStart
+startBatch (i, (MetaData m)) = case i of
        Left f -> do
                mk <- lookupFile f
                case mk of
@@ -169,6 +169,15 @@ startBatch now (i, (MetaData m)) = case i of
                        , keyOptions = Nothing
                        , batchOption = NoBatch
                        }
+               now <- liftIO getPOSIXTime
+               -- It would be bad if two batch mode changes used exactly
+               -- the same timestamp, since the order of adds and removals
+               -- of the same metadata value would then be indeterminate.
+               -- To guarantee that never happens, delay 1 microsecond,
+               -- so the timestamp will always be different. This is
+               -- probably less expensive than cleaner methods,
+               -- such as taking from a list of increasing timestamps.
+               liftIO $ threadDelay 1
                next $ perform now o k
        mkModMeta (f, s)
                | S.null s = DelMeta f Nothing

diff --git a/doc/bugs/Metadata_values_get_stuck_when_repeatedly_modified_in_the_same_batch_mode_run.mdwn b/doc/bugs/Metadata_values_get_stuck_when_repeatedly_modified_in_the_same_batch_mode_run.mdwn
index 1ec07dccbe4eb5bc0042be5a19c33512022ca72a..26790bea3f75db813c88e1b90f23a408c672b639 100644 (file)
--- a/doc/bugs/Metadata_values_get_stuck_when_repeatedly_modified_in_the_same_batch_mode_run.mdwn
+++ b/doc/bugs/Metadata_values_get_stuck_when_repeatedly_modified_in_the_same_batch_mode_run.mdwn
@@ -82,3 +82,4 @@ I love the metadata functionality so much that I wrote [[tips/a_gui_for_metadata
 Metadata driven views are awesome (but I don't like the entire folder hierarchy being appended to the filename).
 I haven't used the other commands much since I have not yet organized most of my stuff (and their naively copy-pasted backups), but I am glad I discovered git-annex before I began organizing.
 
+> [[fixed|done]] --[[Joey]]