From 47338bf2708e35c5433e5b862ffe643c34b80c72 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 25 Jan 2021 17:34:58 -0400 Subject: [PATCH] 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 | 2 ++ Command/Smudge.hs | 28 ++++++++------- .../smudge_clean_can_fail_on_URL_key.mdwn | 34 +------------------ 3 files changed, 19 insertions(+), 45 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b91d85abe2..061dd6b4ce 100644 --- a/CHANGELOG +++ b/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 Mon, 04 Jan 2021 12:52:41 -0400 diff --git a/Command/Smudge.hs b/Command/Smudge.hs index 6286879634..a092b6ad68 100644 --- a/Command/Smudge.hs +++ b/Command/Smudge.hs @@ -1,6 +1,6 @@ {- git-annex command - - - Copyright 2015-2019 Joey Hess + - Copyright 2015-2021 Joey Hess - - 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 diff --git a/doc/bugs/smudge_clean_can_fail_on_URL_key.mdwn b/doc/bugs/smudge_clean_can_fail_on_URL_key.mdwn index 4a16a4bd24..2237c7051b 100644 --- a/doc/bugs/smudge_clean_can_fail_on_URL_key.mdwn +++ b/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]]