diff --git a/Annex/AdjustedBranch.hs b/Annex/AdjustedBranch.hs index 65f95a13f1..c757eae1dd 100644 --- a/Annex/AdjustedBranch.hs +++ b/Annex/AdjustedBranch.hs @@ -205,15 +205,18 @@ preventCommits = bracket setup cleanup - metadata is based on the parent. -} commitAdjustedTree :: Sha -> Ref -> Annex Sha -commitAdjustedTree treesha parent = go =<< catCommit parent +commitAdjustedTree treesha parent = commitAdjustedTree' treesha parent [parent] + +commitAdjustedTree' :: Sha -> Ref -> [Ref] -> Annex Sha +commitAdjustedTree' treesha basis parents = go =<< catCommit basis where go Nothing = inRepo mkcommit - go (Just parentcommit) = inRepo $ commitWithMetaData - (commitAuthorMetaData parentcommit) - (commitCommitterMetaData parentcommit) + go (Just basiscommit) = inRepo $ commitWithMetaData + (commitAuthorMetaData basiscommit) + (commitCommitterMetaData basiscommit) mkcommit mkcommit = Git.Branch.commitTree Git.Branch.AutomaticCommit - adjustedBranchCommitMessage [parent] treesha + adjustedBranchCommitMessage parents treesha adjustedBranchCommitMessage :: String adjustedBranchCommitMessage = "git-annex adjusted branch" @@ -222,13 +225,14 @@ adjustedBranchCommitMessage = "git-annex adjusted branch" - branch into it. -} updateAdjustedBranch :: Branch -> (OrigBranch, Adjustment) -> Git.Branch.CommitMode -> Annex Bool updateAdjustedBranch tomerge (origbranch, adj) commitmode = catchBoolIO $ - join $ preventCommits $ \_ -> go =<< (,) + join $ preventCommits $ \commitsprevented -> go commitsprevented =<< (,) <$> inRepo (Git.Ref.sha tomerge) <*> inRepo Git.Branch.current where - go (Just mergesha, Just currbranch) = + go commitsprevented (Just mergesha, Just currbranch) = ifM (inRepo $ Git.Branch.changed currbranch mergesha) ( do + void $ propigateAdjustedCommits' origbranch (adj, currbranch) commitsprevented adjustedtomerge <- adjust adj mergesha ifM (inRepo $ Git.Branch.changed currbranch adjustedtomerge) ( return $ @@ -242,24 +246,56 @@ updateAdjustedBranch tomerge (origbranch, adj) commitmode = catchBoolIO $ ) , nochangestomerge ) - go _ = return $ return False + go _ _ = return $ return False nochangestomerge = return $ return True - {- Once a merge commit has been made, re-do it, removing - - the old version of the adjusted branch as a parent, and - - making the only parent be the branch that was merged in. + + {- A merge commit has been made on the adjusted branch. + - Now, re-do it, removing the old version of the adjusted branch + - from its history. - - - Doing this ensures that the same commit Sha is - - always arrived at for a given commit from the merged in branch. - - - Also, update the origbranch. + - There are two possible scenarios; either some commits + - were made on top of the adjusted branch's adjusting commit, + - or not. Those commits have already been propigated to the + - orig branch, so we can just check if there are commits in the + - orig branch that are not present in tomerge. -} - recommit currbranch parent (Just commit) = do - commitsha <- commitAdjustedTree (commitTree commit) parent - inRepo $ Git.Branch.update "updating original branch" origbranch parent - inRepo $ Git.Branch.update "rebasing adjusted branch on top of updated original branch after merge" currbranch commitsha - return True + recommit currbranch mergedsha (Just mergecommit) = + ifM (inRepo $ Git.Branch.changed tomerge origbranch) + ( remerge currbranch mergedsha mergecommit + =<< inRepo (Git.Ref.sha origbranch) + , fastforward currbranch mergedsha mergecommit + ) recommit _ _ Nothing = return False + {- Fast-forward scenario. The mergecommit is changed to a non-merge + - commit, with its parent being the mergedsha. + - The orig branch can simply be pointed at the mergedsha. + -} + fastforward currbranch mergedsha mergecommit = do + commitsha <- commitAdjustedTree (commitTree mergecommit) mergedsha + inRepo $ Git.Branch.update "fast-forward update of adjusted branch" currbranch commitsha + inRepo $ Git.Branch.update "updating original branch" origbranch mergedsha + return True + + {- True merge scenario. -} + remerge currbranch mergedsha mergecommit (Just origsha) = do + -- Update origbranch by reverse adjusting the mergecommit, + -- yielding a merge between orig and tomerge. + treesha <- reverseAdjustedTree origsha adj + -- get 1-parent commit because + -- reverseAdjustedTree does not support merges + =<< commitAdjustedTree (commitTree mergecommit) origsha + revadjcommit <- inRepo $ + Git.Branch.commitTree Git.Branch.AutomaticCommit + ("Merge branch " ++ fromRef tomerge) [origsha, mergedsha] treesha + inRepo $ Git.Branch.update "updating original branch" origbranch revadjcommit + -- Update currbranch, reusing mergedsha, but making its + -- parent be the updated origbranch. + adjcommit <- commitAdjustedTree' (commitTree mergecommit) revadjcommit [revadjcommit] + inRepo $ Git.Branch.update rebaseOnTopMsg currbranch adjcommit + return True + remerge _ _ _ Nothing = return False + {- Check for any commits present on the adjusted branch that have not yet - been propigated to the orig branch, and propigate them. - @@ -268,9 +304,16 @@ updateAdjustedBranch tomerge (origbranch, adj) commitmode = catchBoolIO $ -} propigateAdjustedCommits :: OrigBranch -> (Adjustment, AdjBranch) -> Annex () propigateAdjustedCommits origbranch (adj, currbranch) = - preventCommits $ propigateAdjustedCommits' origbranch (adj, currbranch) - -propigateAdjustedCommits' :: OrigBranch -> (Adjustment, AdjBranch) -> CommitsPrevented -> Annex () + preventCommits $ \commitsprevented -> do + join $ propigateAdjustedCommits' origbranch (adj, currbranch) commitsprevented + +{- Returns action which will rebase the adjusted branch on top of the + - updated orig branch. -} +propigateAdjustedCommits' + :: OrigBranch + -> (Adjustment, AdjBranch) + -> CommitsPrevented + -> Annex (Annex ()) propigateAdjustedCommits' origbranch (adj, currbranch) _commitsprevented = do ov <- inRepo $ Git.Ref.sha (Git.Ref.under "refs/heads" origbranch) case ov of @@ -282,11 +325,11 @@ propigateAdjustedCommits' origbranch (adj, currbranch) _commitsprevented = do case v of Left e -> do warning e - return () - Right newparent -> + return $ return () + Right newparent -> return $ rebase currcommit newparent - Nothing -> return () - Nothing -> return () + Nothing -> return $ return () + Nothing -> return $ return () where newcommits = inRepo $ Git.Branch.changedCommits origbranch currbranch -- Get commits oldest first, so they can be processed @@ -312,41 +355,53 @@ propigateAdjustedCommits' origbranch (adj, currbranch) _commitsprevented = do -- and reparent it on top of the new -- version of the origbranch. commitAdjustedTree (commitTree currcommit) newparent - >>= inRepo . Git.Branch.update "rebasing adjusted branch on top of updated original branch" currbranch + >>= inRepo . Git.Branch.update rebaseOnTopMsg currbranch -{- Reverses an adjusted commit, and commit on top of the provided newparent, +rebaseOnTopMsg :: String +rebaseOnTopMsg = "rebasing adjusted branch on top of updated original branch" + +{- Reverses an adjusted commit, and commit with provided commitparent, - yielding a commit sha. - - - Adjust the tree of the newparent, changing only the files that the + - Adjusts the tree of the commitparent, changing only the files that the - commit changed, and reverse adjusting those changes. - - - Note that the commit message, and the author and committer metadata are - - copied over. However, any gpg signature will be lost, and any other - - headers are not copied either. -} + - The commit message, and the author and committer metadata are + - copied over from the basiscommit. However, any gpg signature + - will be lost, and any other headers are not copied either. -} reverseAdjustedCommit :: Sha -> Adjustment -> (Sha, Commit) -> OrigBranch -> Annex (Either String Sha) -reverseAdjustedCommit newparent adj (csha, c) origbranch - -- commitDiff does not support merge commits - | length (commitParent c) > 1 = return $ +reverseAdjustedCommit commitparent adj (csha, basiscommit) origbranch + | length (commitParent basiscommit) > 1 = return $ Left $ "unable to propigate merge commit " ++ show csha ++ " back to " ++ show origbranch | otherwise = do - (diff, cleanup) <- inRepo (Git.DiffTree.commitDiff csha) - let (adds, others) = partition (\dti -> Git.DiffTree.srcsha dti == nullSha) diff - let (removes, changes) = partition (\dti -> Git.DiffTree.dstsha dti == nullSha) others - adds' <- catMaybes <$> - mapM (adjustTreeItem reverseadj) (map diffTreeToTreeItem adds) - treesha <- Git.Tree.adjustTree - (propchanges changes) - adds' - (map Git.DiffTree.file removes) - newparent - =<< Annex.gitRepo - void $ liftIO cleanup + treesha <- reverseAdjustedTree commitparent adj csha revadjcommit <- inRepo $ commitWithMetaData - (commitAuthorMetaData c) - (commitCommitterMetaData c) $ + (commitAuthorMetaData basiscommit) + (commitCommitterMetaData basiscommit) $ Git.Branch.commitTree Git.Branch.AutomaticCommit - (commitMessage c) [newparent] treesha + (commitMessage basiscommit) [commitparent] treesha return (Right revadjcommit) + +{- Adjusts the tree of the basis, changing only the files that the + - commit changed, and reverse adjusting those changes. + - + - commitDiff does not support merge commits, so the csha must not be a + - merge commit. -} +reverseAdjustedTree :: Sha -> Adjustment -> Sha -> Annex Sha +reverseAdjustedTree basis adj csha = do + (diff, cleanup) <- inRepo (Git.DiffTree.commitDiff csha) + let (adds, others) = partition (\dti -> Git.DiffTree.srcsha dti == nullSha) diff + let (removes, changes) = partition (\dti -> Git.DiffTree.dstsha dti == nullSha) others + adds' <- catMaybes <$> + mapM (adjustTreeItem reverseadj) (map diffTreeToTreeItem adds) + treesha <- Git.Tree.adjustTree + (propchanges changes) + adds' + (map Git.DiffTree.file removes) + basis + =<< Annex.gitRepo + void $ liftIO cleanup + return treesha where reverseadj = reverseAdjustment adj propchanges changes ti@(TreeItem f _ _) = diff --git a/Command/Sync.hs b/Command/Sync.hs index 42484e3bae..135f8b42d3 100644 --- a/Command/Sync.hs +++ b/Command/Sync.hs @@ -243,9 +243,7 @@ commitStaged commitmode commitmessage = do return True mergeLocal :: CurrBranch -> CommandStart -mergeLocal currbranch@(Just branch, madj) = do - proptoorig - go =<< needmerge +mergeLocal currbranch@(Just branch, madj) = go =<< needmerge where syncbranch = syncBranch branch needmerge = ifM isBareRepo @@ -260,11 +258,6 @@ mergeLocal currbranch@(Just branch, madj) = do showStart "merge" $ Git.Ref.describe syncbranch next $ next $ merge currbranch Git.Branch.ManualCommit syncbranch branch' = maybe branch (originalToAdjusted branch) madj - -- When in an adjusted branch, propigate any changes made to it - -- back to the original branch. - proptoorig = case madj of - Just adj -> propigateAdjustedCommits branch (adj, branch') - Nothing -> return () mergeLocal (Nothing, _) = stop pushLocal :: CurrBranch -> CommandStart @@ -274,7 +267,13 @@ pushLocal b = do updateSyncBranch :: CurrBranch -> Annex () updateSyncBranch (Nothing, _) = noop -updateSyncBranch (Just branch, _) = do +updateSyncBranch (Just branch, madj) = do + -- When in an adjusted branch, propigate any changes made to it + -- back to the original branch. + case madj of + Just adj -> propigateAdjustedCommits branch + (adj, originalToAdjusted branch adj) + Nothing -> return () -- Update the sync branch to match the new state of the branch inRepo $ updateBranch (syncBranch branch) branch -- In direct mode, we're operating on some special direct mode diff --git a/doc/design/adjusted_branches.mdwn b/doc/design/adjusted_branches.mdwn index 4d5e409298..f790469c8d 100644 --- a/doc/design/adjusted_branches.mdwn +++ b/doc/design/adjusted_branches.mdwn @@ -109,10 +109,10 @@ beginning the merge. There may be staged changes, or changes in the work tree. First filter the new commit: - origin/master adjusted/master - A - |--------------->A' - | | + origin/master adjusted/master master + A A + |--------------->A' | + | | | | | B | @@ -120,10 +120,10 @@ First filter the new commit: Then, merge that into adjusted/master: - origin/master adjusted/master - A - |--------------->A' - | | + origin/master adjusted/master master + A A + |--------------->A' | + | | | | | B | | | @@ -136,35 +136,13 @@ conflict should only affect the work tree/index, so can be resolved without making a commit, but B'' may end up being made to resolve a merge conflict.) ------- +Once the merge is done, we have a merge commit B'' on adjusted/master. +To finish, redo that commit so it does not have A' as its parent. -TODO FIXME: When an adjusted unlocked branch has gotten a file, and a new -commit is merged in, that does not touch that file, there is a false merge -conflict on the file. It's auto-resolved by creating a .variant file. -This is probably a bug in the auto-resolve code for v6 files. - -Test case: - - git clone ~/lib/tmp - cd tmp - git annex upgrade - git annex adjust - git annex get t/foo - # make change in ~/lib/tmp and commit - git annex sync - # t/foo.variant-* is there - ------- - - - -Once the merge is done, we have a commit B'' on adjusted/master. To finish, -adjust that commit so it does not have adjusted/master as its parent. - - origin/master adjusted/master - A - |--------------->A' - | | + origin/master adjusted/master master + A A + |--------------->A' | + | | | | | B | @@ -172,6 +150,16 @@ adjust that commit so it does not have adjusted/master as its parent. | | Finally, update master, by reverse filtering B''. + + origin/master adjusted/master master + A A + |--------------->A' | + | | | + | | | + B | + | | + |--------------->B'' - - - - - - -> B + | | Notice how similar this is to the commit graph. So, "fast-forward" merging the same B commit from origin/master will lead to an identical @@ -191,6 +179,66 @@ between the adjusted work tree and pulled changes. A post-merge hook would be needed to re-adjust the work tree, and there would be a window where eg, not present files would appear in the work tree.] +## another merge scenario + +Another merge scenario is when there's a new commit C on adjusted/master, +and also a new commit B on origin/master. + +Start by adjusting B': + + origin/master adjusted/master master + A A + |--------------->A' | + | | | + | C' + B + | + |---------->B' + +Then, merge B' into adjusted/master: + + origin/master adjusted/master master + A A + |--------------->A' | + | | | + | C' + B | + | | + |----------->B'->M' + +Here M' is the correct tree, but it has A' as its grandparent, +which is the adjusted branch commit, so needs to be dropped in order to +get a commit that can be put on master. + +We don't want to lose commit C', but it's an adjusted +commit, so needs to be de-adjusted. + + origin/master adjusted/master master + A A + |--------------->A' | + | | | + | C'- - - - - - - - > C + B | + | | + |----------->B'->M' + | + +Now, we generate a merge commit, between B and C, with known result M' +(so no actual merging done here). + + origin/master adjusted/master master + A A + |--------------->A' | + | | | + | C'- - - - - - - - > C + B | + | | + |--------------->M'<-----------------| + | + +Finally, update master, by reverse filtering M'. The resulting commit +on master will also be a merge between B and C. + ## annex object add/remove When objects are added/removed from the annex, the associated file has to @@ -303,20 +351,32 @@ into adjusted view worktrees.] * Honor annex.thin when entering an adjusted branch. * Cloning a repo that has an adjusted branch checked out gets into an ugly state. +* There are potentially races in code that assumes a branch like + master is not being changed by someone else. In particular, + propigateAdjustedCommits rebases the adjusted branch on top of master. + That is called by sync. The assumption is that any changes in master + have already been handled by updateAdjustedBranch. But, if another remote + pushed a new master at just the right time, the adjusted branch could be + rebased on top of a master that it doesn't incorporate, which is wrong. -Bug running git-annex sync in adjusted branch when there is a local change -that gets committed (or already has been), and remote changes available. -Both propigateAdjustedCommits and updateAdjustedBranch -get called in this scenario. Neither order of calling the two works entirely. +------ -The reflog has: +TODO FIXME: When an adjusted unlocked branch has gotten a file, and a new +commit is merged in, that does not touch that file, there is a false merge +conflict on the file. It's auto-resolved by creating a .variant file. +This is probably a bug in the auto-resolve code for v6 files. -d585d7f HEAD@{1}: rebasing adjusted branch on top of updated original branch -e51daec HEAD@{2}: merge f7f2b9f3b1d1c97a1ab24f4a94d4a27d84898992: Merge made by the 'recursive' strategy. -9504e7b HEAD@{3}: rebasing adjusted branch on top of updated original branch -6c6fd41 HEAD@{4}: commit: add +Test case: + + git clone ~/lib/tmp + cd tmp + git annex upgrade + git annex adjust + git annex get t/foo + # make change in ~/lib/tmp and commit + git annex sync + # t/foo.variant-* is there + +------ -e51daec has ok correct history; it gets messed up in d585d7f -Problem is just, that the commit made to the adjusted branch -is left out of the history.