Warn when metadata is inherited from a previous version of a file

to avoid the user being surprised in cases where that behavior is not desired or expected

This commit was supported by the NSF-funded DataLad project.

Annex/MetaData.hs

@@ -39,12 +39,20 @@ import Data.Time.Clock.POSIX
  -}
 genMetaData :: Key -> FilePath -> FileStatus -> Annex ()
 genMetaData key file status = do
-	maybe noop (`copyMetaData` key) =<< catKeyFileHEAD file
+	v <- catKeyFileHEAD file
+	case v of
+		Nothing -> noop
+		Just oldkey -> 
+			whenM (copyMetaData oldkey key)
+				warncopied
 	whenM (annexGenMetaData <$> Annex.getGitConfig) $ do
 		curr <- getCurrentMetaData key
 		addMetaData key (dateMetaData mtime curr)
   where
 	mtime = posixSecondsToUTCTime $ realToFrac $ modificationTime status
+	warncopied = warning $ 
+		"Copied metadata from old version of " ++ file ++ " to new version. " ++ 
+		"If you don't want this copied metadata, run: git annex metadata --remove-all " ++ file
 
 {- Generates metadata for a file's date stamp.
  - Does not overwrite any existing metadata values. -}

CHANGELOG

@@ -3,6 +3,9 @@ git-annex (6.20170926) UNRELEASED; urgency=medium
   * webdav: Improve error message for failed request to include the request
     method and path.
   * metadata: Added --remove-all.
+  * Warn when metadata is inherited from a previous version of a file,
+    to avoid the user being surprised in cases where that behavior is not
+    desired or expected.
 
  -- Joey Hess <id@joeyh.name>  Thu, 28 Sep 2017 12:01:39 -0400
 

Logs/MetaData.hs

@@ -55,6 +55,9 @@ getMetaDataLog key = do
 	config <- Annex.getGitConfig
 	readLog $ metaDataLogFile config key
 
+logToCurrentMetaData :: [LogEntry MetaData] -> MetaData
+logToCurrentMetaData = currentMetaData . combineMetaData . map value
+
 {- Go through the log from oldest to newest, and combine it all
  - into a single MetaData representing the current state.
  -
@@ -64,7 +67,7 @@ getMetaDataLog key = do
 getCurrentMetaData :: Key -> Annex MetaData
 getCurrentMetaData k = do
 	ls <- S.toAscList <$> getMetaDataLog k
-	let loggedmeta = currentMetaData $ combineMetaData $ map value ls
+	let loggedmeta = logToCurrentMetaData ls
 	return $ currentMetaData $ unionMetaData loggedmeta
 		(lastchanged ls loggedmeta)
   where
@@ -177,13 +180,18 @@ simplifyLog s = case sl of
  - repository.
  - 
  - Any metadata already attached to the new key is not preserved.
+ -
+ - Returns True when metadata was copied.
  -}
-copyMetaData :: Key -> Key -> Annex ()
+copyMetaData :: Key -> Key -> Annex Bool
 copyMetaData oldkey newkey
-	| oldkey == newkey = noop
+	| oldkey == newkey = return False
 	| otherwise = do
 		l <- getMetaDataLog oldkey
-		unless (S.null l) $ do
-			config <- Annex.getGitConfig
-			Annex.Branch.change (metaDataLogFile config newkey) $
-				const $ showLog l
+		if logToCurrentMetaData (S.toAscList l) == emptyMetaData
+			then return False
+			else do
+				config <- Annex.getGitConfig
+				Annex.Branch.change (metaDataLogFile config newkey) $
+					const $ showLog l
+				return True

doc/bugs/set_metadata_leaks_from_one___40__staged__41___key_to_another_during_rename_of_file/comment_5_fa6d5d0afdc00824da0e04d9bf23d6f4._comment

@@ -0,0 +1,20 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 5"""
+ date="2017-09-28T16:03:28Z"
+ content="""
+Files are not unlocked before modifying in direct mode, and may be
+unlocked all the time in v6 mode. Also, in indirect mode it's of course
+fine to overwrite the symlink with a new version of a file. So detecting 
+if it's been unlocked doesn't seem to help with this.
+
+It may be that there are different sorts of metadata, some of which should
+be inherited by new versions of a file, and others not. If there was a way
+to tell git-annex which metadata was which, it could do the right thing.
+But it feels like stacking complications. Particularly since there might be
+some tags that should be inherited and others not, and tags are values..
+
+In the meantime, I've added the warning when it copies metadata. 
+I also added `git annex metadata --remove-all`, which the warning
+suggests running if you don't want the copied metadata.
+"""]]