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
This commit is contained in:
Joey Hess 2022-02-28 12:54:56 -04:00
parent a6857ddb79
commit ce91f10132
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
4 changed files with 44 additions and 5 deletions

View file

@ -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

View file

@ -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

View file

@ -20,3 +20,5 @@ git-annex version: 10.20220222+git23-g51c528980-1~ndall+1
[[!meta author=yoh]]
[[!tag projects/datalad]]
> [[fixed|done]] --[[Joey]]

View file

@ -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.
"""]]