change v6 git-annex add of staged unmodified unlocked file

v6: When a file is unlocked but has not been modified, and the unlocking is
only staged, git-annex add did not lock it. Now it will, for consistency
with how modified files are handled and with v5.

Note the removal of the sameInodeCache check. Otherwise it would see
that the unmodified file is unmodified and stop there. That check seems to have
been copied from the direct mode branch. But, direct mode had a specific
reason to check for unmodified content, that does not apply to v6.

The second pass means there is potential for a race, eg the unlocked
file could be modified in between the first and second passes.
No problem with that, since both passes do the same thing.

This commit was sponsored by Jake Vosloo on Patreon.

CHANGELOG

@@ -21,6 +21,10 @@ git-annex (6.20180808) UNRELEASED; urgency=medium
   * v6: Fix some race conditions.
   * v6: Fix annex object file permissions when git-annex add is run
     on a modified unlocked file, and in some related cases.
+  * v6: When a file is unlocked but has not been modified,
+    and the unlocking is only staged, git-annex add did not lock it.
+    Now it will, for consistency with how modified files are handled and
+    with v5.
   * Fix git command queue to be concurrency safe.
   * linux standalone: When LOCPATH is already set, use it instead of the
     bundled locales. It can be set to an empty string to use the system

CmdLine/Seek.hs

@@ -4,7 +4,7 @@
  - the values a user passes to a command, and prepare actions operating
  - on them.
  -
- - Copyright 2010-2016 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2018 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU GPL version 3 or higher.
  -}
@@ -128,9 +128,6 @@ withFilesToBeCommitted a l = seekActions $ prepFiltered a $
 withFilesOldUnlocked :: (FilePath -> CommandStart) -> [WorkTreeItem] -> CommandSeek
 withFilesOldUnlocked = withFilesOldUnlocked' LsFiles.typeChanged
 
-withFilesOldUnlockedToBeCommitted :: (FilePath -> CommandStart) -> [WorkTreeItem] -> CommandSeek
-withFilesOldUnlockedToBeCommitted = withFilesOldUnlocked' LsFiles.typeChangedStaged
-
 {- Unlocked files before v6 have changed type from a symlink to a regular file.
  -
  - Furthermore, unlocked files used to be a git-annex symlink,
@@ -146,6 +143,19 @@ isOldUnlocked :: FilePath -> Annex Bool
 isOldUnlocked f = liftIO (notSymlink f) <&&> 
 	(isJust <$> catKeyFile f <||> isJust <$> catKeyFileHEAD f)
 
+withFilesOldUnlockedToBeCommitted :: (FilePath -> CommandStart) -> [WorkTreeItem] -> CommandSeek
+withFilesOldUnlockedToBeCommitted = withFilesOldUnlocked' LsFiles.typeChangedStaged
+
+{- v6 unlocked pointer files that are staged to be committed -}
+withUnlockedPointersToBeCommitted :: (FilePath -> CommandStart) -> [WorkTreeItem] -> CommandSeek
+withUnlockedPointersToBeCommitted a l = seekActions $
+	prepFiltered a unlockedfiles
+  where
+	unlockedfiles = filterM isV6Unlocked =<< seekHelper LsFiles.typeChangedStaged l
+
+isV6Unlocked :: FilePath -> Annex Bool
+isV6Unlocked f = (isJust <$> catKeyFile f <||> isJust <$> catKeyFileHEAD f)
+
 {- Finds files that may be modified. -}
 withFilesMaybeModified :: (FilePath -> CommandStart) -> [WorkTreeItem] -> CommandSeek
 withFilesMaybeModified a params = seekActions $

Command/Add.hs

@@ -69,8 +69,11 @@ seek o = allowConcurrentOutput $ do
 			unless (updateOnly o) $
 				go (withFilesNotInGit (not $ includeDotFiles o))
 			go withFilesMaybeModified
-			unlessM (versionSupportsUnlockedPointers <||> isDirect) $
+			ifM versionSupportsUnlockedPointers
-				go withFilesOldUnlocked
+				( go withUnlockedPointersToBeCommitted
+				, unlessM isDirect $
+					go withFilesOldUnlocked
+				)
 
 {- Pass file off to git-add. -}
 startSmall :: FilePath -> CommandStart
@@ -111,8 +114,7 @@ start file = do
 	addpresent key = ifM versionSupportsUnlockedPointers
 		( liftIO (catchMaybeIO $ getSymbolicLinkStatus file) >>= \case
 			Just s | isSymbolicLink s -> fixuplink key
-			_ -> ifM (sameInodeCache file =<< Database.Keys.getInodeCaches key)
+			_ -> add
-				( stop, add )
 		, ifM isDirect
 			( liftIO (catchMaybeIO $ getSymbolicLinkStatus file) >>= \case
 				Just s | isSymbolicLink s -> fixuplink key

doc/bugs/__39__annex_add__39___locks_unmodified_file_in_V5_but_not_V6.mdwn

@@ -38,3 +38,5 @@ special handling for the V6 behavior.
     upgrade supported from repository versions: 0 1 2 3 4 5
 
 on Debian Stretch, built from source with `stack build`
+
+> [[fixed|done]] --[[Joey]]

doc/bugs/__39__annex_add__39___locks_unmodified_file_in_V5_but_not_V6/comment_1_a54ff1147b025b58e65ce0be6e795029._comment

@@ -0,0 +1,23 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2018-09-12T16:29:55Z"
+ content="""
+In fact, `git annex add` does not process the v6 unlocked file at all
+since it only looks for unstaged changes to files and the unlocked file's
+type change has been staged already.
+
+In v5 mode there is a separate pass to add unlocked files, which is
+necessary since they have to be converted back to locked files before they
+can be committed.
+
+It would need a separate pass in v6 too, since the main pass looks only
+at unstaged modifications and git can't be queried for staged modifications
+at the same time as unstaged.
+
+Hmm, this would though mean that `git annex add` would now be changing
+what's staged. It has never done that before; it's only staged new changes.
+Not convinced by that argument, but something to keep in mind.
+
+I'm feeling this is ok to change, and the patch is not difficult.
+"""]]