Avoid unncessary write to the location log when a file is unlocked and then added back with unchanged content.

Implemented with no additional overhead of compares etc.

This is safe to do for presence logs because of their locality of change;
a given repo's presence logs are only ever changed in that repo, or in a
repo that has just been actively changing the content of that repo.

So, we don't need to worry about a split-brain situation where there'd
be disagreement about the location of a key in a repo. And so, it's ok to
not update the timestamp when that's the only change that would be made
due to logging presence info.

7 files changed, 65 insertions, 14 deletions

diff --git a/Annex/Branch.hs b/Annex/Branch.hs
index 5436132d8..ad96a2073 100644
--- a/Annex/Branch.hs
+++ b/Annex/Branch.hs
@@ -18,6 +18,7 @@ module Annex.Branch (
 	get,
 	getHistorical,
 	change,
+	maybeChange,
 	commit,
 	forceCommit,
 	files,
@@ -224,7 +225,15 @@ getRef ref file = withIndex $ decodeBS <$> catFile ref file
  - modifes the current content of the file on the branch.
  -}
 change :: FilePath -> (String -> String) -> Annex ()
-change file a = lockJournal $ \jl -> a <$> getLocal file >>= set jl file
+change file f = lockJournal $ \jl -> f <$> getLocal file >>= set jl file
+
+{- Applies a function which can modify the content of a file, or not. -}
+maybeChange :: FilePath -> (String -> Maybe String) -> Annex ()
+maybeChange file f = lockJournal $ \jl -> do
+	v <- getLocal file
+	case f v of
+		Just v' | v' /= v -> set jl file v'
+		_ -> noop
 
 {- Records new content of a file into the journal -}
 set :: JournalLocked -> FilePath -> String -> Annex ()
diff --git a/Logs/Location.hs b/Logs/Location.hs
index ba9c31abf..89100805b 100644
--- a/Logs/Location.hs
+++ b/Logs/Location.hs
@@ -48,7 +48,7 @@ logChange = logChange' logNow
 logChange' :: (LogStatus -> String -> Annex LogLine) -> Key -> UUID -> LogStatus -> Annex ()
 logChange' mklog key (UUID u) s = do
 	config <- Annex.getGitConfig
-	addLog (locationLogFile config key) =<< mklog s u
+	maybeAddLog (locationLogFile config key) =<< mklog s u
 logChange' _ _ NoUUID _ = noop
 
 {- Returns a list of repository UUIDs that, according to the log, have
diff --git a/Logs/Presence.hs b/Logs/Presence.hs
index 60e0c542a..f90253421 100644
--- a/Logs/Presence.hs
+++ b/Logs/Presence.hs
@@ -4,7 +4,7 @@
  - a way that can be union merged.
  -
  - A line of the log will look like: "date N INFO"
- - Where N=1 when the INFO is present, and 0 otherwise.
+ - Where N=1 when the INFO is present, 0 otherwise.
  - 
  - Copyright 2010-2014 Joey Hess <id@joeyh.name>
  -
@@ -14,6 +14,7 @@
 module Logs.Presence (
 	module X,
 	addLog,
+	maybeAddLog,
 	readLog,
 	logNow,
 	currentLog,
@@ -28,10 +29,21 @@ import Common.Annex
 import qualified Annex.Branch
 import Git.Types (RefDate)
 
+{- Adds a LogLine to the log, removing any LogLines that are obsoleted by
+ - adding it. -}
 addLog :: FilePath -> LogLine -> Annex ()
-addLog file line = Annex.Branch.change file $ \s -> 
+addLog file line = Annex.Branch.change file $ \s ->
 	showLog $ compactLog (line : parseLog s)
 
+{- When a LogLine already exists with the same status and info, but an
+ - older timestamp, that LogLine is preserved, rather than updating the log
+ - with a newer timestamp.
+ -}
+maybeAddLog :: FilePath -> LogLine -> Annex ()
+maybeAddLog file line = Annex.Branch.maybeChange file $ \s -> do
+	m <- insertNewStatus line $ logMap $ parseLog s
+	return $ showLog $ mapLog m
+
 {- Reads a log file.
  - Note that the LogLines returned may be in any order. -}
 readLog :: FilePath -> Annex [LogLine]
diff --git a/Logs/Presence/Pure.hs b/Logs/Presence/Pure.hs
index b1fc212fd..4e5ff68c0 100644
--- a/Logs/Presence/Pure.hs
+++ b/Logs/Presence/Pure.hs
@@ -61,21 +61,39 @@ filterPresent = filter (\l -> InfoPresent == status l) . compactLog
 {- Compacts a set of logs, returning a subset that contains the current
  - status. -}
 compactLog :: [LogLine] -> [LogLine]
-compactLog = M.elems . foldr mapLog M.empty
+compactLog = mapLog . logMap
 
 type LogMap = M.Map String LogLine
 
-{- Inserts a log into a map of logs, if the log has better (ie, newer)
- - information than the other logs in the map -}
-mapLog :: LogLine -> LogMap -> LogMap
-mapLog l m
-	| better = M.insert i l m
-	| otherwise = m
+mapLog :: LogMap -> [LogLine]
+mapLog = M.elems
+
+logMap :: [LogLine] -> LogMap
+logMap = foldr insertNewerLogLine M.empty
+
+insertBetter :: (LogLine -> Bool) -> LogLine -> LogMap -> Maybe LogMap
+insertBetter betterthan l m
+	| better = Just (M.insert i l m)
+	| otherwise = Nothing
   where
-	better = maybe True newer $ M.lookup i m
-	newer l' = date l' <= date l
+	better = maybe True betterthan (M.lookup i m)
 	i = info l
 
+{- Inserts a log into a map of logs, if the log has newer
+ - information than the other logs in the map for the same info. -}
+insertNewerLogLine :: LogLine -> LogMap -> LogMap
+insertNewerLogLine l m = fromMaybe m $ insertBetter newer l m
+  where
+	newer l' = date l' <= date l
+
+{- Inserts the log unless there's already one in the map with
+ - the same status for its info, in which case there's no need to
+ - change anything, to avoid log churn. -}
+insertNewStatus :: LogLine -> LogMap -> Maybe LogMap
+insertNewStatus l m  = insertBetter diffstatus l m
+  where
+	diffstatus l' = status l' /= status l
+
 instance Arbitrary LogLine where
 	arbitrary = LogLine
 		<$> arbitrary
diff --git a/debian/changelog b/debian/changelog
index 250e183a6..1f336b9ec 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -30,11 +30,13 @@ git-annex (5.20150931) UNRELEASED; urgency=medium
   * copy --auto was checking the wrong repo's preferred content.
     (--from was checking what --to should, and vice-versa.)
     Fixed this bug, which was introduced in version 5.20150727.
+  * Avoid unncessary write to the location log when a file is unlocked
+    and then added back with unchanged content.
   * Debian: Add torrent library to build-depends as it's packaged now,
     and stop recommending bittornado | bittorrent.
   * Debian: Remove dependency on transformers library, as it is now
     included in ghc.
-    
+
  -- Joey Hess <id@joeyh.name>  Thu, 01 Oct 2015 12:42:56 -0400
 
 git-annex (5.20150930) unstable; urgency=medium
diff --git a/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry.mdwn b/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry.mdwn
index d9ea181a8..258b82f6a 100644
--- a/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry.mdwn
+++ b/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry.mdwn
@@ -7,3 +7,5 @@ I keep a database file in git-annex so it stays synced across several machines.
     git annex sync --content
 
 This works fine except it creates an entry in the log for the database file even if that stays unchanged. I would like an option that just returns to the state before `git annex unlock` if the file has not been changed. Maybe `git annex add --restore-unchanged`?
+
+> [[done]] --[[Joey]]
diff --git a/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry/comment_2_3a3d793b32b8440a8213b38bc83ea94a._comment b/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry/comment_2_3a3d793b32b8440a8213b38bc83ea94a._comment
new file mode 100644
index 000000000..aeb98f06e
--- /dev/null
+++ b/doc/todo/wishlist:_allow_re-adding_without_generating_log_entry/comment_2_3a3d793b32b8440a8213b38bc83ea94a._comment
@@ -0,0 +1,8 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 2"""
+ date="2015-10-12T17:37:59Z"
+ content="""
+Ok, I found a way to implement it with no added overhead in the common
+case!
+"""]]