usage: add trace2 entry upon warning()

Emit a trace2 error event whenever warning() is called, just like when
die(), error(), or usage() is called.

This helps debugging issues that would trigger warnings but not errors.
In particular, this might have helped debugging an issue I encountered
with commit graphs at $DAYJOB [1].

There is a tradeoff between including potentially relevant messages and
cluttering up the trace output produced. I think that warning() messages
should be included in traces, because by its nature, Git is used over
multiple invocations of the Git tool, and a failure (currently traced)
in a Git invocation might be caused by an unexpected interaction in a
previous Git invocation that only has a warning (currently untraced) as
a symptom - as is the case in [1].

[1] https://lore.kernel.org/git/20200629220744.1054093-1-jonathantanmy@google.com/

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
index 6b60855..c65ffaf 100644 (file)
--- a/Documentation/technical/api-trace2.txt
+++ b/Documentation/technical/api-trace2.txt
@@ -466,7 +466,7 @@ completed.)
 
 `"error"`::
        This event is emitted when one of the `error()`, `die()`,
-       or `usage()` functions are called.
+       `warning()`, or `usage()` functions are called.
 +
 ------------
 {

diff --git a/usage.c b/usage.c
index 58fb5ff..7c58b26 100644 (file)
--- a/usage.c
+++ b/usage.c
@@ -81,6 +81,12 @@ static void error_builtin(const char *err, va_list params)
 
 static void warn_builtin(const char *warn, va_list params)
 {
+       /*
+        * We call this trace2 function first and expect it to va_copy 'params'
+        * before using it (because an 'ap' can only be walked once).
+        */
+       trace2_cmd_error_va(warn, params);
+
        vreportf("warning: ", warn, params);
 }