[PATCH v3 2/2] usage: add warn_once() helper for repeated warnings
- From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Jun 1, 2022, 01:16 UTC
- Message-ID
- <8e29ac807c6a0cf94ea3a44ee3304011c2ad159c.1654046173.git.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1237.v3.git.1654046173.gitgitgadget@gmail.com>
From: Derrick Stolee <derrickstolee@github.com>
The previous change added a warning when valid_remote() detects credentials in the URL. Since remotes are validated multiple times per process, this causes multiple warnings to print.
To avoid these kinds of repeated, advisory warnings, create a new warn_once() helper that behaves the same as warning(), but only after formatting the output string and adding it to a strset. If that addition signals that the string already exists in the strset, then do not print the warning.
In the case of the credentials in a URL, the existing test demonstrates this per-process limitation: 'git clone' runs 'git-remote-curl' as a child process, giving two messages. This is an improvement over the previous six messages.
Signed-off-by: Derrick Stolee <derrickstolee@github.com> --- git-compat-util.h | 1 + remote.c | 2 +- t/t5601-clone.sh | 5 ++++- usage.c | 22 ++++++++++++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h index 58fd813bd01..776a5f660aa 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -526,6 +526,7 @@ int error(const char *err, ...) __attribute__((format (printf, 1, 2))); int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); void warning(const char *err, ...) __attribute__((format (printf, 1, 2))); void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); +void warn_once(const char *warn, ...) __attribute__((format (printf, 1, 2))); #ifndef NO_OPENSSL #ifdef APPLE_COMMON_CRYPTO diff --git a/remote.c b/remote.c index accf08bf51f..72fffd0d968 100644 --- a/remote.c +++ b/remote.c @@ -60,7 +60,7 @@ static void check_if_creds_in_url(const char *url) "<redacted>", 10); if (!strcmp("warn", value)) - warning(_("URL '%s' uses plaintext credentials"), redacted.buf); + warn_once(_("URL '%s' uses plaintext credentials"), redacted.buf); if (!strcmp("die", value)) die(_("URL '%s' uses plaintext credentials"), redacted.buf); diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index cba3553b7c4..6ae3eec9eb6 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -75,7 +75,10 @@ test_expect_success 'clone warns or fails when using username:password' ' test_must_fail git -c fetch.credentialsInUrl=allow clone https://username:password@localhost attempt1 2>err && ! grep "URL '\''https://username:<redacted>@localhost'\'' uses plaintext credentials" err && test_must_fail git -c fetch.credentialsInUrl=warn clone https://username:password@localhost attempt1 2>err && - grep "warning: URL '\''https://username:<redacted>@localhost'\'' uses plaintext credentials" err && + grep "warning: URL '\''https://username:<redacted>@localhost'\'' uses plaintext credentials" err >warnings && + # The warning is printed twice, for the two processes: + # "git clone" and "git-remote-curl". + test_line_count = 2 warnings && test_must_fail git -c fetch.credentialsInUrl=die clone https://username:password@localhost attempt2 2>err && grep "fatal: URL '\''https://username:<redacted>@localhost'\'' uses plaintext credentials" err ' diff --git a/usage.c b/usage.c index b738dd178b3..242633c5f8d 100644 --- a/usage.c +++ b/usage.c @@ -5,6 +5,7 @@ */ #include "git-compat-util.h" #include "cache.h" +#include "strmap.h" static void vreportf(const char *prefix, const char *err, va_list params) { @@ -287,6 +288,27 @@ void warning(const char *warn, ...) va_end(params); } +static struct strset prev_warnings = STRSET_INIT; + +void warn_once(const char *warn, ...) +{ + char buf[1024]; + va_list params; + va_start(params, warn); + + if (vsnprintf(buf, sizeof(buf), warn, params) >= 0) { + if (!strset_add(&prev_warnings, buf)) { + va_end(params); + return; + } + } + va_end(params); + + va_start(params, warn); + warn_routine(warn, params); + va_end(params); +} + /* Only set this, ever, from t/helper/, when verifying that bugs are caught. */ int BUG_exit_code;
-- gitgitgadget