disable journal read optimisation when alwayscommit=false

The journal read optimisation in aeca7c220 later got fixed in eedd73b84
to stage and commit any files that were left in the journal by a
previous git-annex run. That's necessary for the optimisation to work
correctly. But it also meant that alwayscommit=false started committing
the previous git-annex processes journalled changes, which defeated the
purpose of the config setting entirely.

So, disable the optimisation when alwayscommit=false, leaving the
files in the journal and not committing them. See my comments on the bug
report for why this seemed the best approach.

Also fixes a problem when annex.merge-annex-branches=false and there
are changes in the journal. That config indirectly prevents committing
the journal. (Which seems a bit odd given its name, but it always has..)
So, when there were changes in the journal, perhaps left there due to
alwayscommit=false being set before, the optimisation would prevent
git-annex from reading the journal files, and it would operate with out
of date information.

diff --git a/Annex/Branch.hs b/Annex/Branch.hs
index 712c5c10f092995dcf0ca7bbe957616688291116..52e210c54bb39af7d8fdd38a28ee477ded706732 100644 (file)
--- a/Annex/Branch.hs
+++ b/Annex/Branch.hs
@@ -1,6 +1,6 @@
 {- management of the git-annex branch
  -
- - Copyright 2011-2019 Joey Hess <id@joeyh.name>
+ - Copyright 2011-2020 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -14,6 +14,7 @@ module Annex.Branch (
        hasSibling,
        siblingBranches,
        create,
+       UpdateMade(..),
        update,
        forceUpdate,
        updateTo,
@@ -125,12 +126,17 @@ getBranch = maybe (hasOrigin >>= go >>= use) return =<< branchsha
 {- Ensures that the branch and index are up-to-date; should be
  - called before data is read from it. Runs only once per git-annex run. -}
 update :: Annex BranchState
-update = runUpdateOnce $ void $ updateTo =<< siblingBranches
+update = runUpdateOnce $ journalClean <$$> updateTo =<< siblingBranches
 
 {- Forces an update even if one has already been run. -}
-forceUpdate :: Annex Bool
+forceUpdate :: Annex UpdateMade
 forceUpdate = updateTo =<< siblingBranches
 
+data UpdateMade = UpdateMade
+       { refsWereMerged :: Bool
+       , journalClean :: Bool
+       }
+
 {- Merges the specified Refs into the index, if they have any changes not
  - already in it. The Branch names are only used in the commit message;
  - it's even possible that the provided Branches have not been updated to
@@ -149,13 +155,13 @@ forceUpdate = updateTo =<< siblingBranches
  -
  - Returns True if any refs were merged in, False otherwise.
  -}
-updateTo :: [(Git.Sha, Git.Branch)] -> Annex Bool
+updateTo :: [(Git.Sha, Git.Branch)] -> Annex UpdateMade
 updateTo pairs = ifM (annexMergeAnnexBranches <$> Annex.getGitConfig)
        ( updateTo' pairs
-       , return False
+       , return (UpdateMade False False)
        )
 
-updateTo' :: [(Git.Sha, Git.Branch)] -> Annex Bool
+updateTo' :: [(Git.Sha, Git.Branch)] -> Annex UpdateMade
 updateTo' pairs = do
        -- ensure branch exists, and get its current ref
        branchref <- getBranch
@@ -167,7 +173,7 @@ updateTo' pairs = do
                else do
                        mergedrefs <- getMergedRefs
                        filterM isnewer (excludeset mergedrefs unignoredrefs)
-       if null tomerge
+       journalclean <- if null tomerge
                {- Even when no refs need to be merged, the index
                 - may still be updated if the branch has gotten ahead 
                 - of the index, or just if the journal is dirty. -}
@@ -179,11 +185,23 @@ updateTo' pairs = do
                                 - a commit needs to be done. -}
                                when dirty $
                                        go branchref dirty [] jl
-                       , when dirty $
-                               lockJournal $ go branchref dirty []
+                               return True
+                       , if dirty
+                               then ifM (annexAlwaysCommit <$> Annex.getGitConfig)
+                                       ( do
+                                               lockJournal $ go branchref dirty []
+                                               return True
+                                       , return False
+                                       )
+                               else return True
                        )
-               else lockJournal $ go branchref dirty tomerge
-       return $ not $ null tomerge
+               else do
+                       lockJournal $ go branchref dirty tomerge
+                       return True
+       return $ UpdateMade
+               { refsWereMerged = not (null tomerge)
+               , journalClean = journalclean
+               }
   where
        excludeset s = filter (\(r, _) -> S.notMember r s)
        isnewer (r, _) = inRepo $ Git.Branch.changed fullname r

diff --git a/Annex/BranchState.hs b/Annex/BranchState.hs
index 279cfdfebcbaac9a3d4b07f77c217dfb6561df40..d9da6f36563da4feb1bb31f6cf54ac0a1327a0dc 100644 (file)
--- a/Annex/BranchState.hs
+++ b/Annex/BranchState.hs
@@ -28,26 +28,24 @@ checkIndexOnce a = unlessM (indexChecked <$> getState) $ do
        changeState $ \s -> s { indexChecked = True }
 
 {- Runs an action to update the branch, if it's not been updated before
- - in this run of git-annex. -}
-runUpdateOnce :: Annex () -> Annex BranchState
+ - in this run of git-annex. 
+ -
+ - The action should return True if anything that was in the journal
+ - before got staged (or if the journal was empty). That lets an opmisation
+ - be done: The journal then does not need to be checked going forward,
+ - until new information gets written to it.
+ -}
+runUpdateOnce :: Annex Bool -> Annex BranchState
 runUpdateOnce a = do
        st <- getState
        if branchUpdated st
                then return st
                else do
-                       a
+                       journalstaged <- a
                        let stf = \st' -> st'
                                { branchUpdated = True
-                               -- The update staged anything that was
-                               -- journalled before, so the journal
-                               -- does not need to be checked going
-                               -- forward, unless new information
-                               -- gets written to it, or unless
-                               -- this run of git-annex needs to notice
-                               -- changes journalled by other processes
-                               -- while it's running.
-                               , journalIgnorable = not $
-                                       journalNeverIgnorable st'
+                               , journalIgnorable = journalstaged 
+                                       && not (journalNeverIgnorable st')
                                }
                        changeState stf
                        return (stf st)

diff --git a/Assistant/Sync.hs b/Assistant/Sync.hs
index 1bf76c05fcb607e7b4c6e14a245ed9289c4ed6d4..5b19d5216aaa32c37bfa9b14b4edb5956440f74f 100644 (file)
--- a/Assistant/Sync.hs
+++ b/Assistant/Sync.hs
@@ -216,7 +216,8 @@ manualPull currentbranch remotes = do
                                , return $ Just r
                                )
                else return Nothing
-       haddiverged <- liftAnnex Annex.Branch.forceUpdate
+       haddiverged <- Annex.Branch.refsWereMerged
+               <$> liftAnnex Annex.Branch.forceUpdate
        forM_ remotes $ \r ->
                liftAnnex $ Command.Sync.mergeRemote r
                        currentbranch Command.Sync.mergeConfig def

diff --git a/Assistant/Threads/Merger.hs b/Assistant/Threads/Merger.hs
index 49c3956f10eded0ae715f802ab9b61338395cfc7..efeb5c93e1a84823686fda932bf4d6b7f6cc4865 100644 (file)
--- a/Assistant/Threads/Merger.hs
+++ b/Assistant/Threads/Merger.hs
@@ -65,7 +65,8 @@ onChange file
        | ".lock" `isSuffixOf` file = noop
        | isAnnexBranch file = do
                branchChanged
-               diverged <- liftAnnex Annex.Branch.forceUpdate
+               diverged <- Annex.Branch.refsWereMerged
+                       <$> liftAnnex Annex.Branch.forceUpdate
                when diverged $ do
                        updateExportTreeFromLogAll
                        queueDeferredDownloads "retrying deferred download" Later

diff --git a/doc/bugs/commits_created_despite_alwayscommit__61__false_as_of_recent_change.mdwn b/doc/bugs/commits_created_despite_alwayscommit__61__false_as_of_recent_change.mdwn
index 81473de46c5e72e478bf11efa916f42a93f22dbf..faa0b8bdd5d13044550b4d03740de07da64c09fa 100644 (file)
--- a/doc/bugs/commits_created_despite_alwayscommit__61__false_as_of_recent_change.mdwn
+++ b/doc/bugs/commits_created_despite_alwayscommit__61__false_as_of_recent_change.mdwn
@@ -91,3 +91,5 @@ index 712c5c10f..f248b3eb4 100644
 [[!tag projects/datalad]]
 
 [0]: https://github.com/datalad/datalad-extensions/pull/9
+
+> [[fixed|done]] --[[Joey]]