From 957a87b437d6f056aa159e484a5f0a5af54e45db Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 15 Apr 2020 16:04:05 -0400 Subject: [PATCH] fix absolute filenames fed into --batch and git-annex info --- CHANGELOG | 2 ++ CmdLine/Batch.hs | 21 +++++++++++----- Command/AddUrl.hs | 2 +- Command/CheckPresentKey.hs | 2 +- Command/DropKey.hs | 3 ++- Command/FromKey.hs | 11 +++++--- Command/Info.hs | 10 +++++--- Command/MetaData.hs | 25 +++++++++++-------- Command/ReKey.hs | 14 +++++++---- Command/RmUrl.hs | 12 ++++++--- Command/SetPresentKey.hs | 2 +- ...-batch_will_not_accept_absolute_paths.mdwn | 2 ++ ..._bdb19362d1aeec443f7f1a7b66edc5ae._comment | 14 +++++++++++ ..._b215be5cea338326e25057a4b93ea85f._comment | 23 +++++++++++++++++ 14 files changed, 106 insertions(+), 37 deletions(-) create mode 100644 doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_4_bdb19362d1aeec443f7f1a7b66edc5ae._comment create mode 100644 doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_5_b215be5cea338326e25057a4b93ea85f._comment diff --git a/CHANGELOG b/CHANGELOG index 8b0d372eb0..899e3985de 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,8 @@ git-annex (8.20200331) UNRELEASED; urgency=medium setting with no value, eg "core.bare" is the same as "core.bare = true". * When parsing git configs, support all the documented ways to write true and false, including "yes", "on", "1", etc. + * Fix --batch commands (and git-annex info) to accept absolute filenames + for unlocked files, which already worked for locked files. -- Joey Hess Mon, 30 Mar 2020 15:58:34 -0400 diff --git a/CmdLine/Batch.hs b/CmdLine/Batch.hs index 7639e03f81..e55d3d04a6 100644 --- a/CmdLine/Batch.hs +++ b/CmdLine/Batch.hs @@ -50,7 +50,7 @@ batchable handler parser paramdesc = batchseeker <$> batchparser batchseeker (opts, NoBatch, params) = mapM_ (go NoBatch opts) params batchseeker (opts, batchmode@(Batch fmt), _) = - batchInput fmt Right (go batchmode opts) + batchInput fmt (pure . Right) (go batchmode opts) go batchmode opts p = unlessM (handler opts p) $ @@ -62,13 +62,19 @@ batchBadInput :: BatchMode -> Annex () batchBadInput NoBatch = liftIO exitFailure batchBadInput (Batch _) = liftIO $ putStrLn "" --- Reads lines of batch mode input and passes to the action to handle. -batchInput :: BatchFormat -> (String -> Either String a) -> (a -> Annex ()) -> Annex () +-- Reads lines of batch mode input, runs a parser, and passes the result +-- to the action. +-- +-- Note that if the batch input includes a worktree filename, it should +-- be converted to relative. Normally, filename parameters are passed +-- through git ls-files, which makes them relative, but batch mode does +-- not use that, and absolute worktree files are likely to cause breakage. +batchInput :: BatchFormat -> (String -> Annex (Either String a)) -> (a -> Annex ()) -> Annex () batchInput fmt parser a = go =<< batchLines fmt where go [] = return () go (l:rest) = do - either parseerr a (parser l) + either parseerr a =<< parser l go rest parseerr s = giveup $ "Batch input parse failure: " ++ s @@ -95,9 +101,12 @@ batchCommandAction a = maybe (batchBadInput (Batch BatchLine)) (const noop) -- Reads lines of batch input and passes the filepaths to a CommandStart -- to handle them. -- +-- Absolute filepaths are converted to relative. +-- -- File matching options are not checked. -batchStart :: BatchFormat -> (String -> CommandStart) -> Annex () -batchStart fmt a = batchInput fmt Right $ batchCommandAction . a +batchStart :: BatchFormat -> (FilePath -> CommandStart) -> Annex () +batchStart fmt a = batchInput fmt (Right <$$> liftIO . relPathCwdToFile) $ + batchCommandAction . a -- Like batchStart, but checks the file matching options -- and skips non-matching files. diff --git a/Command/AddUrl.hs b/Command/AddUrl.hs index de393fa669..cd7d75cbd5 100644 --- a/Command/AddUrl.hs +++ b/Command/AddUrl.hs @@ -103,7 +103,7 @@ seek o = startConcurrency commandStages $ do else checkUrl addunlockedmatcher r o' u forM_ (addUrls o) (\u -> go (o, u)) case batchOption o of - Batch fmt -> batchInput fmt (parseBatchInput o) go + Batch fmt -> batchInput fmt (pure . parseBatchInput o) go NoBatch -> noop parseBatchInput :: AddUrlOptions -> String -> Either String (AddUrlOptions, URLString) diff --git a/Command/CheckPresentKey.hs b/Command/CheckPresentKey.hs index b41797fe2f..55ca338e39 100644 --- a/Command/CheckPresentKey.hs +++ b/Command/CheckPresentKey.hs @@ -38,7 +38,7 @@ seek o = case batchOption o of (rn:[]) -> toRemote rn >>= \r -> return (flip check (Just r)) [] -> return (flip check Nothing) _ -> wrongnumparams - batchInput fmt Right $ checker >=> batchResult + batchInput fmt (pure . Right) $ checker >=> batchResult where wrongnumparams = giveup "Wrong number of parameters" diff --git a/Command/DropKey.hs b/Command/DropKey.hs index 60040451ab..5fdf8d4abe 100644 --- a/Command/DropKey.hs +++ b/Command/DropKey.hs @@ -35,7 +35,8 @@ seek o = do 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 parsekey $ batchCommandAction . start + Batch fmt -> batchInput fmt (pure . parsekey) $ + batchCommandAction . start NoBatch -> noop where parsekey = maybe (Left "bad key") Right . deserializeKey diff --git a/Command/FromKey.hs b/Command/FromKey.hs index 5becd3b810..ecd8bd294f 100644 --- a/Command/FromKey.hs +++ b/Command/FromKey.hs @@ -47,11 +47,14 @@ seek o = case (batchOption o, keyFilePairs o) of seekBatch :: BatchFormat -> CommandSeek seekBatch fmt = batchInput fmt parse commandAction where - parse s = + parse s = do let (keyname, file) = separate (== ' ') s - in if not (null keyname) && not (null file) - then Right $ go file (keyOpt keyname) - else Left "Expected pairs of key and filename" + if not (null keyname) && not (null file) + then do + file' <- liftIO $ relPathCwdToFile file + return $ Right $ go file' (keyOpt keyname) + else return $ + Left "Expected pairs of key and filename" go file key = starting "fromkey" (mkActionItem (key, toRawFilePath file)) $ perform key file diff --git a/Command/Info.hs b/Command/Info.hs index 7df42fa39a..b61b8527bc 100644 --- a/Command/Info.hs +++ b/Command/Info.hs @@ -119,7 +119,7 @@ optParser desc = InfoOptions seek :: InfoOptions -> CommandSeek seek o = case batchOption o of NoBatch -> withWords (commandAction . start o) (infoFor o) - Batch fmt -> batchInput fmt Right (itemInfo o) + Batch fmt -> batchInput fmt (pure . Right) (itemInfo o) start :: InfoOptions -> [String] -> CommandStart start o [] = do @@ -152,9 +152,11 @@ itemInfo o p = ifM (isdir p) v' <- Remote.nameToUUID' p case v' of Right u -> uuidInfo o u - Left _ -> ifAnnexed (toRawFilePath p) - (fileInfo o p) - (treeishInfo o p) + Left _ -> do + relp <- liftIO $ relPathCwdToFile p + ifAnnexed (toRawFilePath relp) + (fileInfo o relp) + (treeishInfo o p) ) where isdir = liftIO . catchBoolIO . (isDirectory <$$> getFileStatus) diff --git a/Command/MetaData.hs b/Command/MetaData.hs index 076fe38bb6..a47d67494b 100644 --- a/Command/MetaData.hs +++ b/Command/MetaData.hs @@ -148,16 +148,21 @@ instance FromJSON MetaDataFields where fieldsField :: T.Text fieldsField = T.pack "fields" -parseJSONInput :: String -> Either String (Either RawFilePath Key, MetaData) -parseJSONInput i = do - v <- eitherDecode (BU.fromString i) - let m = case itemAdded v of - Nothing -> emptyMetaData - Just (MetaDataFields m') -> m' - case (itemKey v, itemFile v) of - (Just k, _) -> Right (Right k, m) - (Nothing, Just f) -> Right (Left (toRawFilePath f), m) - (Nothing, Nothing) -> Left "JSON input is missing either file or key" +parseJSONInput :: String -> Annex (Either String (Either RawFilePath Key, MetaData)) +parseJSONInput i = case eitherDecode (BU.fromString i) of + Left e -> return (Left e) + Right v -> do + let m = case itemAdded v of + Nothing -> emptyMetaData + Just (MetaDataFields m') -> m' + case (itemKey v, itemFile v) of + (Just k, _) -> return $ + Right (Right k, m) + (Nothing, Just f) -> do + f' <- liftIO $ relPathCwdToFile f + return $ Right (Left (toRawFilePath f'), m) + (Nothing, Nothing) -> return $ + Left "JSON input is missing either file or key" startBatch :: (Either RawFilePath Key, MetaData) -> CommandStart startBatch (i, (MetaData m)) = case i of diff --git a/Command/ReKey.hs b/Command/ReKey.hs index 068cefe8b9..7fc171bf23 100644 --- a/Command/ReKey.hs +++ b/Command/ReKey.hs @@ -39,17 +39,21 @@ optParser desc = ReKeyOptions -- Split on the last space, since a FilePath can contain whitespace, -- but a Key very rarely does. -batchParser :: String -> Either String (RawFilePath, Key) +batchParser :: String -> Annex (Either String (RawFilePath, Key)) batchParser s = case separate (== ' ') (reverse s) of (rk, rf) - | null rk || null rf -> Left "Expected: \"file key\"" + | null rk || null rf -> return $ Left "Expected: \"file key\"" | otherwise -> case deserializeKey (reverse rk) of - Nothing -> Left "bad key" - Just k -> Right (toRawFilePath (reverse rf), k) + Nothing -> return $ Left "bad key" + Just k -> do + let f = reverse rf + f' <- liftIO $ relPathCwdToFile f + return $ Right (toRawFilePath f', k) seek :: ReKeyOptions -> CommandSeek seek o = case batchOption o of - Batch fmt -> batchInput fmt batchParser (batchCommandAction . start) + Batch fmt -> batchInput fmt batchParser $ + batchCommandAction . start NoBatch -> withPairs (commandAction . start . parsekey) (reKeyThese o) where parsekey (file, skey) = diff --git a/Command/RmUrl.hs b/Command/RmUrl.hs index 04c3165ce5..6558be70b1 100644 --- a/Command/RmUrl.hs +++ b/Command/RmUrl.hs @@ -30,16 +30,20 @@ optParser desc = RmUrlOptions seek :: RmUrlOptions -> CommandSeek seek o = case batchOption o of - Batch fmt -> batchInput fmt batchParser (batchCommandAction . start) + Batch fmt -> batchInput fmt batchParser + (batchCommandAction . start) NoBatch -> withPairs (commandAction . start) (rmThese o) -- Split on the last space, since a FilePath can contain whitespace, -- but a url should not. -batchParser :: String -> Either String (FilePath, URLString) +batchParser :: String -> Annex (Either String (FilePath, URLString)) batchParser s = case separate (== ' ') (reverse s) of (ru, rf) - | null ru || null rf -> Left "Expected: \"file url\"" - | otherwise -> Right (reverse rf, reverse ru) + | null ru || null rf -> return $ Left "Expected: \"file url\"" + | otherwise -> do + let f = reverse rf + f' <- liftIO $ relPathCwdToFile f + return $ Right (f', reverse ru) start :: (FilePath, URLString) -> CommandStart start (file, url) = flip whenAnnexed file' $ \_ key -> diff --git a/Command/SetPresentKey.hs b/Command/SetPresentKey.hs index 616e153cc9..443546b448 100644 --- a/Command/SetPresentKey.hs +++ b/Command/SetPresentKey.hs @@ -31,7 +31,7 @@ optParser desc = SetPresentKeyOptions seek :: SetPresentKeyOptions -> CommandSeek seek o = case batchOption o of Batch fmt -> batchInput fmt - (parseKeyStatus . words) + (pure . parseKeyStatus . words) (batchCommandAction . start) NoBatch -> either giveup (commandAction . start) (parseKeyStatus $ params o) diff --git a/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths.mdwn b/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths.mdwn index 78f802914a..9c8d2b13f5 100644 --- a/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths.mdwn +++ b/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths.mdwn @@ -3,3 +3,5 @@ I tested `git annex lookupkey --batch` which does not have this problem. --spwhitton + +> [[fixed|done]] --[[Joey]] diff --git a/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_4_bdb19362d1aeec443f7f1a7b66edc5ae._comment b/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_4_bdb19362d1aeec443f7f1a7b66edc5ae._comment new file mode 100644 index 0000000000..fe0294f62a --- /dev/null +++ b/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_4_bdb19362d1aeec443f7f1a7b66edc5ae._comment @@ -0,0 +1,14 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 4""" + date="2020-04-15T19:13:39Z" + content=""" +Other commands like whereis --batch also behave the same. + +Looks like what's going on is, when an absolute path is passed +as a parameter, it feeds thru git ls-files, producing a relative file. +But with --batch, it stays absolute. This causes things that try to eg, +look up the file in the tree to not find it. + +So, --batch needs to make filepaths relative too.. +"""]] diff --git a/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_5_b215be5cea338326e25057a4b93ea85f._comment b/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_5_b215be5cea338326e25057a4b93ea85f._comment new file mode 100644 index 0000000000..ee530068b7 --- /dev/null +++ b/doc/todo/git-annex_find_--batch_will_not_accept_absolute_paths/comment_5_b215be5cea338326e25057a4b93ea85f._comment @@ -0,0 +1,23 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 5""" + date="2020-04-15T19:22:12Z" + content=""" +Most of it can be fixed by making batchStart make +files relative. + +Other affected commands that do custom parsing of +batch input, so will need to make the file from it +relative themselves: fromkey metadata rekey rmurl + +Also, `git annex info /path/to/file` fails for unlocked +files and works for locked files, because it does not pass +filenames through git ls-files. I think it's the only +command that does not, when not in batch mode. + +(I suppose alternatively, lookupKey could make the filename relative, +but I don't know if that is the only thing that fails on absolute +filenames, so prefer to make them all relative on input.) + +Ok, all done.. +"""]]