From 76bc6f4b653a9dc94709fc3ee511cc877c487ee1 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 4 Aug 2015 14:01:59 -0400 Subject: proxy: Fix proxy git commit of non-annexed files in direct mode. * proxy: Fix proxy git commit of non-annexed files in direct mode. * proxy: If a non-proxied git command, such as git revert would normally fail because of unstaged files in the work tree, make the proxied command fail the same way. --- Annex/Direct.hs | 10 +++--- Command/Proxy.hs | 39 +++++++++++++++++++++- debian/changelog | 4 +++ ...0__original_file_is_even_get_removed__41__.mdwn | 27 +++++++++++++++ doc/git-annex-proxy.mdwn | 8 +++++ 5 files changed, 82 insertions(+), 6 deletions(-) diff --git a/Annex/Direct.hs b/Annex/Direct.hs index bb470e04c..d88dc43fb 100644 --- a/Annex/Direct.hs +++ b/Annex/Direct.hs @@ -230,7 +230,7 @@ mergeDirectCommit allowff old branch commitmode = do mergeDirectCleanup :: FilePath -> Git.Ref -> Annex () mergeDirectCleanup d oldref = do - updateWorkTree d oldref + updateWorkTree d oldref False liftIO $ removeDirectoryRecursive d {- Updates the direct mode work tree to reflect the changes staged in the @@ -247,8 +247,8 @@ mergeDirectCleanup d oldref = do - order, but we cannot add the directory until the file with the - same name is removed.) -} -updateWorkTree :: FilePath -> Git.Ref -> Annex () -updateWorkTree d oldref = do +updateWorkTree :: FilePath -> Git.Ref -> Bool -> Annex () +updateWorkTree d oldref force = do (items, cleanup) <- inRepo $ DiffTree.diffIndex oldref makeabs <- flip fromTopFilePath <$> gitRepo let fsitems = zip (map (makeabs . DiffTree.file) items) items @@ -281,7 +281,7 @@ updateWorkTree d oldref = do - Otherwise, create the symlink and then if possible, replace it - with the content. -} movein item makeabs k f = unlessM (goodContent k f) $ do - preserveUnannexed item makeabs f oldref + unless force $ preserveUnannexed item makeabs f oldref l <- calcRepo $ gitAnnexLink f k replaceFile f $ makeAnnexLink l toDirect k f @@ -289,7 +289,7 @@ updateWorkTree d oldref = do {- Any new, modified, or renamed files were written to the temp - directory by the merge, and are moved to the real work tree. -} movein_raw item makeabs f = do - preserveUnannexed item makeabs f oldref + unless force $ preserveUnannexed item makeabs f oldref liftIO $ do createDirectoryIfMissing True $ parentDir f void $ tryIO $ rename (d getTopFilePath (DiffTree.file item)) f diff --git a/Command/Proxy.hs b/Command/Proxy.hs index 3c487b9b5..97cfafeaf 100644 --- a/Command/Proxy.hs +++ b/Command/Proxy.hs @@ -13,9 +13,13 @@ import Config import Utility.Tmp import Utility.Env import Annex.Direct +import qualified Git import qualified Git.Sha import qualified Git.Ref import qualified Git.Branch +import qualified Git.LsFiles +import Git.FilePath +import Utility.CopyFile cmd :: Command cmd = notBareRepo $ @@ -38,12 +42,45 @@ start (c:ps) = liftIO . exitWith =<< ifM isDirect go tmp = do oldref <- fromMaybe Git.Sha.emptyTree <$> inRepo Git.Ref.headSha + + setuptmpworktree tmp exitcode <- proxy tmp - mergeDirectCleanup tmp oldref + cleanupproxy tmp oldref + return exitcode + proxy tmp = do usetmp <- liftIO $ Just . addEntry "GIT_WORK_TREE" tmp <$> getEnvironment unlessM (isNothing <$> inRepo Git.Branch.current) $ unlessM (liftIO $ boolSystemEnv "git" [Param "checkout", Param "--", Param "."] usetmp) $ error "Failed to set up proxy work tree." liftIO $ safeSystemEnv c (map Param ps) usetmp + + -- Commands like git revert will fail if there's a file + -- in the work tree, or index, that would be overwritten + -- by the revert. We want that to also happen when such a command + -- is proxied. + -- + -- It suffices to find any files in the real work tree that + -- are not in the index, and hard link (or copy) them + -- into the tmp work tree. This assumes that files that are in the + -- index don't need to appear in the tmp work tree. + setuptmpworktree tmp = do + top <- fromRepo Git.repoPath + (fs, cleanup) <- inRepo $ Git.LsFiles.notInRepo True [top] + forM_ fs $ \f -> do + tf <- inRepo $ toTopFilePath f + let tmpf = tmp getTopFilePath tf + liftIO $ do + createDirectoryIfMissing True (takeDirectory tmpf) + createLinkOrCopy f tmpf + liftIO $ void cleanup + + -- To merge the changes made by the proxied command into + -- the work tree is similar to cleaning up after a + -- direct mode merge. But, here we force updates of any + -- non-annxed files that were changed by the proxied + -- command. + cleanupproxy tmp oldref = do + updateWorkTree tmp oldref True + liftIO $ removeDirectoryRecursive tmp diff --git a/debian/changelog b/debian/changelog index ba76971f2..3470f5ce4 100644 --- a/debian/changelog +++ b/debian/changelog @@ -11,6 +11,10 @@ git-annex (5.20150732) UNRELEASED; urgency=medium commits were made every 1000 files fscked. * Tighten dependency on optparse-applicative to 0.11.0. * Added back debian/cabal-wrapper, since it still seems needed after all. + * proxy: Fix proxy git commit of non-annexed files in direct mode. + * proxy: If a non-proxied git command, such as git revert + would normally fail because of unstaged files in the work tree, + make the proxied command fail the same way. -- Joey Hess Fri, 31 Jul 2015 12:31:39 -0400 diff --git a/doc/bugs/proxy_--_git_commit__for_a_file_under_git__creates_.variant-local___40__original_file_is_even_get_removed__41__.mdwn b/doc/bugs/proxy_--_git_commit__for_a_file_under_git__creates_.variant-local___40__original_file_is_even_get_removed__41__.mdwn index b145f6fc0..d104dc91a 100644 --- a/doc/bugs/proxy_--_git_commit__for_a_file_under_git__creates_.variant-local___40__original_file_is_even_get_removed__41__.mdwn +++ b/doc/bugs/proxy_--_git_commit__for_a_file_under_git__creates_.variant-local___40__original_file_is_even_get_removed__41__.mdwn @@ -108,3 +108,30 @@ Date: Tue Jul 28 17:20:29 2015 -0400 # End of transcript or log. """]] + +> Ok, this is [[fixed|done]]. +> +> What was going on is, proxy was reusing mergeDirectCleanup +> since it's in a similar situation to cleaning up after a direct mode +> merge. But, a direct mode merge can pull in changes to files that exist +> in the local work tree (and may or may not be in the index), but are +> not committed to git locally yet. So, it has to +> detect those and move them aside (to ".varient-local"). The code to do +> that is what was failing in this reuse of mergeDirectCleanup. +> +> So, I made that code path not run when using proxy. And for commits, +> that's good enough. If there's a file in the work tree that's +> not added to git, then a proxied commit can't affect it, so that code +> path is not needed in this case. +> +> Come to think, other proxied actions might affect such a file. For +> example a proxied revert could revert the deletion of a file with the +> same name, that's in the work tree. In this case, should the proxyed revert +> fail because there's a file in the work tree that will be overwritten by +> the revert? Would be good if it did, because git revert will normally fail +> in this situation. +> +> The only way to make a proxied revert, etc exactly match a +> non-proxied revert is to arrange for all files that are in the work +> tree and not checked into git to be present in the temp work tree when +> the proxied command is run. Which I've now done. --[[Joey]] diff --git a/doc/git-annex-proxy.mdwn b/doc/git-annex-proxy.mdwn index 570789cf7..814cc7676 100644 --- a/doc/git-annex-proxy.mdwn +++ b/doc/git-annex-proxy.mdwn @@ -31,6 +31,14 @@ stage the changes in the index, and then proxy a commit: git annex add myfile git annex proxy -- git commit myfile -m foo +Note that git annex proxy cannot be usefully used with git commands that +look at work tree files. For example, it doesn't make sense to proxy "git +add". This is because the temporary work tree used for proxying doesn't +contain all the files that are in the real work tree. However, any unstaged +work tree files are hard linked (or copied) into the temporary work tree, +so that a command like git revert, that will fail if the change it's +making overwrites work tree files, will behave the same when proxied. + # SEE ALSO [[git-annex]](1) -- cgit v1.2.3