Thread (23 messages) flat view 23 messages, 7 authors, 2021-10-29

Re: *Really* noisy encoding warnings post-v2.33.0

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-10-22 23:00:29

On Fri, Oct 08 2021, Jeff King wrote:
quoted hunk ↗ jump to hunk
On Fri, Oct 08, 2021 at 10:36:02PM -0400, Jeff King wrote:
quoted
If that were coupled with, say, an advise() call to explain that output
and matching might be inaccurate (and show that _once_), that might
might it more clear what's going on.

Now I am sympathetic to flooding the user with too many messages, and
maybe reducing this to a single instance of "some commit messages could
not be re-encoded; output and matching might be inaccurate" is the right
thing. But in a sense, it's also working as designed: what you asked for
is producing wrong output over and over, and Git is saying so.
The single-output version would perhaps be something like this:
diff --git a/pretty.c b/pretty.c
index 708b618cfe..c86f41bae7 100644
--- a/pretty.c
+++ b/pretty.c
@@ -606,6 +606,21 @@ static char *replace_encoding_header(char *buf, const char *encoding)
 	return strbuf_detach(&tmp, NULL);
 }
 
+static void show_encoding_warning(const char *output_encoding)
+{
+	static int seen_warning;
+
+	if (seen_warning)
+		return;
+
+	seen_warning = 1;
+	warning("one or more commits could not be re-encoded to '%s'",
+		output_encoding);
+	advise("When re-encoding fails, some output may be in an unexpected\n"
+	       "encoding, and pattern matches against commit data may be\n"
+	       "inaccurate.");
+}
+
 const char *repo_logmsg_reencode(struct repository *r,
 				 const struct commit *commit,
 				 char **commit_encoding,
@@ -673,7 +688,7 @@ const char *repo_logmsg_reencode(struct repository *r,
 	 * case we just return the commit message verbatim.
 	 */
 	if (!out) {
-		warning("unable to reencode commit to '%s'", output_encoding);
+		show_encoding_warning(output_encoding);
 		return msg;
 	}
 	return out;
*Poke* about this. We're getting pretty close to release. I think the
WIP hunk I posted in
https://lore.kernel.org/git/871r4umfnm.fsf@evledraar.gmail.com/ (local) presents
a good way forward with this.

I.e. aside from how wise it is to warn about this in general, I think
there's a pretty bad bug in your implementation where what should
effectively be a parse_options() or git_config()-time one-off warning is
being fired off for every single commit in "git log" in some cases.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help