fix implicit embedcreds regression

Fix bug that made creds not be stored in git when a special remote was
initialized with gpg encryption, but without an explicit embedcreds=yes.

(Yet nother regression introduced in version 7.20200202.7. 5th so far.)

Annex/SpecialRemote/Config.hs

@@ -104,11 +104,11 @@ commonFieldParsers =
 	, optionalStringParser sameasUUIDField HiddenField
 	, optionalStringParser typeField
 		(FieldDesc "type of special remote")
-	, trueFalseParser autoEnableField False
+	, trueFalseParser autoEnableField (Just False)
 		(FieldDesc "automatically enable special remote")
-	, yesNoParser exportTreeField False
+	, yesNoParser exportTreeField (Just False)
 		(FieldDesc "export trees of files to this remote")
-	, yesNoParser importTreeField False
+	, yesNoParser importTreeField (Just False)
 		(FieldDesc "import trees of files from this remote")
 	, optionalStringParser preferreddirField
 		(FieldDesc "directory whose content is preferred")
@@ -232,16 +232,16 @@ optionalStringParser f fielddesc = RemoteConfigFieldParser
 	p (Just v) _c = Right (Just (RemoteConfigValue (fromProposedAccepted v)))
 	p Nothing _c = Right Nothing
 
-yesNoParser :: RemoteConfigField -> Bool -> FieldDesc -> RemoteConfigFieldParser
-yesNoParser f v fd = genParser yesno f v fd
+yesNoParser :: RemoteConfigField -> Maybe Bool -> FieldDesc -> RemoteConfigFieldParser
+yesNoParser f mdef fd = genParser yesno f mdef fd
 	(Just (ValueDesc "yes or no"))
   where
 	yesno "yes" = Just True
 	yesno "no" = Just False
 	yesno _ = Nothing
 
-trueFalseParser :: RemoteConfigField -> Bool -> FieldDesc -> RemoteConfigFieldParser
-trueFalseParser f v fd = genParser trueFalseParser' f v fd
+trueFalseParser :: RemoteConfigField -> Maybe Bool -> FieldDesc -> RemoteConfigFieldParser
+trueFalseParser f mdef fd = genParser trueFalseParser' f mdef fd
 	(Just (ValueDesc "true or false"))
 
 -- Not using Git.Config.isTrueFalse because git supports
@@ -256,22 +256,22 @@ genParser
 	:: Typeable t
 	=> (String -> Maybe t)
 	-> RemoteConfigField
-	-> t -- ^ fallback value
+	-> Maybe t -- ^ default if not configured
 	-> FieldDesc
 	-> Maybe ValueDesc
 	-> RemoteConfigFieldParser
-genParser parse f fallback fielddesc valuedesc = RemoteConfigFieldParser	
+genParser parse f mdef fielddesc valuedesc = RemoteConfigFieldParser	
 	{ parserForField = f
 	, valueParser = p
 	, fieldDesc = fielddesc
 	, valueDesc = valuedesc
 	}
   where
-	p Nothing _c = Right (Just (RemoteConfigValue fallback))
+	p Nothing _c = Right (fmap RemoteConfigValue mdef)
 	p (Just v) _c = case parse (fromProposedAccepted v) of
 		Just b -> Right (Just (RemoteConfigValue b))
 		Nothing -> case v of
-			Accepted _ -> Right (Just (RemoteConfigValue fallback))
+			Accepted _ -> Right (fmap RemoteConfigValue mdef)
 			Proposed _ -> Left $
 				"Bad value for " ++ fromProposedAccepted f ++
 				case valuedesc of

CHANGELOG

@@ -50,6 +50,10 @@ git-annex (8.20200523) UNRELEASED; urgency=medium
     branch. git-annex-remote-googledrive one was special remote affected by
     this bug.
     (Regression introduced in version 7.20200202.7)
+  * Fix bug that made creds not be stored in git when a special remote
+    was initialized with gpg encryption, but without an explicit
+    embedcreds=yes.
+    (Regression introduced in version 7.20200202.7)
 
  -- Joey Hess <id@joeyh.name>  Tue, 26 May 2020 10:20:52 -0400
 

Remote/External.hs

@@ -835,7 +835,7 @@ baseRemoteConfigParser = RemoteConfigParser
 	{ remoteConfigFieldParsers =
 		[ optionalStringParser externaltypeField
 			(FieldDesc "type of external special remote to use")
-		, trueFalseParser readonlyField False
+		, trueFalseParser readonlyField (Just False)
 			(FieldDesc "enable readonly mode")
 		]
 	, remoteConfigRestPassthrough = Just

Remote/Helper/Encryptable.hs

@@ -59,7 +59,7 @@ encryptionConfigParsers =
 	, optionalStringParser cipherField HiddenField
 	, optionalStringParser cipherkeysField HiddenField
 	, optionalStringParser pubkeysField HiddenField
-	, yesNoParser embedCredsField False
+	, yesNoParser embedCredsField Nothing
 		(FieldDesc "embed credentials into git repository")
 	, macFieldParser
 	, optionalStringParser (Accepted "keyid")

Remote/Rsync.hs

@@ -128,8 +128,8 @@ gen r u rc gc rs = do
 -- Things used by genRsyncOpts
 rsyncRemoteConfigs :: [RemoteConfigFieldParser]
 rsyncRemoteConfigs = 
-	[ yesNoParser shellEscapeField True
-		(FieldDesc "avoid usual shell escaping (not recommended)")
+	[ yesNoParser shellEscapeField (Just True)
+		(FieldDesc "set to no to avoid usual shell escaping (not recommended)")
 	]
 
 genRsyncOpts :: ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts

Remote/S3.hs

@@ -87,10 +87,10 @@ remote = specialRemoteType $ RemoteType
 				(FieldDesc "storage class, eg STANDARD or STANDARD_IA or ONEZONE_IA")
 			, optionalStringParser fileprefixField
 				(FieldDesc "prefix to add to filenames in the bucket")
-			, yesNoParser versioningField False
+			, yesNoParser versioningField (Just False)
 				(FieldDesc "enable versioning of bucket content")
-			, yesNoParser publicField False
-				(FieldDesc "allow public read access to the buckey")
+			, yesNoParser publicField (Just False)
+				(FieldDesc "allow public read access to the bucket")
 			, optionalStringParser publicurlField
 				(FieldDesc "url that can be used by public to download files")
 			, optionalStringParser protocolField
@@ -157,7 +157,7 @@ newtype SignatureVersion = SignatureVersion Int
 
 signatureVersionParser :: RemoteConfigField -> FieldDesc -> RemoteConfigFieldParser
 signatureVersionParser f fd =
-	genParser go f defver fd
+	genParser go f (Just defver) fd
 		(Just (ValueDesc "v2 or v4"))
   where
 	go "v2" = Just (SignatureVersion 2)

doc/bugs/embedcreds_with_external_special_remotes/comment_3_b2b4c8997129328531aa93db828c26e3._comment

@@ -8,15 +8,22 @@ could break existing workflows in a surprising way, and even maybe result
 in data loss if the user was relying on git-annex embedding the creds and
 didn't otherwise have a way to get them.
 
-The cause is that there's a externalConfigChanges that SETCONFIG
+It's (yet another) regression caused by 7.20200202.7's sweeping changes to
+remote configuration.
+
+Problem is, there's a externalConfigChanges that SETCONFIG
 updates, but SETCREDS does not, instead it swap in a new externalConfig.
 But that externalConfig is not examined when extracting the config changes
 to store in remote.log, because the types don't match up any longer.
 
 So, SETCREDS needs to also update externalConfigChanges.
 
-Related reversion: When SETCONFIG is used, followed by GETCONFIG
+Related regression: When SETCONFIG is used, followed by GETCONFIG
 of the same value, it does not return the value. This doesn't affect
 SETCONFIG at init time followed by GETCONFIG later, so it's unlikely to
 affect anything, but it's still wrong, and so I've fixed it.
+
+And they don't stop coming, yet another regression: Not setting embedcreds
+was treated as embedcreds=no, because the bool parser defaulted to False.
+So the implicit embedcreds when using encryption=pubkey broke.
 """]]