sync: Improve integration with receive.denyCurrentBranch=updateInstead

By displaying error messages from the remote then it fails to update
its checked out branch.

Error messages in the default receive.denyCurrentBranch are still
suppressed, which matches user expectations.

This commit was sponsored by Nick Daly on Patreon.

3 files changed, 45 insertions, 24 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7fbbf01b2..456b12981 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,9 @@ git-annex (6.20170215) UNRELEASED; urgency=medium
     git-annex commands that communicate with a remote over ssh may also
     have been consuming stdin that they shouldn't have, which could have
     impacted using them in eg, shell scripts.
+  * sync: Improve integration with receive.denyCurrentBranch=updateInstead,
+    displaying error messages from the remote then it fails to update
+    its checked out branch.
 
  -- Joey Hess <id@joeyh.name>  Tue, 14 Feb 2017 15:54:25 -0400
 
diff --git a/Command/Sync.hs b/Command/Sync.hs
index d095ae20a..ca6e86271 100644
--- a/Command/Sync.hs
+++ b/Command/Sync.hs
@@ -389,18 +389,20 @@ pushRemote o remote (Just branch, _) = stopUnless (pure (pushOption o) <&&> need
 {- Pushes a regular branch like master to a remote. Also pushes the git-annex
  - branch.
  -
- - If the remote is a bare git repository, it's best to push the regular 
+ - If the remote is a bare git repository, it's best to push the regular
  - branch directly to it, so that cloning/pulling will get it.
  - On the other hand, if it's not bare, pushing to the checked out branch
- - will fail, and this is why we push to its syncBranch.
+ - will generally fail (except with receive.denyCurrentBranch=updateInstead),
+ - and this is why we push to its syncBranch.
  -
  - Git offers no way to tell if a remote is bare or not, so both methods
  - are tried.
  -
- - The direct push is likely to spew an ugly error message, so stderr is
- - elided. Since git progress display goes to stderr too, the sync push
- - is done first, and actually sends the data. Then the direct push is
- - tried, with stderr discarded, to update the branch ref on the remote.
+ - The direct push is likely to spew an ugly error message, so its stderr is
+ - often elided. Since git progress display goes to stderr too, the 
+ - sync push is done first, and actually sends the data. Then the
+ - direct push is tried, with stderr discarded, to update the branch ref
+ - on the remote.
  -
  - The sync push forces the update of the remote synced/git-annex branch.
  - This is necessary if a transition has rewritten the git-annex branch.
@@ -416,16 +418,30 @@ pushRemote o remote (Just branch, _) = stopUnless (pure (pushOption o) <&&> need
  - set on the remote.
  -}
 pushBranch :: Remote -> Git.Branch -> Git.Repo -> IO Bool
-pushBranch remote branch g = tryIO (directpush g) `after` syncpush g
+pushBranch remote branch g = directpush `after` syncpush
   where
-	syncpush = Git.Command.runBool $ pushparams
+	syncpush = flip Git.Command.runBool g $ pushparams
 		[ Git.Branch.forcePush $ refspec Annex.Branch.name
 		, refspec $ fromAdjustedBranch branch
 		]
-	directpush = Git.Command.runQuiet $ pushparams
-		[ Git.fromRef $ Git.Ref.base $ Annex.Branch.name
-		, Git.fromRef $ Git.Ref.base $ fromDirectBranch $ fromAdjustedBranch branch
-		]
+	directpush = do
+		-- Git prints out an error message when this fails.
+		-- In the default configuration of receive.denyCurrentBranch,
+		-- the error message mentions that config setting
+		-- (and should even if it is localized), and is quite long,
+		-- and the user was not intending to update the checked out
+		-- branch, so in that case, avoid displaying the error
+		-- message. Do display other error messages though,
+		-- including the error displayed when
+		-- receive.denyCurrentBranch=updateInstead -- the user
+		-- will want to see that one.
+		let p = flip Git.Command.gitCreateProcess g $ pushparams
+			[ Git.fromRef $ Git.Ref.base $ Annex.Branch.name
+			, Git.fromRef $ Git.Ref.base $ fromDirectBranch $ fromAdjustedBranch branch
+			]
+		(transcript, ok) <- processTranscript' p Nothing
+		when (not ok && not ("denyCurrentBranch" `isInfixOf` transcript)) $
+			hPutStr stderr transcript
 	pushparams branches =
 		[ Param "push"
 		, Param $ Remote.name remote
diff --git a/doc/todo/simpler_setup_for_post-update_merge.mdwn b/doc/todo/simpler_setup_for_post-update_merge.mdwn
index 27bace3ec..8d25f44b9 100644
--- a/doc/todo/simpler_setup_for_post-update_merge.mdwn
+++ b/doc/todo/simpler_setup_for_post-update_merge.mdwn
@@ -25,18 +25,20 @@ filesystems, and on Windows.)
 
 Alternatively, receive.denyCurrentBranch can be set to updateInstead.
 With this configuration, `git annex sync` automatically updates the
-work-tree of the remote already.
-
-However, any differences in the remote's work tree or index (other than new
-unstaged files) prevent the work tree update. Currently, `git annex sync`
-does the master:master push quietly, because in standard non-bare repos
-that's expected to fail. So, failures to update the remote work tree
-won't be noticed. (`git annex sync` could look at the remote's
-receive.denyCurrentBranch setting, but this would be hard to do for ssh
-remotes).
-
-Also, this wouldn't work for direct mode repositories, which are often used
-on removable drives.
+work-tree of the remote already. 
+
+This wouldn't work for direct mode repositories, which are often used
+on removable drives, since git thinks they're bare repos.
+
+Nor will it work for adjusted branches, since the adjusted branch is not
+updated by the push.
+
+---
+
+Could the updateInstead configuration be made to work for direct mode and
+adjusted branches? Could install a post-update hook, that runs a git-annex
+command that checks for updateInstead, and emulates its behavior, handling
+direct mode and adjusted branches.
 
 ---