From: David Aguilar Date: Wed, 22 Feb 2012 09:15:17 GMT Subject: Re: [PATCH] contrib: added git-diffall Message-ID: In-Reply-To: <7vbooregj6.fsf@alter.siamese.dyndns.org> On Tue, Feb 21, 2012 at 6:41 PM, Junio C Hamano wrote: > Tim Henigan writes: > >> There is no specific reason it must be bash.  I changed from >> "#!/bin/sh" to "#!/bin/bash -e" due to a bug report from a user on >> Ubuntu [1].  The user reported that: >> >>     "If you use /bin/sh on ubuntu you get the dash shell instead of bash shell. >>     This causes git_merge_tool_path to fail. The error isn't trapped, >> so it exits >>     without displaying anything and without cleaning up." >> >> Given that all the other scripts distributed with git use /bin/sh, I >> will change this script to match. > > You need to dig back to that bug report deeper and find out what exactly > is causing the "failure", before blindly allowing /bin/dash to be used. > > I think the above function name is a typo of get_merge_tool_path that is > borrowed from git-mergetool--lib.sh, but nothing in the function jumps as > a blatant bash-ism at me from a quick reading. > > David, any idea on this? I don't see any bash-isms there myself, either. We should keep this stuff without bash-isms. I haven't had time to read these patches in depth yet but will try to read the re-roll. Can we ask the github user to elaborate on what exactly was erroring out? Does dash not handle || inside $()? We can only make wild guesses without their help. The only hint from the pull request is "silent exit with no results". Do we do that? There are a few code paths where we do "exit 1" but that's only under error conditions. We haven't had any reports about git-mergetool/difftool, which use these functions... are we certain the problem was not some other bash-isms in the script? -- David