On Tue, Feb 21, 2012 at 6:41 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Tim Henigan <tim.henigan@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html