From 2aeb79249b7cfc67ace3266f82f268e34be97161 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 23 Apr 2020 14:59:38 -0400 Subject: [PATCH] external: stop storing readonly=true in remote.log readonly=true is used to make an external special remote that does not need the external program to be installed. It was stored in the remote.log by default, and so every time it was specified in an enableremote or initremote, whatever value was used became the new default for subsequent enableremotes of that remote. That was surprising, and I consider it to be a bug. It does not make much sense to pass it to initremote because then how would you populate that remote with anything? You would have to enableremote elsewhere, and store content there. I'm assuming nobody used it that way. Someone might rely on passing it to enableremote once, and then that being inherited in other clones. But that is not how it's documented to be used. It is barely documented in git-annex at all, only in the external special remote protocol, and the documentation there says to "Document that this external special remote can be used in readonly mode." (by the user of it passing readonly=true to enableremote). The one external special remote that I know of that does document that is (the one that motivated adding it). That one's docs do say to pass it to enableremote. So, it seemed safe to make this behavior change. If someone was in fact relying on one of those behaviors, all their current repos will still work as they configured them (although they will need to deal with the related change in 9f3c2dfedae7ec840365f9578763209abae1005c). In new clones, they will find enableremote fails, complaining the external program is not in path. An easy enough problem to recover from. --- CHANGELOG | 2 + Remote/External.hs | 2 +- ..._bfbc9a9fbae3715fea0f4a11549ec8a9._comment | 40 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 doc/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_7_bfbc9a9fbae3715fea0f4a11549ec8a9._comment diff --git a/CHANGELOG b/CHANGELOG index 641b831d02..034b668278 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -29,6 +29,8 @@ git-annex (8.20200331) UNRELEASED; urgency=medium To disable running the external special remote program, now need to set remote.name.annex-externaltype=readonly. That is done when git-annex enableremote is passed readonly=true. + * Stop storing readonly=true in remote.log of external special remotes; + it is a local setting only. -- Joey Hess Mon, 30 Mar 2020 15:58:34 -0400 diff --git a/Remote/External.hs b/Remote/External.hs index 175d1abced..72f1a2c52c 100644 --- a/Remote/External.hs +++ b/Remote/External.hs @@ -201,7 +201,7 @@ externalSetup _ mu _ c gc = do return (changes c') gitConfigSpecialRemote u c'' [("externaltype", externaltype)] - return (c'', u) + return (M.delete readonlyField c'', u) checkExportSupported :: ParsedRemoteConfig -> RemoteGitConfig -> Annex Bool checkExportSupported c gc = do diff --git a/doc/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_7_bfbc9a9fbae3715fea0f4a11549ec8a9._comment b/doc/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_7_bfbc9a9fbae3715fea0f4a11549ec8a9._comment new file mode 100644 index 0000000000..a7f951b7b7 --- /dev/null +++ b/doc/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/comment_7_bfbc9a9fbae3715fea0f4a11549ec8a9._comment @@ -0,0 +1,40 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 7""" + date="2020-04-23T16:39:16Z" + content=""" +Looking closer at the affected special remote configs: + +* readonly=true is only used by external special remotes. It used to be + stored in the remote.log, but after examining it, I have changed that. + + So, it could be safely changed to also accept readonly=yes with no + back-compat issues. + +* When git-annex init is run, any autoenable= that is stored never results + in an error no matter what the value is; if it doesn't recognise it, it + assumes false. (And more generally, no value already stored in remote.log + causes a parse error, even if it cannot be parsed.) So autoenable=yes + would only result in older git-annex init not enabling the remote. Which + I guess is not a hard problem for a user to recover from, the remote + clearly won't be enabled when they try to use it. Two arguments in favor + of not worrying about backwards compatability for autoenable=yes: + + 1. When autoenable=true was added originally, we obviously didn't worry + that older git-annex's would ignore that config and not autoenable. + + 2. If the user does anything other than git-annex init, autoenabling + doesn't happen currently, which seems like rather more foot shooting + potential for ending up w/o a remote autoenabled, but I'm not seeing + much complaint about that. Though IIRC there might be a todo about it. + +So I'm leaning a bit toward it would be ok to accept yes/no as well as +true/false for those two, without worrying about translation to values old +git-annex will understand. + +But.. Allowing yes/no for these two while still also allowing true/false +does not remove the special case, it just buries it a bit. All the other +initremote booleans are yes/no and not true/false. And changing them +*would* need a translation layer. I don't feel that bit of uniformity is +worth that added complexity. +"""]]