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
<https://github.com/bgilbert/gcsannex> (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 9f3c2dfeda).
In new clones, they will find enableremote fails, complaining the
external program is not in path. An easy enough problem to recover from.
This commit is contained in:
Joey Hess 2020-04-23 14:59:38 -04:00
parent 9f3c2dfeda
commit 2aeb79249b
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
3 changed files with 43 additions and 1 deletions

View file

@ -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 <id@joeyh.name> Mon, 30 Mar 2020 15:58:34 -0400

View file

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

View file

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