Improved display of errors when accessing a git http remote fails.

New error message:

  Remote foo not usable by git-annex; setting annex-ignore

  http://localhost/foo/config download failed: Configuration of annex.security.allowed-ip-addresses does not allow accessing address ::1

If git config parse fails, or the git config file is not available at the url,
a better error message for that is also shown.

This commit was sponsored by Mark Reidenbach on Patreon.

Annex/Url.hs

@@ -19,7 +19,6 @@ module Annex.Url (
 	download',
 	exists,
 	getUrlInfo,
-	U.downloadQuiet,
 	U.URLString,
 	U.UrlOptions(..),
 	U.UrlInfo(..),

CHANGELOG

@@ -25,6 +25,7 @@ git-annex (8.20210311) UNRELEASED; urgency=medium
     some expensive things needed to support unlocked files.
   * Sped up git-annex init in a clone of an existing repository.
   * Fix build with attoparsec-0.14.
+  * Improved display of errors when accessing a git http remote fails.
 
  -- Joey Hess <id@joeyh.name>  Fri, 12 Mar 2021 12:06:37 -0400
 

Remote/Git.hs

@@ -260,7 +260,9 @@ tryGitConfigRead autoinit r hasuuid
 	| haveconfig r = return r -- already read
 	| Git.repoIsSsh r = storeUpdatedRemote $ do
 		v <- Ssh.onRemote NoConsumeStdin r
-			(pipedconfig Git.Config.ConfigList autoinit (Git.repoDescribe r), return (Left $ giveup "configlist failed"))
+			( pipedconfig Git.Config.ConfigList autoinit (Git.repoDescribe r)
+			, return (Left "configlist failed")
+			)
 			"configlist" [] configlistfields
 		case v of
 			Right r'
@@ -289,25 +291,32 @@ tryGitConfigRead autoinit r hasuuid
 				return $ Right r'
 			Left l -> do
 				warning $ "Unable to parse git config from " ++ configloc
-				return $ Left l
+				return $ Left (show l)
 
 	geturlconfig = Url.withUrlOptionsPromptingCreds $ \uo -> do
+		let url = Git.repoLocation r ++ "/config"
 		v <- withTmpFile "git-annex.tmp" $ \tmpfile h -> do
 			liftIO $ hClose h
-			let url = Git.repoLocation r ++ "/config"
-			ifM (liftIO $ Url.downloadQuiet nullMeterUpdate url tmpfile uo)
-				( Just <$> pipedconfig Git.Config.ConfigNullList False url "git" [Param "config", Param "--null", Param "--list", Param "--file", File tmpfile]
-				, return Nothing
-				)
+			Url.download' nullMeterUpdate url tmpfile uo >>= \case
+				Right () -> pipedconfig Git.Config.ConfigNullList
+					False url "git"
+					[ Param "config"
+					, Param "--null"
+					, Param "--list"
+					, Param "--file"
+					, File tmpfile
+					]
+				Left err -> return (Left err)
 		case v of
-			Just (Right r') -> do
+			Right r' -> do
 				-- Cache when http remote is not bare for
 				-- optimisation.
 				unless (Git.Config.isBare r') $
 					setremote setRemoteBare False
 				return r'
-			_ -> do
+			Left err -> do
 				set_ignore "not usable by git-annex" False
+				warning $ url ++ " " ++ err
 				return r
 
 	{- Is this remote just not available, or does

Utility/Url.hs

@@ -30,7 +30,6 @@ module Utility.Url (
 	getUrlInfo,
 	assumeUrlExists,
 	download,
-	downloadQuiet,
 	downloadConduit,
 	sinkResponseFile,
 	downloadPartial,
@@ -364,11 +363,6 @@ headRequest r = r
 download :: MeterUpdate -> URLString -> FilePath -> UrlOptions -> IO (Either String ())
 download = download' False
 
-{- Avoids displaying any error message, including silencing curl errors. -}
-downloadQuiet :: MeterUpdate -> URLString -> FilePath -> UrlOptions -> IO Bool
-downloadQuiet meterupdate url file uo = isRight 
-	<$> download' True meterupdate url file uo
-
 download' :: Bool -> MeterUpdate -> URLString -> FilePath -> UrlOptions -> IO (Either String ())
 download' nocurlerror meterupdate url file uo =
 	catchJust matchHttpException go showhttpexception

doc/bugs/http_remote_on_local_network_bad_error_message.mdwn

@@ -7,3 +7,12 @@ There needs to be a better error message, or possibly don't apply
 annex.security.allowed-ip-addresses to uuid discovery -- which does not
 seem like it would involve the kind of security problem that config exists
 to prevent. --[[Joey]]
+
+> While I think it would be safe to bypass
+> annex.security.allowed-ip-addresses in this case, it seemed complicated
+> to implement that, and also getting that wrong would be a security hole.
+> Since this is a pretty unusual case, I think it's ok to need that to be
+> set.
+> 
+> So, I've improved the error message in this case. (And a few other
+> cases.) [[done]] --[[Joey]]