diff mbox series

[v8,05/15] bugreport: add uname info

Message ID 20200220015858.181086-6-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series add git-bugreport tool | expand

Commit Message

Emily Shaffer Feb. 20, 2020, 1:58 a.m. UTC
The contents of uname() can give us some insight into what sort of
system the user is running on, and help us replicate their setup if need
be. The domainname field is not guaranteed to be available, so don't
collect it.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     | 13 +++++++++++++
 2 files changed, 14 insertions(+)

Comments

Junio C Hamano Feb. 20, 2020, 8:12 p.m. UTC | #1
Emily Shaffer <emilyshaffer@google.com> writes:

> +	/* system call for other version info */
> +	strbuf_addstr(sys_info, "uname -a: ");

Is that "-a" portable across systems?  That is, when given "-a",
would "uname" on all platforms show sysname, release, version and
machine in that order and nothing else?

If we are merely saying "in this section, we are showing a selected
subset from what we learned about the system with uname(2)", perhaps
just stop at saying "uname: "?

> +	if (uname(&uname_info))
> +		strbuf_addf(sys_info, "uname() failed with code %d\n", errno);

i18n/l10n?  Don't we want to use strerror() here?

> +	else
> +		strbuf_addf(sys_info, "%s %s %s %s\n",
> +			    uname_info.sysname,
> +			    uname_info.release,
> +			    uname_info.version,
> +			    uname_info.machine);
>  }
>  
>  static const char * const bugreport_usage[] = {
Emily Shaffer Feb. 20, 2020, 11:20 p.m. UTC | #2
On Thu, Feb 20, 2020 at 12:12:58PM -0800, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > +	/* system call for other version info */
> > +	strbuf_addstr(sys_info, "uname -a: ");
> 
> Is that "-a" portable across systems?  That is, when given "-a",
> would "uname" on all platforms show sysname, release, version and
> machine in that order and nothing else?
> 
> If we are merely saying "in this section, we are showing a selected
> subset from what we learned about the system with uname(2)", perhaps
> just stop at saying "uname: "?

Sure, that's good enough for me. I think actually 'uname -a' on a GNU
system isn't going to match what's output here anyways, since I don't
show the nodename and 'uname -a' would.

> 
> > +	if (uname(&uname_info))
> > +		strbuf_addf(sys_info, "uname() failed with code %d\n", errno);
> 
> i18n/l10n?  Don't we want to use strerror() here?

Much much earlier[1] in the series, you suggested not i18n/l10n the
contents of the bugreport, as this list would probably be ill-equipped
to process such a report. Since it's being added to sys_info, that means this line
is bound for the finished report, not for the user.

Speaking of locale, strerror() will print the error in the current
locale; if we're still assuming "the list only takes reports in English"
should I be explicit and use strerror_l()? Should I print strerror() in
current locale and include the errno, e.g. ("uname() failed with '%s'
(%d)\n", strerror(errno), errno)?

[1] https://lore.kernel.org/git/xmqqzhka2tbv.fsf@gitster-ct.c.googlers.com
diff mbox series

Patch

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index f44ae8cbe7..17b0d14e8d 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -26,6 +26,7 @@  The following information is requested from the user:
 The following information is captured automatically:
 
  - 'git version --build-options'
+ - uname sysname, release, version, and machine strings
 
 This tool is invoked via the typical Git setup process, which means that in some
 cases, it might not be able to launch - for example, if a relevant config file
diff --git a/bugreport.c b/bugreport.c
index 27f813643d..06a63cc283 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -7,10 +7,23 @@ 
 
 static void get_system_info(struct strbuf *sys_info)
 {
+	struct utsname uname_info;
+
 	/* get git version from native cmd */
 	strbuf_addstr(sys_info, "git version:\n");
 	get_version_info(sys_info, 1);
 	strbuf_complete_line(sys_info);
+
+	/* system call for other version info */
+	strbuf_addstr(sys_info, "uname -a: ");
+	if (uname(&uname_info))
+		strbuf_addf(sys_info, "uname() failed with code %d\n", errno);
+	else
+		strbuf_addf(sys_info, "%s %s %s %s\n",
+			    uname_info.sysname,
+			    uname_info.release,
+			    uname_info.version,
+			    uname_info.machine);
 }
 
 static const char * const bugreport_usage[] = {