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

diff --git a/CHANGELOG b/CHANGELOG
index 5f2b2538fc3b12fbd92fad9f8a9e91c4d346db96..799ed1baa63b31bb750f6ec46bea395b23dc4bba 100644 (file)
--- 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 <id@joeyh.name>  Wed, 23 Feb 2022 14:14:09 -0400
 

diff --git a/CmdLine/Seek.hs b/CmdLine/Seek.hs
index 3daaa5ad805e219441f5fbf5a839e35686f7f723..3bc56dc7500a313ec5ae7655e6f541629a1d6fb4 100644 (file)
--- 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 0fd184112b0987174f36760553abc6b11e5604aa..2ecb0c10c54ed07c81d1960f60c4cf9918e0f4f5 100644 (file)
--- 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 (file)
index 0000000..b026f98
--- /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.
+"""]]