better error message when git config fails to parse remote config

Rather than leaking the name of the temp file, just say the config parse
failed, and where the config was downloaded from.

Not closing the bug report because two issues were reported in the same
bug report, because the universe wants me to continually re-read old
unclosed bug reports to waste my time determining what still needs to be
done.
This commit is contained in:
Joey Hess 2020-01-22 13:20:06 -04:00
parent 6cc1f1dce9
commit 75059c9f3b
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
5 changed files with 30 additions and 14 deletions

View file

@ -1,6 +1,6 @@
{- git repository configuration handling
-
- Copyright 2010-2019 Joey Hess <id@joeyh.name>
- Copyright 2010-2020 Joey Hess <id@joeyh.name>
-
- Licensed under the GNU AGPL version 3 or higher.
-}
@ -14,6 +14,7 @@ import qualified Data.ByteString as S
import qualified Data.ByteString.Char8 as S8
import Data.Char
import qualified System.FilePath.ByteString as P
import Control.Concurrent.Async
import Common
import Git
@ -184,19 +185,22 @@ coreBare = "core.bare"
{- Runs a command to get the configuration of a repo,
- and returns a repo populated with the configuration, as well as the raw
- output of the command. -}
fromPipe :: Repo -> String -> [CommandParam] -> IO (Either SomeException (Repo, S.ByteString))
- output and any standard output of the command. -}
fromPipe :: Repo -> String -> [CommandParam] -> IO (Either SomeException (Repo, S.ByteString, S.ByteString))
fromPipe r cmd params = try $
withHandle StdoutHandle createProcessSuccess p $ \h -> do
val <- S.hGetContents h
withOEHandles createProcessSuccess p $ \(hout, herr) -> do
geterr <- async $ S.hGetContents herr
getval <- async $ S.hGetContents hout
val <- wait getval
err <- wait geterr
r' <- store val r
return (r', val)
return (r', val, err)
where
p = proc cmd $ toCommand params
{- Reads git config from a specified file and returns the repo populated
- with the configuration. -}
fromFile :: Repo -> FilePath -> IO (Either SomeException (Repo, S.ByteString))
fromFile :: Repo -> FilePath -> IO (Either SomeException (Repo, S.ByteString, S.ByteString))
fromFile r f = fromPipe r "git"
[ Param "config"
, Param "--file"

View file

@ -56,6 +56,7 @@ import Utility.Tmp
import Logs.Remote
import Utility.Gpg
import Utility.SshHost
import Utility.Tuple
import Messages.Progress
import Types.ProposedAccepted
@ -471,7 +472,7 @@ getGCryptId :: Bool -> Git.Repo -> RemoteGitConfig -> Annex (Maybe Git.GCrypt.GC
getGCryptId fast r gc
| Git.repoIsLocal r || Git.repoIsLocalUnknown r = extract <$>
liftIO (catchMaybeIO $ Git.Config.read r)
| not fast = extract . liftM fst <$> getM (eitherToMaybe <$>)
| not fast = extract . liftM fst3 <$> getM (eitherToMaybe <$>)
[ Ssh.onRemote NoConsumeStdin r (\f p -> liftIO (Git.Config.fromPipe r f p), return (Left $ error "configlist failed")) "configlist" [] []
, getConfigViaRsync r gc
]
@ -480,7 +481,7 @@ getGCryptId fast r gc
extract Nothing = (Nothing, r)
extract (Just r') = (fromConfigValue <$> Git.Config.getMaybe coreGCryptId r', r')
getConfigViaRsync :: Git.Repo -> RemoteGitConfig -> Annex (Either SomeException (Git.Repo, S.ByteString))
getConfigViaRsync :: Git.Repo -> RemoteGitConfig -> Annex (Either SomeException (Git.Repo, S.ByteString, S.ByteString))
getConfigViaRsync r gc = do
(rsynctransport, rsyncurl, _) <- rsyncTransport r gc
opts <- rsynctransport

View file

@ -248,7 +248,7 @@ tryGitConfigRead autoinit r
| haveconfig r = return r -- already read
| Git.repoIsSsh r = storeUpdatedRemote $ do
v <- Ssh.onRemote NoConsumeStdin r
(pipedconfig autoinit, return (Left $ giveup "configlist failed"))
(pipedconfig autoinit (Git.repoDescribe r), return (Left $ giveup "configlist failed"))
"configlist" [] configlistfields
case v of
Right r'
@ -263,23 +263,25 @@ tryGitConfigRead autoinit r
where
haveconfig = not . M.null . Git.config
pipedconfig mustincludeuuuid cmd params = do
pipedconfig mustincludeuuuid configloc cmd params = do
v <- liftIO $ Git.Config.fromPipe r cmd params
case v of
Right (r', val) -> do
Right (r', val, _err) -> do
unless (isUUIDConfigured r' || S.null val || not mustincludeuuuid) $ do
warning $ "Failed to get annex.uuid configuration of repository " ++ Git.repoDescribe r
warning $ "Instead, got: " ++ show val
warning $ "This is unexpected; please check the network transport!"
return $ Right r'
Left l -> return $ Left l
Left l -> do
warning $ "Unable to parse git config from " ++ configloc
return $ Left l
geturlconfig = Url.withUrlOptions $ \uo -> do
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 False "git" [Param "config", Param "--null", Param "--list", Param "--file", File tmpfile]
( Just <$> pipedconfig False url "git" [Param "config", Param "--null", Param "--list", Param "--file", File tmpfile]
, return Nothing
)
case v of

View file

@ -1,3 +1,5 @@
[[!meta title="http remotes that require authentication are not yet supported"]]
It is not a ground shaking issue, but probably would be best to handle it more gracefully.
Initially mentioned while doing install using datalad. Account/permission is required to access this particular repo, ask Canadians for access if you don't have it yet Joey. credentials I guess got asked for and cached by git upon initial invocation, so upon subsequent calls didn't ask for any:

View file

@ -0,0 +1,7 @@
[[!comment format=mdwn
username="joey"
subject="""comment 5"""
date="2020-01-22T17:04:09Z"
content="""
Error message has been improved.
"""]]