diff options
author | Joey Hess <joey@kitenet.net> | 2014-07-09 15:07:53 -0400 |
---|---|---|
committer | Joey Hess <joey@kitenet.net> | 2014-07-09 15:08:19 -0400 |
commit | 79de0bb85e576b20117ce867ed76aab74815098d (patch) | |
tree | 1610a43fd5cc605aa0b537e67341ac73536b3742 | |
parent | 32a2bf7d2050f8d639e41645069d745f7b7d56a3 (diff) |
prospective fix for bad_merge_commit_deleting_all_files
Assuming my analysis of a race is correct. In any case, this certianly closes a
race..
-rw-r--r-- | Annex/Direct.hs | 35 | ||||
-rw-r--r-- | Locations.hs | 5 | ||||
-rw-r--r-- | debian/changelog | 4 | ||||
-rw-r--r-- | doc/bugs/bad_merge_commit_deleting_all_files.mdwn | 8 |
4 files changed, 44 insertions, 8 deletions
diff --git a/Annex/Direct.hs b/Annex/Direct.hs index 356354aa9..c41f9cf49 100644 --- a/Annex/Direct.hs +++ b/Annex/Direct.hs @@ -5,6 +5,8 @@ - Licensed under the GNU GPL version 3 or higher. -} +{-# LANGUAGE CPP #-} + module Annex.Direct where import Common.Annex @@ -150,13 +152,16 @@ addDirect file cache = do - directory, and the merge is staged into a copy of the index. - Then the work tree is updated to reflect the merge, and - finally, the merge is committed and the real index updated. + - + - A lock file is used to avoid races with any other caller of mergeDirect. + - + - To avoid other git processes from making change to the index while our + - merge is in progress, the index lock file is used as the temp index + - file. This is the same as what git does when updating the index + - normally. -} mergeDirect :: Maybe Git.Ref -> Maybe Git.Ref -> Git.Branch -> Annex Bool -> Git.Branch.CommitMode -> Annex Bool -mergeDirect startbranch oldref branch resolvemerge commitmode = do - -- Use the index lock file as the temp index file. - -- This is actually what git does when updating the index, - -- and so it will prevent other git processes from making - -- any changes to the index while our merge is in progress. +mergeDirect startbranch oldref branch resolvemerge commitmode = lockMerge $ do reali <- fromRepo indexFile tmpi <- fromRepo indexFileLock liftIO $ copyFile reali tmpi @@ -174,9 +179,29 @@ mergeDirect startbranch oldref branch resolvemerge commitmode = do else resolvemerge mergeDirectCleanup d (fromMaybe Git.Sha.emptyTree oldref) mergeDirectCommit merged startbranch branch commitmode + liftIO $ rename tmpi reali + return r +lockMerge :: Annex a -> Annex a +lockMerge a = do + lockfile <- fromRepo gitAnnexMergeLock + createAnnexDirectory $ takeDirectory lockfile + mode <- annexFileMode + bracketIO (lock lockfile mode) unlock (const a) + where +#ifndef mingw32_HOST_OS + lock lockfile mode = do + l <- noUmask mode $ createFile lockfile mode + waitToSetLock l (WriteLock, AbsoluteSeek, 0, 0) + return l + unlock = closeFd +#else + lock lockfile _mode = waitToLock $ lockExclusive lockfile + unlock = dropLock +#endif + {- Stage a merge into the index, avoiding changing HEAD or the current - branch. -} stageMerge :: FilePath -> Git.Branch -> Git.Branch.CommitMode -> Annex Bool diff --git a/Locations.hs b/Locations.hs index 95aba169c..d397a97be 100644 --- a/Locations.hs +++ b/Locations.hs @@ -42,6 +42,7 @@ module Locations ( gitAnnexJournalDir, gitAnnexJournalLock, gitAnnexPreCommitLock, + gitAnnexMergeLock, gitAnnexIndex, gitAnnexIndexStatus, gitAnnexViewIndex, @@ -262,6 +263,10 @@ gitAnnexJournalLock r = gitAnnexDir r </> "journal.lck" gitAnnexPreCommitLock :: Git.Repo -> FilePath gitAnnexPreCommitLock r = gitAnnexDir r </> "precommit.lck" +{- Lock file for direct mode merge. -} +gitAnnexMergeLock :: Git.Repo -> FilePath +gitAnnexMergeLock r = gitAnnexDir r </> "merge.lck" + {- .git/annex/index is used to stage changes to the git-annex branch -} gitAnnexIndex :: Git.Repo -> FilePath gitAnnexIndex r = gitAnnexDir r </> "index" diff --git a/debian/changelog b/debian/changelog index 6d151ee08..294e404df 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,10 @@ git-annex (5.20140708) UNRELEASED; urgency=medium * Fix git version that supported --no-gpg-sign. * Fix bug in automatic merge conflict resolution, when one side is an annexed symlink, and the other side is a non-annexed symlink. + * Fix race in direct mode merge code that could cause all files in the + repository to be removed. It should be able to recover repositories + experiencing this bug without data loss. See: + http://git-annex.branchable.com/bugs/bad_merge_commit_deleting_all_files/ -- Joey Hess <joeyh@debian.org> Tue, 08 Jul 2014 12:44:42 -0400 diff --git a/doc/bugs/bad_merge_commit_deleting_all_files.mdwn b/doc/bugs/bad_merge_commit_deleting_all_files.mdwn index efcc39553..6cfb27002 100644 --- a/doc/bugs/bad_merge_commit_deleting_all_files.mdwn +++ b/doc/bugs/bad_merge_commit_deleting_all_files.mdwn @@ -27,9 +27,11 @@ redo, passing `-m 2` instead.) Once the revert is done, all your files should be back. You can switch the repository back to direct mode if desired (`git annex direct`) and can `git annex sync` to push the fix out to all other clones. -Everywhere the fix lands should restore the deleted files. (Although -it's possible that some repositories may have pruned the deleted -files as unused.) +Everywhere the fix lands should restore the deleted files. + +Note that it's possible that some repositories may have pruned the deleted +files as unused. This is most likely to happen in the repository that made +the bad merge commit in the first place. [[!tag confirmed urgent]] |