From 835c50966a6ce4342b158af131176db55bf7a973 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 26 Jan 2022 12:59:55 -0400 Subject: [PATCH] reject batch options combined with non-batch options Reject combinations of --batch (or --batch-keys) with options like --all or --key or with filenames. Most commands ignored the non-batch items when batch mode was enabled. For some reason, addurl and dropkey both processed first the specified non-batch items, followed by entering batch mode. Changed them to also error out, for consistency. Sponsored-by: Dartmouth College's Datalad project --- CHANGELOG | 2 ++ CmdLine/Batch.hs | 10 ++++++++-- Command/Add.hs | 3 ++- Command/AddUrl.hs | 6 +++--- Command/Copy.hs | 3 ++- Command/Drop.hs | 3 ++- Command/DropKey.hs | 8 ++++---- Command/Find.hs | 3 ++- Command/FromKey.hs | 3 ++- Command/Get.hs | 3 ++- Command/Info.hs | 3 ++- Command/MetaData.hs | 5 +++-- Command/Move.hs | 3 ++- Command/ReKey.hs | 5 +++-- Command/RegisterUrl.hs | 2 +- Command/RmUrl.hs | 3 ++- Command/SetPresentKey.hs | 7 ++++--- Command/Whereis.hs | 3 ++- .../should_error_on_whereis_--batch-keys_--all.mdwn | 2 ++ 19 files changed, 50 insertions(+), 27 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c6468a020a..04e0e32fae 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -26,6 +26,8 @@ git-annex (10.20220121) UNRELEASED; urgency=medium * adb: Added ignorefinderror configuration parameter. * Avoid crashing when run in a bare git repo that somehow contains an index file. + * Reject combinations of --batch (or --batch-keys) with options like + --all or --key or with filenames. -- Joey Hess Mon, 03 Jan 2022 14:01:14 -0400 diff --git a/CmdLine/Batch.hs b/CmdLine/Batch.hs index 15fd89710e..80c901ecca 100644 --- a/CmdLine/Batch.hs +++ b/CmdLine/Batch.hs @@ -77,8 +77,9 @@ batchable handler parser paramdesc = batchseeker <$> batchparser batchseeker (opts, NoBatch, params) = mapM_ (\p -> go NoBatch opts (SeekInput [p], p)) params - batchseeker (opts, batchmode@(Batch fmt), _) = - batchInput fmt (pure . Right) (go batchmode opts) + batchseeker (opts, batchmode@(Batch fmt), params) = + batchOnly Nothing params $ + batchInput fmt (pure . Right) (go batchmode opts) go batchmode opts (si, p) = unlessM (handler opts si p) $ @@ -209,3 +210,8 @@ batchAnnexed fmt seeker keyaction = do , providedMimeEncoding = Nothing , providedLinkType = Nothing } + +batchOnly :: Maybe KeyOptions -> CmdParams -> Annex () -> Annex () +batchOnly Nothing [] a = a +batchOnly _ _ _ = giveup "Cannot combine batch option with file or key options" + diff --git a/Command/Add.hs b/Command/Add.hs index 4da6f7354f..57ee51b5a2 100644 --- a/Command/Add.hs +++ b/Command/Add.hs @@ -95,7 +95,8 @@ seek o = startConcurrency commandStages $ do Batch fmt | updateOnly o -> giveup "--update --batch is not supported" - | otherwise -> batchFiles fmt gofile + | otherwise -> batchOnly Nothing (addThese o) $ + batchFiles fmt gofile NoBatch -> do -- Avoid git ls-files complaining about files that -- are not known to git yet, since this will add diff --git a/Command/AddUrl.hs b/Command/AddUrl.hs index 30443e649b..9c35b01e51 100644 --- a/Command/AddUrl.hs +++ b/Command/AddUrl.hs @@ -119,10 +119,10 @@ seek o = startConcurrency commandStages $ do then void $ commandAction $ startWeb addunlockedmatcher o' si u else checkUrl addunlockedmatcher r o' si u - forM_ (addUrls o) (\u -> go (SeekInput [u], (o, u))) case batchOption o of - Batch fmt -> batchInput fmt (pure . parseBatchInput o) go - NoBatch -> noop + Batch fmt -> batchOnly Nothing (addUrls o) $ + batchInput fmt (pure . parseBatchInput o) go + NoBatch -> forM_ (addUrls o) (\u -> go (SeekInput [u], (o, u))) parseBatchInput :: AddUrlOptions -> String -> Either String (AddUrlOptions, URLString) parseBatchInput o s diff --git a/Command/Copy.hs b/Command/Copy.hs index a6ddf4f80f..9e18cf7e2d 100644 --- a/Command/Copy.hs +++ b/Command/Copy.hs @@ -51,7 +51,8 @@ seek o = startConcurrency commandStages $ do (commandAction . keyaction) (withFilesInGitAnnex ww seeker) =<< workTreeItems ww (copyFiles o) - Batch fmt -> batchAnnexed fmt seeker keyaction + Batch fmt -> batchOnly (keyOptions o) (copyFiles o) $ + batchAnnexed fmt seeker keyaction where ww = WarnUnmatchLsFiles diff --git a/Command/Drop.hs b/Command/Drop.hs index a9a4314cac..b5774b999f 100644 --- a/Command/Drop.hs +++ b/Command/Drop.hs @@ -71,7 +71,8 @@ seek o = startConcurrency commandStages $ do (commandAction . startKeys o from) (withFilesInGitAnnex ww seeker) =<< workTreeItems ww (dropFiles o) - Batch fmt -> batchAnnexed fmt seeker (startKeys o from) + Batch fmt -> batchOnly (keyOptions o) (dropFiles o) $ + batchAnnexed fmt seeker (startKeys o from) where ww = WarnUnmatchLsFiles diff --git a/Command/DropKey.hs b/Command/DropKey.hs index 580c6f1c22..86248b0cc0 100644 --- a/Command/DropKey.hs +++ b/Command/DropKey.hs @@ -33,11 +33,11 @@ seek :: DropKeyOptions -> CommandSeek seek o = do unlessM (Annex.getState Annex.force) $ giveup "dropkey can cause data loss; use --force if you're sure you want to do this" - withKeys (commandAction . start) (toDrop o) case batchOption o of - Batch fmt -> batchInput fmt (pure . parsekey) $ - batchCommandAction . start - NoBatch -> noop + NoBatch -> withKeys (commandAction . start) (toDrop o) + Batch fmt -> batchOnly Nothing (toDrop o) $ + batchInput fmt (pure . parsekey) $ + batchCommandAction . start where parsekey = maybe (Left "bad key") Right . deserializeKey diff --git a/Command/Find.hs b/Command/Find.hs index 0a5544e437..271543d1e6 100644 --- a/Command/Find.hs +++ b/Command/Find.hs @@ -70,7 +70,8 @@ seek o = do (commandAction . startKeys o) (withFilesInGitAnnex ww seeker) =<< workTreeItems ww (findThese o) - Batch fmt -> batchAnnexedFiles fmt seeker + Batch fmt -> batchOnly (keyOptions o) (findThese o) $ + batchAnnexedFiles fmt seeker where ww = WarnUnmatchLsFiles diff --git a/Command/FromKey.hs b/Command/FromKey.hs index e4591a184f..22eafcefe5 100644 --- a/Command/FromKey.hs +++ b/Command/FromKey.hs @@ -41,7 +41,8 @@ seek :: FromKeyOptions -> CommandSeek seek o = do matcher <- addUnlockedMatcher case (batchOption o, keyFilePairs o) of - (Batch fmt, _) -> seekBatch matcher fmt + (Batch fmt, _) -> batchOnly Nothing (keyFilePairs o) $ + seekBatch matcher fmt -- older way of enabling batch input, does not support BatchNull (NoBatch, []) -> seekBatch matcher (BatchFormat BatchLine (BatchKeys False)) (NoBatch, ps) -> do diff --git a/Command/Get.hs b/Command/Get.hs index eef40c4eb3..c3faf308d4 100644 --- a/Command/Get.hs +++ b/Command/Get.hs @@ -49,7 +49,8 @@ seek o = startConcurrency downloadStages $ do (commandAction . startKeys from) (withFilesInGitAnnex ww seeker) =<< workTreeItems ww (getFiles o) - Batch fmt -> batchAnnexed fmt seeker (startKeys from) + Batch fmt -> batchOnly (keyOptions o) (getFiles o) $ + batchAnnexed fmt seeker (startKeys from) where ww = WarnUnmatchLsFiles diff --git a/Command/Info.hs b/Command/Info.hs index 8f9f243a0b..6594498e3a 100644 --- a/Command/Info.hs +++ b/Command/Info.hs @@ -119,7 +119,8 @@ optParser desc = InfoOptions seek :: InfoOptions -> CommandSeek seek o = case batchOption o of NoBatch -> withWords (commandAction . start o) (infoFor o) - Batch fmt -> batchInput fmt (pure . Right) (itemInfo o) + Batch fmt -> batchOnly Nothing (infoFor o) $ + batchInput fmt (pure . Right) (itemInfo o) start :: InfoOptions -> [String] -> CommandStart start o [] = do diff --git a/Command/MetaData.hs b/Command/MetaData.hs index 2ab6e37ee4..cd48734b98 100644 --- a/Command/MetaData.hs +++ b/Command/MetaData.hs @@ -93,8 +93,9 @@ seek o = case batchOption o of Batch fmt -> withMessageState $ \s -> case outputType s of JSONOutput _ -> ifM limited ( giveup "combining --batch with file matching options is not currently supported" - , batchInput fmt parseJSONInput - (commandAction . batchCommandStart . startBatch) + , batchOnly (keyOptions o) (forFiles o) $ + batchInput fmt parseJSONInput + (commandAction . batchCommandStart . startBatch) ) _ -> giveup "--batch is currently only supported in --json mode" diff --git a/Command/Move.hs b/Command/Move.hs index 781bdb6138..31f6d05a79 100644 --- a/Command/Move.hs +++ b/Command/Move.hs @@ -64,7 +64,8 @@ seek o = startConcurrency stages $ do (commandAction . keyaction) (withFilesInGitAnnex ww seeker) =<< workTreeItems ww (moveFiles o) - Batch fmt -> batchAnnexed fmt seeker keyaction + Batch fmt -> batchOnly (keyOptions o) (moveFiles o) $ + batchAnnexed fmt seeker keyaction where seeker = AnnexedFileSeeker { startAction = start (fromToOptions o) (removeWhen o) diff --git a/Command/ReKey.hs b/Command/ReKey.hs index ab2f9fa241..d00cad566f 100644 --- a/Command/ReKey.hs +++ b/Command/ReKey.hs @@ -50,8 +50,9 @@ batchParser s = case separate (== ' ') (reverse s) of seek :: ReKeyOptions -> CommandSeek seek o = case batchOption o of - Batch fmt -> batchInput fmt batchParser - (batchCommandAction . uncurry start) + Batch fmt -> batchOnly Nothing (reKeyThese o) $ + batchInput fmt batchParser + (batchCommandAction . uncurry start) NoBatch -> withPairs (\(si, p) -> commandAction (start si (parsekey p))) (reKeyThese o) diff --git a/Command/RegisterUrl.hs b/Command/RegisterUrl.hs index 1ec953d02b..f422729572 100644 --- a/Command/RegisterUrl.hs +++ b/Command/RegisterUrl.hs @@ -32,7 +32,7 @@ optParser desc = RegisterUrlOptions seek :: RegisterUrlOptions -> CommandSeek seek o = case (batchOption o, keyUrlPairs o) of - (Batch (BatchFormat sep _), _) -> + (Batch (BatchFormat sep _), _) -> batchOnly Nothing (keyUrlPairs o) $ commandAction $ startMass setUrlPresent sep -- older way of enabling batch input, does not support BatchNull (NoBatch, []) -> commandAction $ startMass setUrlPresent BatchLine diff --git a/Command/RmUrl.hs b/Command/RmUrl.hs index bd854f64a0..93443b227a 100644 --- a/Command/RmUrl.hs +++ b/Command/RmUrl.hs @@ -29,7 +29,8 @@ optParser desc = RmUrlOptions seek :: RmUrlOptions -> CommandSeek seek o = case batchOption o of - Batch fmt -> batchInput fmt batchParser (batchCommandAction . start) + Batch fmt -> batchOnly Nothing (rmThese o) $ + batchInput fmt batchParser (batchCommandAction . start) NoBatch -> withPairs (commandAction . start) (rmThese o) -- Split on the last space, since a FilePath can contain whitespace, diff --git a/Command/SetPresentKey.hs b/Command/SetPresentKey.hs index 50749886e6..839d23cec7 100644 --- a/Command/SetPresentKey.hs +++ b/Command/SetPresentKey.hs @@ -30,9 +30,10 @@ optParser desc = SetPresentKeyOptions seek :: SetPresentKeyOptions -> CommandSeek seek o = case batchOption o of - Batch fmt -> batchInput fmt - (pure . parseKeyStatus . words) - (batchCommandAction . uncurry start) + Batch fmt -> batchOnly Nothing (params o) $ + batchInput fmt + (pure . parseKeyStatus . words) + (batchCommandAction . uncurry start) NoBatch -> either giveup (commandAction . start (SeekInput (params o))) (parseKeyStatus $ params o) diff --git a/Command/Whereis.hs b/Command/Whereis.hs index 58f26db1f1..d3efb74658 100644 --- a/Command/Whereis.hs +++ b/Command/Whereis.hs @@ -62,7 +62,8 @@ seek o = do (commandAction . startKeys o m) (withFilesInGitAnnex ww seeker) =<< workTreeItems ww (whereisFiles o) - Batch fmt -> batchAnnexed fmt seeker (startKeys o m) + Batch fmt -> batchOnly (keyOptions o) (whereisFiles o) $ + batchAnnexed fmt seeker (startKeys o m) where ww = WarnUnmatchLsFiles diff --git a/doc/bugs/should_error_on_whereis_--batch-keys_--all.mdwn b/doc/bugs/should_error_on_whereis_--batch-keys_--all.mdwn index 09d403bbce..0ee8f21ebb 100644 --- a/doc/bugs/should_error_on_whereis_--batch-keys_--all.mdwn +++ b/doc/bugs/should_error_on_whereis_--batch-keys_--all.mdwn @@ -21,3 +21,5 @@ MD5E-s5663237--4608ffbd6b78ce3a325eb338fa556589.nii.gz ### What version of git-annex are you using? On what operating system? `8.20211231+git140-gc3817495f-1~ndall+1` + +> [[fixed|done]] --[[Joey]]