stop using remote.name.annex-readonly for two distinct things

CHANGELOG

@@ -23,6 +23,12 @@ git-annex (8.20200331) UNRELEASED; urgency=medium
     to silently skip files that the location log thought were present on the
     remote, when the remote actually no longer contained them. Since that
     behavior could be surprising, now instead display a warning.
+  * external special remotes: remote.name.annex-readonly=true no longer
+    disables running the external special remote program. Instead, it just
+    makes the remote operate in a readonly mode, same as any remote.
+    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.
 
  -- Joey Hess <id@joeyh.name>  Mon, 30 Mar 2020 15:58:34 -0400
 

Remote/External.hs

@@ -65,7 +65,7 @@ readonlyField = Accepted "readonly"
 gen :: Git.Repo -> UUID -> RemoteConfig -> RemoteGitConfig -> RemoteStateHandle -> Annex (Maybe Remote)
 gen r u rc gc rs
 	-- readonly mode only downloads urls; does not use external program
-	| remoteAnnexReadOnly gc = do
+	| externaltype == "readonly" = do
 		c <- parsedRemoteConfig remote rc
 		cst <- remoteCost gc expensiveRemoteCost
 		mk c cst GloballyAvailable
@@ -165,15 +165,22 @@ externalSetup :: SetupStage -> Maybe UUID -> Maybe CredPair -> RemoteConfig -> R
 externalSetup _ mu _ c gc = do
 	u <- maybe (liftIO genUUID) return mu
 	pc <- either giveup return $ parseRemoteConfig c lenientRemoteConfigParser
-	let externaltype = fromMaybe (giveup "Specify externaltype=") $
-		getRemoteConfigValue externaltypeField pc
+	let readonlyconfig = getRemoteConfigValue readonlyField pc == Just True
+	let externaltype = if readonlyconfig
+		then "readonly"
+		else fromMaybe (giveup "Specify externaltype=") $
+			getRemoteConfigValue externaltypeField pc
 	(c', _encsetup) <- encryptionSetup c gc
 
-	c'' <- case getRemoteConfigValue readonlyField pc of
-		Just True -> do
+	c'' <- if readonlyconfig
+		then do
+			-- Setting annex-readonly is not really necessary
+			-- anymore, but older versions of git-annex used
+			-- this, not externaltype=readonly, so still set
+			-- it.
 			setConfig (remoteAnnexConfig (fromJust (lookupName c)) "readonly") (boolConfig True)
 			return c'
-		_ -> do
+		else do
 			pc' <- either giveup return $ parseRemoteConfig c' lenientRemoteConfigParser
 			external <- newExternal externaltype (Just u) pc' (Just gc) Nothing
 			-- Now that we have an external, ask it to LISTCONFIGS, 
@@ -200,8 +207,10 @@ checkExportSupported :: ParsedRemoteConfig -> RemoteGitConfig -> Annex Bool
 checkExportSupported c gc = do
 	let externaltype = fromMaybe (giveup "Specify externaltype=") $
 		remoteAnnexExternalType gc <|> getRemoteConfigValue externaltypeField c
-	checkExportSupported' 
-		=<< newExternal externaltype Nothing c (Just gc) Nothing
+	if externaltype == "readonly"
+		then return False
+		else checkExportSupported' 
+			=<< newExternal externaltype Nothing c (Just gc) Nothing
 
 checkExportSupported' :: External -> Annex Bool
 checkExportSupported' external = go `catchNonAsync` (const (return False))
@@ -649,7 +658,16 @@ startExternal external = do
 		giveup $ "Cannot run " ++ cmd ++ " -- Make sure it's executable and that its dependencies are installed."
 	runerr Nothing _ = do
 		path <- intercalate ":" <$> getSearchPath
-		giveup $ "Cannot run " ++ basecmd ++ " -- It is not installed in PATH (" ++ path ++ ")"
+		let err = "Cannot run " ++ basecmd ++ " -- It is not installed in PATH (" ++ path ++ ")"
+		case (lookupName (unparsedRemoteConfig (externalDefaultConfig external)), remoteAnnexReadOnly <$> externalGitConfig external) of
+			(Just rname, Just True) -> giveup $ unlines
+				[ err
+				, "This remote has annex-readonly=true, and previous versions of"
+				, "git-annex would tried to download from it without"
+				, "installing " ++ basecmd ++ ". If you want that, you need to set:"
+				, "git config remote." ++ rname ++ ".annex-externaltype readonly"
+				]
+			_ -> giveup err
 
 stopExternal :: External -> Annex ()
 stopExternal external = liftIO $ do

doc/bugs/checkpresentkey_fails_on_read-only_remote.mdwn

@@ -8,3 +8,4 @@ In an external special remote, if I set annex-readonly=true, I get
 
 Removing the annex-readonly setting lets checkpresentkey work.   But isn't checkpresentkey a read-only operation?
 
+> [[fixed|done]] --[[Joey]]

doc/bugs/checkpresentkey_fails_on_read-only_remote/comment_3_04859f6abcb548f32ff6ce12d858fae5._comment

@@ -0,0 +1,50 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 3"""
+ date="2020-04-23T17:13:42Z"
+ content="""
+So, enableremote readonly=true sets remote.name.annex-readonly
+in git config. And it assumes that, if that is set, you don't want to use
+the external special remote program at all, but instead a previous use of
+it elsewhere should have stored the urls where git-annex can download content
+stored in that remote.
+
+The use case for enableremote readonly=true is that you want to provide a
+way for users to get content you have stored in your remote without the
+bother of installing a third-party program to access it. While you could
+set remote.name.annex-readonly in git config after enableremote,
+you would need to have the program installed for the enableremote step,
+and readonly=true avoids that.
+
+It's the same as `git annex copy --to s3` registering the content as stored
+in the s3 remote, not the web remote, even if git-annex knows there's a
+publically available url that can be used to access it.
+
+If git-annex treated that as also storing the content to the web remote,
+then it would be maintaining two sets of books for the same copy of the file.
+So instead the user needs to enable use of the s3 remote 
+(even if without any S3 creds in a necessarily read-only mode) in order for
+git-annex to access files stored on it. And same with these external
+special remotes, except their code is not built into git-annex, so
+readonly=true provides a way to not need to run their code at all.
+
+Now, you did not pass readonly=true to enableremote from what I
+understand, but instead came along later and set
+remote.name.annex-readonly=true in git config. And I think your goal was to
+keep using the special remote program for eg downloads, but prevent
+writing to the remote.
+
+So yes, it would be better if enableremote readonly=true set some other
+config than remote.name.annex-readonly. As it is, there's no way to
+distinguish between the two use cases.
+
+---
+
+Ok, I'm gonna make remote.name.annex-externaltype=readonly be a special
+case that avoids running the external special remote program. 
+
+In the case where no program is available, it will check if
+remote.name.annex-readonly is set, and if so when it fails it will
+suggest the user might want to set annex-externaltype=readonly to deal with
+this change.
+"""]]

doc/git-annex.mdwn

@@ -1527,10 +1527,21 @@ Remotes are configured using these settings in `.git/config`.
 
   It is set to "true" if this is a git-lfs remote.
 
-* `remote.<name>.annex-hooktype`, `remote.<name>.annex-externaltype`
+* `remote.<name>.annex-externaltype`
 
-  Used by hook special remotes and external special remotes to record
-  the type of the remote.
+  Used external special remotes to record the type of the remote.
+
+  Eg, if this is set to "foo", git-annex will run a "git-annex-remote-foo"
+  program to communicate with the external special remote.
+
+  If this is set to "readonly", then git-annex will not run any external
+  special remote program, but will try to access things stored in the
+  remote using http. That only works for some external special remotes,
+  so consult the documentation of the one you are using.
+
+* `remote.<name>.annex-hooktype`
+
+  Used by hook special remotes to record the type of the remote.
 
 * `annex.web-options`