diff --git a/Annex/Url.hs b/Annex/Url.hs index 863234544f..f76c29516b 100644 --- a/Annex/Url.hs +++ b/Annex/Url.hs @@ -19,7 +19,6 @@ module Annex.Url ( download', exists, getUrlInfo, - U.downloadQuiet, U.URLString, U.UrlOptions(..), U.UrlInfo(..), diff --git a/CHANGELOG b/CHANGELOG index 61aed958ae..26b42f609f 100644 --- a/CHANGELOG +++ b/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 Fri, 12 Mar 2021 12:06:37 -0400 diff --git a/Remote/Git.hs b/Remote/Git.hs index 8cf05f53cc..b2f49593d8 100644 --- a/Remote/Git.hs +++ b/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 diff --git a/Utility/Url.hs b/Utility/Url.hs index b8a1735630..4d385508eb 100644 --- a/Utility/Url.hs +++ b/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 diff --git a/doc/bugs/http_remote_on_local_network_bad_error_message.mdwn b/doc/bugs/http_remote_on_local_network_bad_error_message.mdwn index f9ac048a6d..832df35a60 100644 --- a/doc/bugs/http_remote_on_local_network_bad_error_message.mdwn +++ b/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]]