From adb09117f1f9e5bc79100536f4b1a903d0608886 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 5 Jul 2023 17:01:39 -0400 Subject: [PATCH] propigateAdjustedCommits: avoid overwriting diverged original branch Bug fix: Re-running git-annex adjust or sync when in an adjusted branch would overwrite the original branch, losing any commits that had been made to it since the adjusted branch was created. When git-annex adjust is run in this situation, it will display a warning about the diverged branches. When git-annex sync is run in this situation, mergeToAdjustedBranch will merge the changes from the original branch to the adjusted branch. So it does not need to display the divergence warning. Note that for some reason, I'm needing to run sync twice for that to happen. The first run does not do the merge and the second does. I'm unsure why and so am not fully done with this bug. Sponsored-By: the NIH-funded NICEMAN (ReproNim TR&D3) project --- Annex/AdjustedBranch.hs | 30 ++++++++++------- Annex/AdjustedBranch/Merge.hs | 2 +- CHANGELOG | 3 ++ ..._84346dcb210b86e86bd474820bb5f494._comment | 33 +++++++++++++++++++ ..._c420c76e5aebb1339f4c478b4d56d5de._comment | 11 +++++++ 5 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_1_84346dcb210b86e86bd474820bb5f494._comment create mode 100644 doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_2_c420c76e5aebb1339f4c478b4d56d5de._comment diff --git a/Annex/AdjustedBranch.hs b/Annex/AdjustedBranch.hs index 9d983af27b..a7b6f03ed7 100644 --- a/Annex/AdjustedBranch.hs +++ b/Annex/AdjustedBranch.hs @@ -258,7 +258,7 @@ updateAdjustedBranch adj (AdjBranch currbranch) origbranch -- Avoid losing any commits that the adjusted branch -- has that have not yet been propigated back to the -- origbranch. - _ <- propigateAdjustedCommits' origbranch adj commitlck + _ <- propigateAdjustedCommits' True origbranch adj commitlck origheadfile <- inRepo $ readFileStrict . Git.Ref.headFile origheadsha <- inRepo (Git.Ref.sha currbranch) @@ -294,7 +294,7 @@ updateAdjustedBranch adj (AdjBranch currbranch) origbranch return ok | otherwise = preventCommits $ \commitlck -> do -- Done for consistency. - _ <- propigateAdjustedCommits' origbranch adj commitlck + _ <- propigateAdjustedCommits' True origbranch adj commitlck -- No need to actually update the branch because the -- adjustment is stable. return True @@ -495,20 +495,21 @@ findAdjustingCommit (AdjBranch b) = go =<< catCommit b propigateAdjustedCommits :: OrigBranch -> Adjustment -> Annex () propigateAdjustedCommits origbranch adj = preventCommits $ \commitsprevented -> - join $ snd <$> propigateAdjustedCommits' origbranch adj commitsprevented + join $ snd <$> propigateAdjustedCommits' True origbranch adj commitsprevented {- Returns sha of updated basis branch, and action which will rebase - the adjusted branch on top of the updated basis branch. -} propigateAdjustedCommits' - :: OrigBranch + :: Bool + -> OrigBranch -> Adjustment -> CommitsPrevented -> Annex (Maybe Sha, Annex ()) -propigateAdjustedCommits' origbranch adj _commitsprevented = +propigateAdjustedCommits' warnwhendiverged origbranch adj _commitsprevented = inRepo (Git.Ref.sha basis) >>= \case Just origsha -> catCommit currbranch >>= \case Just currcommit -> - newcommits >>= go origsha False >>= \case + newcommits >>= go origsha origsha False >>= \case Left e -> do warning (UnquotedString e) return (Nothing, return ()) @@ -528,20 +529,25 @@ propigateAdjustedCommits' origbranch adj _commitsprevented = -- Get commits oldest first, so they can be processed -- in order made. [Param "--reverse"] - go parent _ [] = do + go origsha parent _ [] = do setBasisBranch (BasisBranch basis) parent - inRepo $ Git.Branch.update' origbranch parent + inRepo (Git.Ref.sha origbranch) >>= \case + Just origbranchsha | origbranchsha /= origsha -> + when warnwhendiverged $ + warning $ UnquotedString $ + "Original branch " ++ fromRef origbranch ++ " has diverged from current adjusted branch " ++ fromRef currbranch + _ -> inRepo $ Git.Branch.update' origbranch parent return (Right parent) - go parent pastadjcommit (sha:l) = catCommit sha >>= \case + go origsha parent pastadjcommit (sha:l) = catCommit sha >>= \case Just c | commitMessage c == adjustedBranchCommitMessage -> - go parent True l + go origsha parent True l | pastadjcommit -> reverseAdjustedCommit parent adj (sha, c) origbranch >>= \case Left e -> return (Left e) - Right commit -> go commit pastadjcommit l - _ -> go parent pastadjcommit l + Right commit -> go origsha commit pastadjcommit l + _ -> go origsha parent pastadjcommit l rebase currcommit newparent = do -- Reuse the current adjusted tree, and reparent it -- on top of the newparent. diff --git a/Annex/AdjustedBranch/Merge.hs b/Annex/AdjustedBranch/Merge.hs index f9c4bd5d43..065bd5f855 100644 --- a/Annex/AdjustedBranch/Merge.hs +++ b/Annex/AdjustedBranch/Merge.hs @@ -46,7 +46,7 @@ mergeToAdjustedBranch tomerge (origbranch, adj) mergeconfig canresolvemerge comm ifM (inRepo $ Git.Branch.changed currbranch tomerge) ( do (updatedorig, _) <- propigateAdjustedCommits' - origbranch adj commitsprevented + False origbranch adj commitsprevented changestomerge updatedorig , nochangestomerge ) diff --git a/CHANGELOG b/CHANGELOG index b3d0e06ef7..b0ca2f620d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,9 @@ git-annex (10.20230627) UNRELEASED; urgency=medium annexed text files. * assist: With --jobs, parallelize transferring content to/from remotes. * Fix breakage when git is configured with safe.bareRepository = explicit. + * Bug fix: Re-running git-annex adjust or sync when in an adjusted branch + would overwrite the original branch, losing any commits that had been + made to it since the adjusted branch was created. -- Joey Hess Mon, 26 Jun 2023 13:10:40 -0400 diff --git a/doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_1_84346dcb210b86e86bd474820bb5f494._comment b/doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_1_84346dcb210b86e86bd474820bb5f494._comment new file mode 100644 index 0000000000..e648393674 --- /dev/null +++ b/doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_1_84346dcb210b86e86bd474820bb5f494._comment @@ -0,0 +1,33 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 1""" + date="2023-07-05T19:49:19Z" + content=""" +Simplified test case: + + git init tc + cd tc + git-annex init + echo 1 > foo + git-annex add + git commit -m add + git annex adjust --unlock + git checkout master + rm foo + echo 2 > foo + git-annex add + git commit -m "this commit will be lost" + git checkout 'adjusted/master(unlocked)' + git annex adjust --unlock # or git-annex sync + git log master + +What an unfortunate oversight! And it's not a reversion, it's been there +since the beginning of adjusted branches. + +git-annex adjust should display a warning message in that situation, +since the original branch has diverged from the adjusted branch. + +And git-annex sync should be able to resolve the divergence by +auto-merging the changes from the original branch into the adjusted +branch. +"""]] diff --git a/doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_2_c420c76e5aebb1339f4c478b4d56d5de._comment b/doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_2_c420c76e5aebb1339f4c478b4d56d5de._comment new file mode 100644 index 0000000000..280b9b318e --- /dev/null +++ b/doc/bugs/annex_sync_silently_resets_master_to_previous_sync/comment_2_c420c76e5aebb1339f4c478b4d56d5de._comment @@ -0,0 +1,11 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 2""" + date="2023-07-05T21:01:53Z" + content=""" +I've fixed the data loss part of this bug. + +`git-annex sync` is able to resolve the divergence too. But for some +reason, the first time it's run after the divergence, it leaves it +diverged, and the second time it resolves it. That needs to be fixed. +"""]]