support modifying and running git add on an unlocked file that used an URL key

Avoids the smudge --clean filter failing because URL keys do not support
genKey. Instead the modified content will be added using the default
backend.

This commit was sponsored by Jochen Bartl on Patreon.

CHANGELOG

@@ -37,6 +37,8 @@ git-annex (8.20201130) UNRELEASED; urgency=medium
   * Fix a bug in view filename generation when a metadata value ended with
     "/" (or ":" or "\" on Windows)
   * adjust: Fix some bad behavior when unlocked files use URL keys.
+  * smudge: Fix some bad behavior when git add is run on an unlocked
+    and modified file that used an URL key.
 
  -- Joey Hess <id@joeyh.name>  Mon, 04 Jan 2021 12:52:41 -0400
 

Command/Smudge.hs

@@ -1,6 +1,6 @@
 {- git-annex command
  -
- - Copyright 2015-2019 Joey Hess <id@joeyh.name>
+ - Copyright 2015-2021 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -28,6 +28,7 @@ import Utility.Metered
 import Annex.InodeSentinal
 import Utility.InodeCache
 import Config.GitConfig
+import qualified Types.Backend
 
 import qualified Data.ByteString as S
 import qualified Data.ByteString.Lazy as L
@@ -120,28 +121,31 @@ clean file = do
 			-- Optimization for the case when the file is already
 			-- annexed and is unmodified.
 			case oldkey of
-				Nothing -> doingest oldkey
+				Nothing -> doingest Nothing
 				Just ko -> ifM (isUnmodifiedCheap ko file)
 					( liftIO $ emitPointer ko
-					, doingest oldkey
+					, updateingest ko
 					)
 		, liftIO $ L.hPut stdout b
 		)
 	
-	doingest oldkey = do
-		-- Look up the backend that was used for this file
-		-- before, so that when git re-cleans a file its
-		-- backend does not change.
-		oldbackend <- maybe
-			(pure Nothing)
-			(maybeLookupBackendVariety . fromKey keyVariety)
-			oldkey
+	-- Use the same backend that was used before, when possible.
+	-- If the old key's backend does not support generating keys,
+	-- use the default backend.
+	updateingest oldkey =
+		maybeLookupBackendVariety (fromKey keyVariety oldkey) >>= \case
+			Nothing -> doingest Nothing
+			Just oldbackend -> case Types.Backend.genKey oldbackend of
+				Just _ -> doingest (Just oldbackend)
+				Nothing -> doingest Nothing
+	
+	doingest preferredbackend = do
 		-- Can't restage associated files because git add
 		-- runs this and has the index locked.
 		let norestage = Restage False
 		liftIO . emitPointer
 			=<< postingest
-			=<< (\ld -> ingest' oldbackend nullMeterUpdate ld Nothing norestage)
+			=<< (\ld -> ingest' preferredbackend nullMeterUpdate ld Nothing norestage)
 			=<< lockDown cfg (fromRawFilePath file)
 
 	postingest (Just k, _) = do

doc/bugs/smudge_clean_can_fail_on_URL_key.mdwn

@@ -18,36 +18,4 @@ modify it, and run `git add`.
 
 Since the smudge filter fails, git stages the file content into git.
 
-I investigated fixes for this, but nothing seemed to handle both cases
-well.
-
-One approach is for Command.Smudge to check if genKey is supported
-for the old backend, and if not, use the default backend. This handles the
-git add case, but in the adjust case, the adjusted branch has a file using
-the default backend, which is a change the user didn't intend, and which
-will propagate back to the master branch. 
-
-Or, Command.Smudge could check isUnmodified, and if not, emit the old
-key. But the URL backend can't verify content, so then a modification to
-the file that keeps the size the same, when `git add` is used, will
-generate the same old URL key, which invites data loss.
-
-Maybe have smudge use the default backend, and when adjusting a branch,
-don't unlock files that don't support genKey?
-
-> This seems least bad, although maybe it will be a problem for
-> some users particularly on crippled filesystems. I think anyone who
-> sees a behavior change due to this would have been doing something
-> that would put them at risk for experiencing the bug above, which
-> is worse because it can stage large files into git.. I have an
-> implementation of this in the noadjustURL branch.
-
-Or is there some way to make git-annex adjust checkout the branch
-w/o smudging the files? I don't understand why it does. git-annex
-unlock of an URL key file does not have the problem, so it seems it
-should be possible for checking out the adjusted branch to also avoid the
-problem.
-
-(Or well, could just remove addurl --fast and so remove URL keys...
-Keys that don't support genKey and also don't support verification
-being the root of the problem.)
+> [[fixed|done]] --[[Joey]]