diff mbox series

[v13,4/5] bugreport: add uname info

Message ID 20200416211807.60811-5-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series git-bugreport with fixed VS build | expand

Commit Message

Emily Shaffer April 16, 2020, 9:18 p.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                     | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason April 8, 2021, 10:19 p.m. UTC | #1
On Thu, Apr 16 2020, Emily Shaffer wrote:

> 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.

Even with _GNU_SOURCE would anyone care about the domainname (the NIS/YP
name, not DNS) these days, as opposed to the portable POSIX "nodename"
field you're not including?

In any case, I'd think it's a good idea to omit both. People tend not to
want to want to include their FQDN (e.g. their employer), and I can't
think of a reason we'd care about it for debugging git.

> [...]
> +		strbuf_addf(sys_info, "%s %s %s %s\n",
> +			    uname_info.sysname,
> +			    uname_info.release,
> +			    uname_info.version,
> +			    uname_info.machine);

Since this is completely free-form I'd think:

    "sysname: %s\nrelease: %s\nversion: %s\nmachine: %s\nnodename: %s\ndomainname: %s\n",

Or something like that would be better (after pruning out the fields we
don't care about).
Junio C Hamano April 8, 2021, 10:25 p.m. UTC | #2
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Thu, Apr 16 2020, Emily Shaffer wrote:
>
>> 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.
>
> Even with _GNU_SOURCE would anyone care about the domainname (the NIS/YP
> name, not DNS) these days, as opposed to the portable POSIX "nodename"
> field you're not including?
>
> In any case, I'd think it's a good idea to omit both. People tend not to
> want to want to include their FQDN (e.g. their employer), and I can't
> think of a reason we'd care about it for debugging git.
>
>> [...]
>> +		strbuf_addf(sys_info, "%s %s %s %s\n",
>> +			    uname_info.sysname,
>> +			    uname_info.release,
>> +			    uname_info.version,
>> +			    uname_info.machine);
>
> Since this is completely free-form I'd think:
>
>     "sysname: %s\nrelease: %s\nversion: %s\nmachine: %s\nnodename: %s\ndomainname: %s\n",
>
> Or something like that would be better (after pruning out the fields we
> don't care about).

All true.

By the way, what's this sudden interest in re-reviewing an age old
topic?
Ævar Arnfjörð Bjarmason April 8, 2021, 10:33 p.m. UTC | #3
On Fri, Apr 09 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Thu, Apr 16 2020, Emily Shaffer wrote:
>>
>>> 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.
>>
>> Even with _GNU_SOURCE would anyone care about the domainname (the NIS/YP
>> name, not DNS) these days, as opposed to the portable POSIX "nodename"
>> field you're not including?
>>
>> In any case, I'd think it's a good idea to omit both. People tend not to
>> want to want to include their FQDN (e.g. their employer), and I can't
>> think of a reason we'd care about it for debugging git.
>>
>>> [...]
>>> +		strbuf_addf(sys_info, "%s %s %s %s\n",
>>> +			    uname_info.sysname,
>>> +			    uname_info.release,
>>> +			    uname_info.version,
>>> +			    uname_info.machine);
>>
>> Since this is completely free-form I'd think:
>>
>>     "sysname: %s\nrelease: %s\nversion: %s\nmachine: %s\nnodename: %s\ndomainname: %s\n",
>>
>> Or something like that would be better (after pruning out the fields we
>> don't care about).
>
> All true.
>
> By the way, what's this sudden interest in re-reviewing an age old
> topic?

The thread got bumped by SZEDER in [1] and I'd read an April date
without noticing the year, so I see this has long-since landed,
nevermind :)

1. https://lore.kernel.org/git/20200416211807.60811-2-emilyshaffer@google.com/
Emily Shaffer April 8, 2021, 11:41 p.m. UTC | #4
On Fri, Apr 09, 2021 at 12:33:42AM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Fri, Apr 09 2021, Junio C Hamano wrote:
> > By the way, what's this sudden interest in re-reviewing an age old
> > topic?
> 
> The thread got bumped by SZEDER in [1] and I'd read an April date
> without noticing the year, so I see this has long-since landed,
> nevermind :)
> 
> 1. https://lore.kernel.org/git/20200416211807.60811-2-emilyshaffer@google.com/

Phew, you scared me :)

 - Emily
Junio C Hamano April 8, 2021, 11:58 p.m. UTC | #5
Emily Shaffer <emilyshaffer@google.com> writes:

> On Fri, Apr 09, 2021 at 12:33:42AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Apr 09 2021, Junio C Hamano wrote:
>> > By the way, what's this sudden interest in re-reviewing an age old
>> > topic?
>> 
>> The thread got bumped by SZEDER in [1] and I'd read an April date
>> without noticing the year, so I see this has long-since landed,
>> nevermind :)
>> 
>> 1. https://lore.kernel.org/git/20200416211807.60811-2-emilyshaffer@google.com/
>
> Phew, you scared me :)

Even if it has been in the releases, the room for improvement is
still there, no?
SZEDER Gábor April 9, 2021, 9:27 p.m. UTC | #6
On Thu, Apr 08, 2021 at 04:41:59PM -0700, Emily Shaffer wrote:
> On Fri, Apr 09, 2021 at 12:33:42AM +0200, Ævar Arnfjörð Bjarmason wrote:
> > On Fri, Apr 09 2021, Junio C Hamano wrote:
> > > By the way, what's this sudden interest in re-reviewing an age old
> > > topic?
> > 
> > The thread got bumped by SZEDER in [1] and I'd read an April date
> > without noticing the year, so I see this has long-since landed,
> > nevermind :)
> > 
> > 1. https://lore.kernel.org/git/20200416211807.60811-2-emilyshaffer@google.com/
> 
> Phew, you scared me :)

Doesn't the output of your bugreport tests scares you?

  Initialized empty Git repository in /home/szeder/src/git/t/trash directory.t0091-bugreport/.git/
  expecting success of 0091.1 'creates a report with content in the right places': 
  	test_when_finished rm git-bugreport-check-headers.txt &&
  	git bugreport -s check-headers &&
  	check_all_headers_populated <git-bugreport-check-headers.txt
  
  Created new report at 'git-bugreport-check-headers.txt'.
  grep: Thank you for filling out a Git bug report!: No such file or directory
  grep: Please answer the following questions to help us understand your issue.: No such file or directory
  grep: : No such file or directory
  grep: What did you do before the bug happened? (Steps to reproduce your issue): No such file or directory
  grep: : No such file or directory
  grep: What did you expect to happen? (Expected behavior): No such file or directory
  grep: : No such file or directory
  grep: What happened instead? (Actual behavior): No such file or directory
  grep: : No such file or directory
  grep: What's different between what you expected and what actually happened?: No such file or directory
  grep: : No such file or directory
  grep: Anything else you want to add:: No such file or directory
  grep: : No such file or directory
  grep: Please review the rest of the bug report below.: No such file or directory
  grep: You can delete any lines you don't wish to share.: No such file or directory
  grep: : No such file or directory
  grep: : No such file or directory
  grep: [System Info]: No such file or directory
  grep: git version:: No such file or directory
  grep: git version 2.31.0.7.ga9ff022d9b: No such file or directory
  grep: cpu: x86_64: No such file or directory
  grep: built from commit: a9ff022d9b49e64336612f89100eb5220ed793bd: No such file or directory
  grep: sizeof-long: 8: No such file or directory
  grep: sizeof-size_t: 8: No such file or directory
  grep: shell-path: /bin/sh: No such file or directory
  grep: uname: Linux 5.10.17-051017-generic #202102170631 SMP Wed Feb 17 11:37:41 UTC 2021 x86_64: No such file or directory
  grep: compiler info: gnuc: 9.3: No such file or directory
  grep: libc info: glibc: 2.31: No such file or directory
  grep: $SHELL (typically, interactive shell): /bin/bash: No such file or directory
  grep: : No such file or directory
  grep: : No such file or directory
  grep: [Enabled Hooks]: No such file or directory
  
  ok 1 - creates a report with content in the right places

It does scare me...
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 4cdb58bbaa..1a3172bcec 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -7,10 +7,24 @@ 
 
 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: ");
+	if (uname(&uname_info))
+		strbuf_addf(sys_info, _("uname() failed with error '%s' (%d)\n"),
+			    strerror(errno),
+			    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[] = {