diff --git a/Annex/Content.hs b/Annex/Content.hs index 12af39618c..076fce1edb 100644 --- a/Annex/Content.hs +++ b/Annex/Content.hs @@ -1,6 +1,6 @@ {- git-annex file content managing - - - Copyright 2010-2020 Joey Hess + - Copyright 2010-2021 Joey Hess - - Licensed under the GNU AGPL version 3 or higher. -} @@ -36,6 +36,7 @@ module Annex.Content ( linkOrCopy', sendAnnex, prepSendAnnex, + prepSendAnnex', removeAnnex, moveBad, KeyLocation(..), @@ -446,14 +447,15 @@ unlinkAnnex key = do - The rollback action should remove the data that was transferred. -} sendAnnex :: Key -> Annex () -> (FilePath -> Annex a) -> Annex a -sendAnnex key rollback sendobject = go =<< prepSendAnnex key +sendAnnex key rollback sendobject = go =<< prepSendAnnex' key where - go (Just (f, checksuccess)) = do + go (Just (f, check)) = do r <- sendobject f - unlessM checksuccess $ do - rollback - giveup "content changed while it was being sent" - return r + check >>= \case + Nothing -> return r + Just err -> do + rollback + giveup err go Nothing = giveup "content not available to send" {- Returns a file that contains an object's content, @@ -483,6 +485,16 @@ prepSendAnnex key = withObjectLoc key $ \f -> do then Nothing else Just (fromRawFilePath f, sameInodeCache f cache') +prepSendAnnex' :: Key -> Annex (Maybe (FilePath, Annex (Maybe String))) +prepSendAnnex' key = prepSendAnnex key >>= \case + Just (f, checksuccess) -> + let checksuccess' = ifM checksuccess + ( return Nothing + , return (Just "content changed while it was being sent") + ) + in return (Just (f, checksuccess')) + Nothing -> return Nothing + cleanObjectLoc :: Key -> Annex () -> Annex () cleanObjectLoc key cleaner = do file <- calcRepo (gitAnnexLocation key) diff --git a/Annex/CopyFile.hs b/Annex/CopyFile.hs index b7d05cf97a..63eb1a95f1 100644 --- a/Annex/CopyFile.hs +++ b/Annex/CopyFile.hs @@ -113,7 +113,9 @@ fileCopier copycowtried src dest k meterupdate check verifyconfig = ( case iv of Just x -> ifM (liftIO $ finalizeIncremental x) ( return (True, Verified) - , return (False, UnVerified) + , do + warning "verification of content failed" + return (False, UnVerified) ) Nothing -> return (True, UnVerified) , return (False, UnVerified) diff --git a/CHANGELOG b/CHANGELOG index 9c32f4ad89..69cab38a45 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ git-annex (8.20210622) UNRELEASED; urgency=medium synced/git-annex first. * Added annex.freezecontent-command and annex.thawcontent-command configs. + * Improve display of errors when transfers fail. -- Joey Hess Mon, 21 Jun 2021 12:25:25 -0400 diff --git a/Remote/Git.hs b/Remote/Git.hs index 43105a4fcd..c2a23789a5 100644 --- a/Remote/Git.hs +++ b/Remote/Git.hs @@ -549,8 +549,11 @@ copyFromRemote'' repo forcersync r st@(State connpool _ _ _ _) key file dest met u <- getUUID hardlink <- wantHardLink -- run copy from perspective of remote - onLocalFast st $ Annex.Content.prepSendAnnex key >>= \case - Just (object, checksuccess) -> do + onLocalFast st $ Annex.Content.prepSendAnnex' key >>= \case + Just (object, check) -> do + let checksuccess = check >>= \case + Just err -> giveup err + Nothing -> return True let verify = Annex.Content.RemoteVerify r copier <- mkFileCopier hardlink st (ok, v) <- runTransfer (Transfer Download u (fromKey id key)) @@ -673,7 +676,7 @@ copyToRemote' :: Git.Repo -> Remote -> State -> Key -> AssociatedFile -> MeterUp copyToRemote' repo r st@(State connpool duc _ _ _) key file meterupdate | not $ Git.repoIsUrl repo = ifM duc ( guardUsable repo (giveup "cannot access remote") $ commitOnCleanup repo r st $ - copylocal =<< Annex.Content.prepSendAnnex key + copylocal =<< Annex.Content.prepSendAnnex' key , giveup "remote does not have expected annex.uuid value" ) | Git.repoIsSsh repo = commitOnCleanup repo r st $ @@ -684,11 +687,11 @@ copyToRemote' repo r st@(State connpool duc _ _ _) key file meterupdate | otherwise = giveup "copying to non-ssh repo not supported" where copylocal Nothing = giveup "content not available" - copylocal (Just (object, checksuccess)) = do - -- The checksuccess action is going to be run in + copylocal (Just (object, check)) = do + -- The check action is going to be run in -- the remote's Annex, but it needs access to the local -- Annex monad's state. - checksuccessio <- Annex.withCurrentState checksuccess + checkio <- Annex.withCurrentState check u <- getUUID hardlink <- wantHardLink -- run copy from perspective of remote @@ -698,9 +701,12 @@ copyToRemote' repo r st@(State connpool duc _ _ _) key file meterupdate let verify = Annex.Content.RemoteVerify r copier <- mkFileCopier hardlink st let rsp = RetrievalAllKeysSecure + let checksuccess = liftIO checkio >>= \case + Just err -> giveup err + Nothing -> return True res <- logStatusAfter key $ Annex.Content.getViaTmp rsp verify key file $ \dest -> metered (Just (combineMeterUpdate meterupdate p)) key $ \_ p' -> - copier object (fromRawFilePath dest) key p' (liftIO checksuccessio) verify + copier object (fromRawFilePath dest) key p' checksuccess verify Annex.Content.saveState True return res ) diff --git a/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_4_af8d80956d3ccedfe9aeb0fe22b2592a._comment b/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_4_af8d80956d3ccedfe9aeb0fe22b2592a._comment new file mode 100644 index 0000000000..3af04e4347 --- /dev/null +++ b/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_4_af8d80956d3ccedfe9aeb0fe22b2592a._comment @@ -0,0 +1,27 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 4""" + date="2021-06-25T16:15:40Z" + content=""" +I've taken a close look at the error handling in the relevant code path. +Normally when there's an exception it gets displayed. It is possible +for the transfer to fail without displaying anything and without throwing an +exception. When I synthesize such a failure, I see the same +output you described. + +One way that could happen is if git-annex thinks the file got +modified while it was being sent. If the file was unlocked, and was touched +by something else while the copy was running, it would be enough to make +git-annex think that, even though the content is still good. I've made +it display a reason for the failure in that case. + +The only other way I found was if the hash fails to verify, which seems +already ruled out in your case, but it did result in the same hard to understand +output, so I've fixed that too. + +It would be good to know if one of these changes has fixed it for you. +The autobuild for linux should be updated within an hour. +While the scenario with the file having been touched seems unlikely to +happen repeatedly with different files, it does otherwise match your +circumstances and seems like the most likely fit. +"""]]