From ce91f10132805d11448896304821b0aa9c6d9845 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 28 Feb 2022 12:54:56 -0400 Subject: [PATCH] fix annex.skipunknown false error propagation Propagate nonzero exit status from git ls-files when a specified file does not exist, or a specified directory does not contain any files checked into git. The recent completion of the annex.skipunknown transition exposed this bug, that has unfortunately been lurking all along. It is also possible that git ls-files errors out for some other reason -- perhaps a permission problem -- and this will also fix error propagation in such situations. Sponsored-by: Dartmouth College's Datalad project --- CHANGELOG | 3 ++ CmdLine/Seek.hs | 15 ++++++---- ..._for___34__find_..._nonexisting__34__.mdwn | 2 ++ ..._28060f4bb31730e3334cf3db56fc032b._comment | 29 +++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__/comment_3_28060f4bb31730e3334cf3db56fc032b._comment diff --git a/CHANGELOG b/CHANGELOG index 5f2b2538fc..799ed1baa6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,9 @@ git-annex (10.20220223) UNRELEASED; urgency=medium * Run annex.thawcontent-command before deleting an object file, in case annex.freezecontent-command did something that would prevent deletion. + * Propagate nonzero exit status from git ls-files when a specified + file does not exist, or a specified directory does not contain + any files checked into git. -- Joey Hess Wed, 23 Feb 2022 14:14:09 -0400 diff --git a/CmdLine/Seek.hs b/CmdLine/Seek.hs index 3daaa5ad80..3bc56dc750 100644 --- a/CmdLine/Seek.hs +++ b/CmdLine/Seek.hs @@ -77,13 +77,13 @@ withFilesInGitAnnexNonRecursive ww needforce a (WorkTreeItems l) = ifM (Annex.ge (fs, cleanup) <- inRepo $ LsFiles.inRepoDetails os [toRawFilePath p] r <- case fs of [f] -> do - void $ liftIO $ cleanup + propagateLsFilesError cleanup fst <$> getfiles ((SeekInput [p], f):c) ps [] -> do - void $ liftIO $ cleanup + propagateLsFilesError cleanup fst <$> getfiles c ps _ -> do - void $ liftIO $ cleanup + propagateLsFilesError cleanup giveup needforce return (r, pure True) withFilesInGitAnnexNonRecursive _ _ _ NoWorkTreeItems = noop @@ -321,7 +321,7 @@ seekFiltered prefilter a listfs = do checktimelimit <- mkCheckTimeLimit (fs, cleanup) <- listfs go matcher checktimelimit fs - liftIO $ void cleanup + propagateLsFilesError cleanup where go _ _ [] = return () go matcher checktimelimit (v@(_si, f):rest) = checktimelimit noop $ do @@ -373,7 +373,7 @@ seekFilteredKeys seeker listfs = do ) join (liftIO (wait mdprocessertid)) join (liftIO (wait processertid)) - liftIO $ void cleanup + propagateLsFilesError cleanup where finisher mi oreader checktimelimit = liftIO oreader >>= \case Just ((si, f), content) -> checktimelimit (liftIO discard) $ do @@ -618,3 +618,8 @@ mkCheckTimeLimit = Annex.getState Annex.timelimit >>= \case cleanup liftIO $ exitWith $ ExitFailure 101 else a + +propagateLsFilesError :: IO Bool -> Annex () +propagateLsFilesError cleanup = + unlessM (liftIO cleanup) $ + Annex.incError diff --git a/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__.mdwn b/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__.mdwn index 0fd184112b..2ecb0c10c5 100644 --- a/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__.mdwn +++ b/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__.mdwn @@ -20,3 +20,5 @@ git-annex version: 10.20220222+git23-g51c528980-1~ndall+1 [[!meta author=yoh]] [[!tag projects/datalad]] + +> [[fixed|done]] --[[Joey]] diff --git a/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__/comment_3_28060f4bb31730e3334cf3db56fc032b._comment b/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__/comment_3_28060f4bb31730e3334cf3db56fc032b._comment new file mode 100644 index 0000000000..b026f985a8 --- /dev/null +++ b/doc/bugs/no_longer_non-0_exit_for___34__find_..._nonexisting__34__/comment_3_28060f4bb31730e3334cf3db56fc032b._comment @@ -0,0 +1,29 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 3""" + date="2022-02-28T16:32:08Z" + content=""" +I've fixed it to propagate the non-zero exit status from git ls-files. + +The git ls-files message still goes to stderr, not to +--json-error-messages, the same as any stderr output by a git command +that git-annex runs. And this is actually not a behavior change; +the old git-annex output of "not found" did not get included in +--json-error-messages either: + + joey@darkstar:/tmp/mmm>/usr/bin/git-annex drop dne foo --json --json-error-messages + git-annex: dne not found + {"command":"drop","wanted":[],"note":"unsafe\nCould only verify the existence of 0 out of 1 necessary copy\n(Use --force to override this check, or adjust numcopies.)","success":false,"input":["foo"],"key":"SHA256E-s3--98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4","error-messages":[],"file":"foo/bar"} + drop: 2 failed + +Bear in mind that --json-error-messages +is about errors encountered while processing a particular file that it +outputs a json record for. This error comes before it starts processing any +particular file. Also, of course, error messages output by git commands +are never included in --json-error-messages. + +I think it might be a good idea to add an actual API for detecting when +there is this kind of problem, so datalad does not have to parse +error messages (which could easily change). But that would need to be a +separate discussion. +"""]]