Hi Emily,

On Fri, 8 Nov 2019, Emily Shaffer wrote:

On Mon, Oct 28, 2019 at 02:49:29PM +0100, Johannes Schindelin wrote:

quoted

On Thu, 24 Oct 2019, Emily Shaffer wrote:

quoted

diff --git a/bugreport.c b/bugreport.c
new file mode 100644
index 0000000000..ada54fe583
--- /dev/null
+++ b/bugreport.c

[...]
+	strbuf_addstr(sys_info, "curl-config --version: ");
+	strbuf_addbuf(sys_info, &std_out);
+	strbuf_complete_line(sys_info);
+
+	argv_array_clear(&cp.args);
+	strbuf_reset(&std_out);
+
+
+	argv_array_push(&cp.args, "ldd");
+	argv_array_push(&cp.args, "--version");
+	capture_command(&cp, &std_out, 0);

Again, this command will only be present in few setups. I am not
actually sure that the output of this is interesting to begin with.

It was a suggestion, I believe, from Jonathan Nieder.

Yes, I guess Jonathan builds their Git locally, too.

It _is_ easy to forget that most users find this too involved to even
try.

Nothing like reading through a bug tracker quite frequently to learn
about the actual troubles actual users have :-)

quoted

What I _do_ think is that a much more interesting piece of information
would be the exact GLIBC version, the OS name and the OS version.

The glibc version is easy; I've done that. It certainly looks nicer than
the ldd call.

I guess I may be missing something, because as I start to look into how
to the OS info, I fall down a hole of many, many preprocessor defines to
check. If that's the approach you want me to take, just say the word,
but it will be ugly :) I suppose I had hoped the uname info would give us
a close enough idea that full OS info isn't necessary.

We could go down the pre-processor route, but that would give us the OS
name and version at build time, not at run time. I think we will be
mostly interested in the latter, though.

We might need to enhance our `uname()` emulation in `compat/mingw.c`,
but I think we already have enough information there.

When it comes to glibc, I think `gnu_get_libc_version()` would get us
what we want. A trickier thing might be to determine whether we're
actually linking against glibc; I do not want to break musl builds
again, I already did that inadvertently when requiring `REG_STARTEND`
back in the days.

quoted

quoted

diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index 2ef16440a0..7232d31be7 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c

@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "bugreport.h"
 #include "stdio.h"
 #include "strbuf.h"
 #include "time.h"

@@ -27,6 +28,13 @@ int get_bug_template(struct strbuf *template)
 	return 0;
 }

+void add_header(FILE *report, const char *title)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	strbuf_addf(&buffer, "\n\n[%s]\n", title);
+	strbuf_write(&buffer, report);

This leaks `buffer`. Why not write into `report` via `fprintf()`
directly?

Rather, to match the style of the rest of the builtin, modified
get_header to add the header to a passed-in strbuf instead of
modifying the file directly.

Hmm. It makes the code less elegant in my opinion. I would rather either
render the entire bug report into a single `strbuf` and then write it
via `write_in_full()`, or use `fprintf()` directly.

Ciao,
Dscho