{"thread":{"id":"53456","subject":"[PATCH 0/2] bugreport: collect shell settings","startedAt":"2020-05-12T23:42:21Z","lastAt":"2020-05-13T18:45:15Z","messageCount":7,"participants":["Emily Shaffer","brian m. carlson","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"397641","messageId":"20200512234213.63651-1-emilyshaffer@google.com","threadId":"53456","inReplyTo":null,"subject":"[PATCH 0/2] bugreport: collect shell settings","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-05-12T23:42:11Z","receivedAt":"2020-05-12T23:42:21Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Note: To avoid merge conflicts, this is based on\nes/bugreport-with-hooks. It doesn't rely on any of the content in that\npatch and the conflicts would be straightforward to resolve, but they\nwould be annoying. (If that's not the right way to go about something\nlike this, let me know - I've got a handful more bugreport topics to go\nafter this one.)\n\nThe first patch in the series captures the shell we use during build\n(e.g. to run GIT-VERSION-GEN) and the second patch captures the user\nshell at runtime.\n\n - Emily\n\nEmily Shaffer (2):\n help: add shell-path to --build-options\n bugreport: include user interactive shell\n\n Documentation/git-bugreport.txt | 1 +\n bugreport.c | 6 ++++++\n help.c | 1 +\n 3 files changed, 8 insertions(+)\n\n-- \n2.26.2.645.ge9eca65c58-goog\n\n"},{"id":"397642","messageId":"20200512234213.63651-2-emilyshaffer@google.com","threadId":"53456","inReplyTo":"20200512234213.63651-1-emilyshaffer@google.com","subject":"[PATCH 1/2] help: add shell-path to --build-options","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-05-12T23:42:12Z","receivedAt":"2020-05-12T23:42:23Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"It may be useful to know which shell Git was built to try to point to,\nin the event that shell-based Git commands are failing. $SHELL_PATH is\nset during the build and used to launch the manpage viewer, as well as\nby git-compat-util.h, and it's used during tests. 'git version\n--build-options' is encouraged for use in bug reports, so it makes sense\nto include this information there.\n\nSigned-off-by: Emily Shaffer \n---\n help.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/help.c b/help.c\nindex 1de9c0d589..44cee69c11 100644\n--- a/help.c\n+++ b/help.c\n@@ -641,6 +641,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)\n \t\t\tstrbuf_addstr(buf, \"no commit associated with this build\\n\");\n \t\tstrbuf_addf(buf, \"sizeof-long: %d\\n\", (int)sizeof(long));\n \t\tstrbuf_addf(buf, \"sizeof-size_t: %d\\n\", (int)sizeof(size_t));\n+\t\tstrbuf_addf(buf, \"shell-path: %s\\n\", SHELL_PATH);\n \t\t/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */\n \t}\n }\n-- \n2.26.2.645.ge9eca65c58-goog\n\n"},{"id":"397643","messageId":"20200512234213.63651-3-emilyshaffer@google.com","threadId":"53456","inReplyTo":"20200512234213.63651-1-emilyshaffer@google.com","subject":"[PATCH 2/2] bugreport: include user interactive shell","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-05-12T23:42:13Z","receivedAt":"2020-05-12T23:42:25Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"It's possible a user may complain about the way that Git interacts with\ntheir interactive shell, e.g. autocompletion or shell prompt. In that\ncase, it's useful for us to know which shell they're using\ninteractively.\n\nSigned-off-by: Emily Shaffer \n---\n Documentation/git-bugreport.txt | 1 +\n bugreport.c | 6 ++++++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt\nindex 7fe9aef34e..4afc48c452 100644\n--- a/Documentation/git-bugreport.txt\n+++ b/Documentation/git-bugreport.txt\n@@ -29,6 +29,7 @@ The following information is captured automatically:\n - uname sysname, release, version, and machine strings\n - Compiler-specific info string\n - A list of enabled hooks\n+ - $SHELL\n \n This tool is invoked via the typical Git setup process, which means that in some\n cases, it might not be able to launch - for example, if a relevant config file\ndiff --git a/bugreport.c b/bugreport.c\nindex aa8a489c35..28f4568b01 100644\n--- a/bugreport.c\n+++ b/bugreport.c\n@@ -9,6 +9,7 @@\n static void get_system_info(struct strbuf *sys_info)\n {\n \tstruct utsname uname_info;\n+\tchar *shell = NULL;\n \n \t/* get git version from native cmd */\n \tstrbuf_addstr(sys_info, _(\"git version:\\n\"));\n@@ -29,8 +30,13 @@ static void get_system_info(struct strbuf *sys_info)\n \n \tstrbuf_addstr(sys_info, _(\"compiler info: \"));\n \tget_compiler_info(sys_info);\n+\n \tstrbuf_addstr(sys_info, _(\"libc info: \"));\n \tget_libc_info(sys_info);\n+\n+\tshell = getenv(\"SHELL\");\n+\tstrbuf_addf(sys_info, \"$SHELL (typically, interactive shell): %s\\n\",\n+\t\t shell ? shell : \"\");\n }\n \n static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n-- \n2.26.2.645.ge9eca65c58-goog\n\n"},{"id":"397644","messageId":"20200512235924.GC6605@camp.crustytoothpaste.net","threadId":"53456","inReplyTo":"20200512234213.63651-2-emilyshaffer@google.com","subject":"Re: [PATCH 1/2] help: add shell-path to --build-options","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-05-12T23:59:24Z","receivedAt":"2020-05-12T23:59:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-05-12 at 23:42:12, Emily Shaffer wrote:\n> It may be useful to know which shell Git was built to try to point to,\n> in the event that shell-based Git commands are failing. $SHELL_PATH is\n> set during the build and used to launch the manpage viewer, as well as\n> by git-compat-util.h, and it's used during tests. 'git version\n> --build-options' is encouraged for use in bug reports, so it makes sense\n> to include this information there.\n> \n> Signed-off-by: Emily Shaffer \n> ---\n> help.c | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/help.c b/help.c\n> index 1de9c0d589..44cee69c11 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -641,6 +641,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)\n> \t\t\tstrbuf_addstr(buf, \"no commit associated with this build\\n\");\n> \t\tstrbuf_addf(buf, \"sizeof-long: %d\\n\", (int)sizeof(long));\n> \t\tstrbuf_addf(buf, \"sizeof-size_t: %d\\n\", (int)sizeof(size_t));\n> +\t\tstrbuf_addf(buf, \"shell-path: %s\\n\", SHELL_PATH);\n> \t\t/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */\n> \t}\n> }\n\nThis seems straightforward and logical (as does the rest of the series),\nbut I wondered if it might be a good idea to try to interrogate the\nshell for more information. The reason I mention it is that Debian\npermits any shell that meets certain standards to be /bin/sh, and all\nprograms that invoke /bin/sh must depend on only those features. The\ndefault is dash, but people could use bash, which is more featureful, or\nposh, which is intentionally designed to provide the bare minimum\n/bin/sh experience[0], among others. A value of \"/bin/sh\" doesn't\nnecessarily tell us very much on Debian (or on macOS, for that matter).\n\nNow, that of course does mean that we have to have some way to\ndistinguish between shells, and that is the hard part, so I'm completely\nfine with us leaving it out until we have a good way to do it (or until\nwe decide we need it, which may be never). I just wanted to mention it\nas a potential approach for the future. I'm happy with this series as\nit stands right now.\n\n[0] Quite literally, in that it's supposed to be a tool for testing\ncompatibility with the policy requirements.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"397700","messageId":"xmqqo8qswhpv.fsf@gitster.c.googlers.com","threadId":"53456","inReplyTo":"20200512235924.GC6605@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/2] help: add shell-path to --build-options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T05:02:04Z","receivedAt":"2020-05-13T05:02:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> This seems straightforward and logical (as does the rest of the series),\n> but I wondered if it might be a good idea to try to interrogate the\n> shell for more information. The reason I mention it is that Debian\n> permits any shell that meets certain standards to be /bin/sh, and all\n> programs that invoke /bin/sh must depend on only those features. The\n> default is dash, but people could use bash, which is more featureful, or\n> posh, which is intentionally designed to provide the bare minimum\n> /bin/sh experience[0], among others. A value of \"/bin/sh\" doesn't\n> necessarily tell us very much on Debian (or on macOS, for that matter).\n\nGood point. Perhaps readlink(3) on it, then?\n\nlrwxrwxrwx 1 root root 9 Mar 11 2018 /bin/sh -> /bin/bash\n\n> Now, that of course does mean that we have to have some way to\n> distinguish between shells, and that is the hard part, so I'm completely\n> fine with us leaving it out until we have a good way to do it (or until\n> we decide we need it, which may be never). I just wanted to mention it\n> as a potential approach for the future. I'm happy with this series as\n> it stands right now.\n>\n> [0] Quite literally, in that it's supposed to be a tool for testing\n> compatibility with the policy requirements.\n"},{"id":"397702","messageId":"CAPig+cRGrExRsucs8pmmiP+Zc2CmR3AU=8ACuiyH9tkz9JSCgQ@mail.gmail.com","threadId":"53456","inReplyTo":"xmqqo8qswhpv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/2] help: add shell-path to --build-options","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-05-13T05:10:36Z","receivedAt":"2020-05-13T05:10:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 13, 2020 at 1:02 AM Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> > [...] A value of \"/bin/sh\" doesn't\n> > necessarily tell us very much on Debian (or on macOS, for that matter).\n>\n> Good point. Perhaps readlink(3) on it, then?\n>\n> lrwxrwxrwx 1 root root 9 Mar 11 2018 /bin/sh -> /bin/bash\n\nThat wouldn't help on Mac OS:\n\n $ ls -l /bin/sh /bin/bash\n -r-xr-xr-x 1 root wheel 618448 Mar 18 22:04 /bin/bash\n -r-xr-xr-x 1 root wheel 618512 Mar 18 22:04 /bin/sh\n\nAlthough /bin/sh is neither a hard link nor a symbolic link to\n/bin/bash, nor is the file size the same, it is nevertheless Bash (in\nsome form or another).\n"},{"id":"397733","messageId":"20200513184507.GA196295@google.com","threadId":"53456","inReplyTo":"CAPig+cRGrExRsucs8pmmiP+Zc2CmR3AU=8ACuiyH9tkz9JSCgQ@mail.gmail.com","subject":"Re: [PATCH 1/2] help: add shell-path to --build-options","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-05-13T18:45:07Z","receivedAt":"2020-05-13T18:45:15Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, May 13, 2020 at 01:10:36AM -0400, Eric Sunshine wrote:\n> \n> On Wed, May 13, 2020 at 1:02 AM Junio C Hamano wrote:\n> > \"brian m. carlson\" writes:\n> > > [...] A value of \"/bin/sh\" doesn't\n> > > necessarily tell us very much on Debian (or on macOS, for that matter).\n> >\n> > Good point. Perhaps readlink(3) on it, then?\n> >\n> > lrwxrwxrwx 1 root root 9 Mar 11 2018 /bin/sh -> /bin/bash\n> \n> That wouldn't help on Mac OS:\n> \n> $ ls -l /bin/sh /bin/bash\n> -r-xr-xr-x 1 root wheel 618448 Mar 18 22:04 /bin/bash\n> -r-xr-xr-x 1 root wheel 618512 Mar 18 22:04 /bin/sh\n> \n> Although /bin/sh is neither a hard link nor a symbolic link to\n> /bin/bash, nor is the file size the same, it is nevertheless Bash (in\n> some form or another).\n\nHum. It seems some useful things exist like $BASH_VERSION and\n$ZSH_VERSION, but I'm not terribly excited about the idea of making a\nlist and checking each one in bugreport (as it's sure to be\nnonexhaustive). I found a few more heuristics in a SO post[1] but this\napproach generally looks like a pain, and somewhat unreliable.\n\nMost other alternatives I found with a cursory Google involve checking\nthe name of the shell, which I think will have the same issues as\nchecking $SHELL:\n\n - `ps -p $$` for info about the current process (didn't reveal bash when I `cp\n /bin/bash ~/bin/nish && ~/bin/nish`)\n - `echo $0` since it's coming from the command line, of course will be\n the same as $SHELL\n\nI suppose if we wanted to do the heuristics it'd be a better experience\nfor us, the bugreport receivers, to have the bugreport library collect\nthe heuristics and try to make a guess, rather than just dump all the\nheuristics and let the humans try to remember which shell has $PS4 and\nwhich shell has $version and so on. But to me that sounds prone to rot\nat best.\n\n - Emily\n\n[1]: https://stackoverflow.com/a/3327022\n"}]}