From d9490685fd5311c59ad4194ea162a42a0ca6e8b8 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 13 Dec 2016 11:07:49 -0400 Subject: [PATCH] metadata --batch: Fix bug when conflicting metadata changes were made in the same batch run. 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. --- CHANGELOG | 2 + Command/MetaData.hs | 47 +++++++++++-------- ...y_modified_in_the_same_batch_mode_run.mdwn | 1 + 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6978142805..839af6559a 100644 --- 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 Sun, 11 Dec 2016 21:29:51 -0400 diff --git a/Command/MetaData.hs b/Command/MetaData.hs index 04d859e4cf..ebb9d0f174 100644 --- 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 1ec07dccbe..26790bea3f 100644 --- 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]]