From 4773713cc932e800729d6d67ce2276f45ccb560d Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 16 Jun 2020 15:16:36 -0400 Subject: [PATCH] analysis of regression and fix related less serious regression --- CHANGELOG | 3 +++ Remote/External.hs | 6 +++++- ...mment_3_b2b4c8997129328531aa93db828c26e3._comment | 12 ++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index f93f3c0775..9de05d25f7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,6 +42,9 @@ git-annex (8.20200523) UNRELEASED; urgency=medium * checkpresentkey: When no remote is specified, try all remotes, not only ones that the location log says contain the key. This is what the documentation has always said it did. + * Fix regression in external special remote handling: GETCONFIG did not + return a value that was set with SETCONFIG immediately before. + (Regression introduced in version 7.20200202.7) -- Joey Hess Tue, 26 May 2020 10:20:52 -0400 diff --git a/Remote/External.hs b/Remote/External.hs index 7d0a3b17c5..97a013f367 100644 --- a/Remote/External.hs +++ b/Remote/External.hs @@ -414,7 +414,11 @@ handleRequest' st external req mp responsehandler (Accepted setting) (RemoteConfigValue (PassedThrough value)) m - in ParsedRemoteConfig m' c + c' = M.insert + (Accepted setting) + (Accepted value) + c + in ParsedRemoteConfig m' c' modifyTVar' (externalConfigChanges st) $ \f -> f . M.insert (Accepted setting) (Accepted value) handleRemoteRequest (GETCONFIG setting) = do diff --git a/doc/bugs/embedcreds_with_external_special_remotes/comment_3_b2b4c8997129328531aa93db828c26e3._comment b/doc/bugs/embedcreds_with_external_special_remotes/comment_3_b2b4c8997129328531aa93db828c26e3._comment index 75be126c6b..0ef1a75ec3 100644 --- a/doc/bugs/embedcreds_with_external_special_remotes/comment_3_b2b4c8997129328531aa93db828c26e3._comment +++ b/doc/bugs/embedcreds_with_external_special_remotes/comment_3_b2b4c8997129328531aa93db828c26e3._comment @@ -7,4 +7,16 @@ Confirmed this bug. This feels like it should trigger a release, as it 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 +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 +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. """]]