From 846384fc3a413d9f0400edadccb789d8f34401a1 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 31 Jul 2023 12:36:13 -0400 Subject: [PATCH] --explain for numcopies checks And closed the todo as completed. Sponsored-by: Dartmouth College's DANDI project --- Annex/NumCopies.hs | 19 +++++++---- doc/todo/option_to_explain.mdwn | 2 ++ ..._28919c6f816f11811b1e8c73b07403f7._comment | 32 +++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 doc/todo/option_to_explain/comment_4_28919c6f816f11811b1e8c73b07403f7._comment diff --git a/Annex/NumCopies.hs b/Annex/NumCopies.hs index 82f963058c..a3c6f92dcd 100644 --- a/Annex/NumCopies.hs +++ b/Annex/NumCopies.hs @@ -197,8 +197,12 @@ numCopiesCheck file key vs = do numCopiesCheck' :: RawFilePath -> (Int -> Int -> v) -> [UUID] -> Annex v numCopiesCheck' file vs have = do - needed <- fst <$> getFileNumMinCopies file - return $ length have `vs` fromNumCopies needed + needed <- fromNumCopies . fst <$> getFileNumMinCopies file + let nhave = length have + explain (ActionItemTreeFile file) $ Just $ UnquotedString $ + "has " ++ show nhave ++ " " ++ pluralCopies nhave ++ + ", and the configured annex.numcopies is " ++ show needed + return $ nhave `vs` needed data UnVerifiedCopy = UnVerifiedRemote Remote | UnVerifiedHere deriving (Ord, Eq) @@ -280,10 +284,10 @@ notEnoughCopies key neednum needmin have skip bad nolocmsg lockunsupported = do then showLongNote $ UnquotedString $ "Could only verify the existence of " ++ show (length have) ++ " out of " ++ show (fromNumCopies neednum) ++ - " necessary " ++ pluralcopies (fromNumCopies neednum) + " necessary " ++ pluralCopies (fromNumCopies neednum) else do showLongNote $ UnquotedString $ "Unable to lock down " ++ show (fromMinCopies needmin) ++ - " " ++ pluralcopies (fromMinCopies needmin) ++ + " " ++ pluralCopies (fromMinCopies needmin) ++ " of file necessary to safely drop it." if null lockunsupported then showLongNote "(This could have happened because of a concurrent drop, or because a remote has too old a version of git-annex-shell installed.)" @@ -292,9 +296,10 @@ notEnoughCopies key neednum needmin have skip bad nolocmsg lockunsupported = do Remote.showTriedRemotes bad Remote.showLocations True key (map toUUID have++skip) nolocmsg - where - pluralcopies 1 = "copy" - pluralcopies _ = "copies" + +pluralCopies :: Int -> String +pluralCopies 1 = "copy" +pluralCopies _ = "copies" {- Finds locations of a key that can be used to get VerifiedCopies, - in order to allow dropping the key. diff --git a/doc/todo/option_to_explain.mdwn b/doc/todo/option_to_explain.mdwn index 1a504a2f15..29b6e3f496 100644 --- a/doc/todo/option_to_explain.mdwn +++ b/doc/todo/option_to_explain.mdwn @@ -38,3 +38,5 @@ included expression, eg: "standard(group==backup, include==*)" [[!tag confirmed]] --[[Joey]] [[!tag projects/dandi]] + +> [[done]] --[[Joey]] diff --git a/doc/todo/option_to_explain/comment_4_28919c6f816f11811b1e8c73b07403f7._comment b/doc/todo/option_to_explain/comment_4_28919c6f816f11811b1e8c73b07403f7._comment new file mode 100644 index 0000000000..fbeec53b7f --- /dev/null +++ b/doc/todo/option_to_explain/comment_4_28919c6f816f11811b1e8c73b07403f7._comment @@ -0,0 +1,32 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 4""" + date="2023-07-31T16:38:05Z" + content=""" +--explain now also shows when numcopies is taken into account when not +dropping, eg `get --auto`. + +When dropping, git-annex already explains when numcopies is not satisfied +of course. As to displaying the drop safety proof, I think it's too much in +the weeds for a user to want it explained? Users probably don't want to +worry about the distinction between RecentlyVerifiedCopy and LockedCopy, +for example; those details are git-annex's job. It already gives a well +calibrated explanation when it can't drop, so explaining when it can drop +seems unncessary. + +I also looked at adding it for file content checking, but inAnnex is pretty core +and is used in a lot of situations where --explain output would not be +useful, eg after git-annex has downloaded a file, it will check inAnnex. +While it would be possible to add something that explains that is only +called when deciding whether to operate on a file, that would be an +intrusive code change and would add complexity. + +Also, while it can be confusing to users sometimes why eg `git-annex get +foo` doesn't output anything due to the file being present already, it +seems like adding --explain output to that would generally not be useful to +more experienced users. Explain needs to remain relatively concise, or it's +just more debug output. + +So, closing this as completed, although of course more can be added +to --explain later.. +"""]]