pre-commit: Update direct mode mappings.

Making the pre-commit hook look at git diff-index to find changed direct
mode files and update the mappings works pretty well.

One case where it does not work is when a file is git annex added, and then
git rmed, and then this is committed. That's a no-op commit, so the hook
probably doesn't even run, and it certianly never notices that the file
was deleted, so the mapping will still have the original filename in it.

For this and other reasons, it's important that the mappings still be
treated as possibly inconsistent.

Also, the assistant now allows the pre-commit hook to run when in direct
mode, so the mappings also get updated there.

Assistant/Threads/Committer.hs

@@ -82,12 +82,16 @@ commitStaged = do
 	case v of
 		Left _ -> return False
 		Right _ -> do
-			void $ inRepo $ Git.Command.runBool "commit" $ nomessage
-				[ Param "--quiet"
-				{- Avoid running the usual git-annex pre-commit hook;
-				 - watch does the same symlink fixing, and we don't want
-				 - to deal with unlocked files in these commits. -}
-				, Param "--no-verify"
+			direct <- isDirect
+			void $ inRepo $ Git.Command.runBool "commit" $ nomessage $
+				catMaybes
+				[ Just $ Param "--quiet"
+				{- In indirect mode, avoid running the
+				 - usual git-annex pre-commit hook;
+				 - watch does the same symlink fixing,
+				 - and we don't want to deal with unlocked
+				 - files in these commits. -}
+				, if direct then Nothing else Just $ Param "--no-verify"
 				]
 			{- Empty commits may be made if tree changes cancel
 			 - each other out, etc. Git returns nonzero on those,

Command/PreCommit.hs

@@ -1,6 +1,6 @@
 {- git-annex command
  -
- - Copyright 2010 Joey Hess <joey@kitenet.net>
+ - Copyright 2010, 2013 Joey Hess <joey@kitenet.net>
  -
  - Licensed under the GNU GPL version 3 or higher.
  -}
@@ -11,22 +11,42 @@ import Common.Annex
 import Command
 import qualified Command.Add
 import qualified Command.Fix
+import qualified Git.DiffTree
+import Annex.CatFile
+import Annex.Content.Direct
+import Git.Sha
 
 def :: [Command]
 def = [command "pre-commit" paramPaths seek "run by git pre-commit hook"]
 
-{- The pre-commit hook needs to fix symlinks to all files being committed.
- - And, it needs to inject unlocked files into the annex. -}
 seek :: [CommandSeek]
 seek =
+	-- fix symlinks to files being committed
 	[ whenNotDirect $ withFilesToBeCommitted $ whenAnnexed $ Command.Fix.start
-	, whenNotDirect $ withFilesUnlockedToBeCommitted start]
+	-- inject unlocked files into the annex
+	, whenNotDirect $ withFilesUnlockedToBeCommitted startIndirect
+	-- update direct mode mappings for committed files
+	, whenDirect $ withWords startDirect
+	]
 
-start :: FilePath -> CommandStart
-start file = next $ perform file
-
-perform :: FilePath -> CommandPerform
-perform file = do
+startIndirect :: FilePath -> CommandStart
+startIndirect file = next $ do
 	unlessM (doCommand $ Command.Add.start file) $
 		error $ "failed to add " ++ file ++ "; canceling commit"
 	next $ return True
+
+startDirect :: [String] -> CommandStart
+startDirect _ = next $ do
+	(diffs, clean) <- inRepo $ Git.DiffTree.diffIndex
+	forM_ diffs go
+	next $ liftIO clean
+  where
+	go diff = do
+		withkey (Git.DiffTree.srcsha diff) removeAssociatedFile
+		withkey (Git.DiffTree.dstsha diff) addAssociatedFile
+	  where
+		withkey sha a = when (sha /= nullSha) $ do
+			k <- catKey sha
+			case k of
+				Nothing -> noop
+				Just key -> void $ a key (Git.DiffTree.file diff)

Git/DiffTree.hs

@@ -9,7 +9,7 @@ module Git.DiffTree (
 	DiffTreeItem(..),
 	diffTree,
 	diffTreeRecursive,
-	parseDiffTree
+	diffIndex,
 ) where
 
 import Numeric
@@ -20,6 +20,7 @@ import Git
 import Git.Sha
 import Git.Command
 import qualified Git.Filename
+import qualified Git.Ref
 
 data DiffTreeItem = DiffTreeItem
 	{ srcmode :: FileMode
@@ -32,19 +33,29 @@ data DiffTreeItem = DiffTreeItem
 
 {- Diffs two tree Refs. -}
 diffTree :: Ref -> Ref -> Repo -> IO ([DiffTreeItem], IO Bool)
-diffTree = diffTree' []
+diffTree src dst = getdiff (Param "diff-tree")
+	[Param (show src), Param (show dst)]
 
 {- Diffs two tree Refs, recursing into sub-trees -}
 diffTreeRecursive :: Ref -> Ref -> Repo -> IO ([DiffTreeItem], IO Bool)
-diffTreeRecursive = diffTree' [Param "-r"]
+diffTreeRecursive src dst = getdiff (Param "diff-tree")
+	[Param "-r", Param (show src), Param (show dst)]
 
-diffTree' :: [CommandParam] -> Ref -> Ref -> Repo -> IO ([DiffTreeItem], IO Bool)
-diffTree' params src dst repo = do
+{- Diffs between the repository and index. Does nothing if there is not
+ - yet a commit in the repository. -}
+diffIndex :: Repo -> IO ([DiffTreeItem], IO Bool)
+diffIndex repo = do
+	ifM (Git.Ref.headExists repo)
+		( getdiff (Param "diff-index") [Param "--cached", Param "HEAD"] repo
+		, return ([], return True)
+		)
+
+getdiff :: CommandParam -> [CommandParam] -> Repo -> IO ([DiffTreeItem], IO Bool)
+getdiff command params repo = do
 	(diff, cleanup) <- pipeNullSplit ps repo
 	return (parseDiffTree diff, cleanup)
   where
-	ps = Params "diff-tree -z --raw --no-renames -l0" : params ++
-		[Param (show src), Param (show dst)]
+	ps = command : Params "-z --raw --no-renames -l0" : params
 
 {- Parses diff-tree output. -}
 parseDiffTree :: [String] -> [DiffTreeItem]

Git/Ref.hs

@@ -37,6 +37,13 @@ exists :: Ref -> Repo -> IO Bool
 exists ref = runBool "show-ref" 
 	[Param "--verify", Param "-q", Param $ show ref]
 
+{- Checks if HEAD exists. It generally will, except for in a repository
+ - that was just created. -}
+headExists :: Repo -> IO Bool
+headExists repo = do
+	ls <- lines <$> pipeReadStrict [Param "show-ref", Param "--head"] repo
+	return $ any (" HEAD" `isSuffixOf`) ls
+
 {- Get the sha of a fully qualified git ref, if it exists. -}
 sha :: Branch -> Repo -> IO (Maybe Sha)
 sha branch repo = process <$> showref repo

Seek.hs

@@ -127,3 +127,6 @@ notSymlink f = liftIO $ not . isSymbolicLink <$> getSymbolicLinkStatus f
 
 whenNotDirect :: CommandSeek -> CommandSeek
 whenNotDirect a params = ifM isDirect ( return [] , a params )
+
+whenDirect :: CommandSeek -> CommandSeek
+whenDirect a params = ifM isDirect ( a params, return [] )

debian/changelog

@@ -10,6 +10,7 @@ git-annex (3.20130125) UNRELEASED; urgency=low
   * assistant: Ignore .DS_Store on OSX.
   * assistant: Fix location log when adding new file in direct mode.
   * Deal with stale mappings for deleted file in direct mode.
+  * pre-commit: Update direct mode mappings. 
 
  -- Joey Hess <joeyh@debian.org>  Sat, 26 Jan 2013 15:48:40 +1100
 

doc/bugs/direct_mode_renames.mdwn

@@ -8,6 +8,8 @@ file working properly in direct mode.
 Perhaps the pre-commit hook needs to update the mapping for files that were
 deleted or added.
 
-(This also affects moves of files when the assistant is being used.
+This also affects moves of files when the assistant is being used.
 In this case, the assistant updates the mapping to add the new name,
-but does not delete the old name from the mapping.)
+but does not delete the old name from the mapping.
+
+> [[done]]; the pre-commit hook now updates the mappings. --[[Joey]]

doc/design/assistant/desymlink.mdwn

@@ -58,9 +58,9 @@ is converted to a real file when it becomes present.
   This allows just cloning one of these repositories normally, and then
   as the files are synced in, they become real files.
 * Maintain a local mapping from keys to files in the tree. This is needed
-  when sending/receiving keys to know what file to access. Note that a key
-  can map to multiple files. And that when a file is deleted or moved, the
-  mapping needs to be updated.
+  when sending/receiving/dropping keys to know what file to access.
+  Note that a key can map to multiple files. And that when a file is
+  deleted or moved, the mapping needs to be updated.
 * May need a reverse mapping, from files in the tree to keys? TBD
   (Currently, getting by looking up symlinks using `git cat-file`)
   (Needed to make things like `git annex drop` that want to map from the
@@ -87,12 +87,6 @@ is converted to a real file when it becomes present.
 * kqueue does not deliver an event when an existing file is modified.
   This doesn't affect OSX, which uses FSEvents now, but it makes direct
   mode assistant not 100% on other BSD's.
-* The mapping is not updated when files are deleted (or for the deletion
-  part of a rename). So it can contain old filenames that are no longer in
-  use. Code that uses the mapping has to take care to check that the
-  files they refer to exists, which is a good idea anyway. But,
-  it would be good to at some point update the mappings to remove deleted
-  files (fsck does this FWIW).
 
 ## done