Re: Cannot run `git submodule init` on Cygwin from script with strict error checking

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sep 11, 2020, 13:19 UTC
Message-ID: <20200911131943.GA23146@szeder.dev>
In-Reply-To: <CABJqhQMST-n6-uEDpFCUSsYueF_=7ZLPCtj_mQnu679oY99ZVg@mail.gmail.com>

On Fri, Sep 11, 2020 at 01:46:43PM +0200, Jędrzej Dudkiewicz wrote:

Show 25 quoted lines

> On Fri, Sep 11, 2020 at 1:30 PM Adam Dinwoodie <adam@dinwoodie.org> wrote:
> >
> > Hi Jędrzej,
> 
> > I think there's something odd about the way you're calling `git
> > submodule init`: it should normally be a separate execution that
> > wouldn't inherit the `-aeu` or `-x` settings from the parent Bash
> > process.
> 
> Sorry for not including the test script. Here it is:
> 
> ----8<----8<----8<-- CUT HERE--8<----8<----8<----8<----
> #!/bin/bash
> 
> set -aeu
> 
> export SHELLOPTS
> 
> set -x
> 
> git submodule init
> ----8<----8<----8<-- CUT HERE--8<----8<----8<----8<----
> 
> I use "export SHELLOPTS" because I want these flags to be effective in
> subshells.

You don't need 'export SHELLOPTS' to make those flags effective in _subshells_:

  $ cat test.sh 
  #!/bin/sh
  
  set -ex
  
  (
          false
          echo "after false"
  )
  exit 0
  $ ./test.sh
  + false
  1
  $ echo $?
  1

What 'export SHELLOPTS' does is make those flags effective even in separate shell scripts executed by your script, but, as you just found out, that's not really a good idea, because those scripts are beyond your control.

> As a workaround I'm currently calling "set +u" before each
> execution of "git submodule init" and my script works, but it isn't
> very nice and IMHO shouldn't be required (i.e. it would be extremely
> nice if someone fixed it).

The right workaround would be to apply those shell options only to your script, i.e. to remove that 'export SHELLOPTS'.

Having said that, unlike 'git submodule', 'git-sh-setup' is meant to be dot-sourced into users' shell scripts, and, therefore, should work with the shell options set in users' scripts, including even 'set -u'.

The patch below may or may not make it work; it's well over two years old, and I haven't tested it at all since then.

  ---  >8  ---

Subject: [PATCH] git-sh-setup, git-sh-i18n: make our shell libraries work with
 'set -u'

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 git-sh-i18n.sh  |  6 +++---
 git-sh-setup.sh | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 8eef60b43f..4eb2a7ad70 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -7,7 +7,7 @@
 # Export the TEXTDOMAIN* data that we need for Git
 TEXTDOMAIN=git
 export TEXTDOMAIN
-if test -z "$GIT_TEXTDOMAINDIR"
+if test -z "${GIT_TEXTDOMAINDIR-}"
 then
 	TEXTDOMAINDIR="@@LOCALEDIR@@"
 else
@@ -17,7 +17,7 @@ export TEXTDOMAINDIR
 
 # First decide what scheme to use...
 GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "$GIT_TEST_GETTEXT_POISON" &&
+if test -n "${GIT_TEST_GETTEXT_POISON-}" &&
 	    git env--helper --type=bool --default=0 --exit-code \
 		GIT_TEST_GETTEXT_POISON
 then
@@ -25,7 +25,7 @@ then
 elif test -n "@@USE_GETTEXT_SCHEME@@"
 then
 	GIT_INTERNAL_GETTEXT_SH_SCHEME="@@USE_GETTEXT_SCHEME@@"
-elif test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+elif test -n "${GIT_INTERNAL_GETTEXT_TEST_FALLBACKS-}"
 then
 	: no probing necessary
 elif type gettext.sh >/dev/null 2>&1
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 10d9764185..1cb1c8a2e8 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -66,16 +66,16 @@ say () {
 	fi
 }
 
-if test -n "$OPTIONS_SPEC"; then
+if test -n "${OPTIONS_SPEC-}"; then
 	usage() {
 		"$0" -h
 		exit 1
 	}
 
 	parseopt_extra=
-	[ -n "$OPTIONS_KEEPDASHDASH" ] &&
+	[ -n "${OPTIONS_KEEPDASHDASH-}" ] &&
 		parseopt_extra="--keep-dashdash"
-	[ -n "$OPTIONS_STUCKLONG" ] &&
+	[ -n "${OPTIONS_STUCKLONG-}" ] &&
 		parseopt_extra="$parseopt_extra --stuck-long"
 
 	eval "$(
@@ -89,7 +89,7 @@ else
 		die "$(eval_gettext "usage: \$dashless \$USAGE")"
 	}
 
-	if [ -z "$LONG_USAGE" ]
+	if [ -z "${LONG_USAGE-}" ]
 	then
 		LONG_USAGE="$(eval_gettext "usage: \$dashless \$USAGE")"
 	else
@@ -98,7 +98,7 @@ else
 $LONG_USAGE")"
 	fi
 
-	case "$1" in
+	case "${1-}" in
 		-h)
 		echo "$LONG_USAGE"
 		case "$0" in *git-legacy-stash) exit 129;; esac
@@ -366,7 +366,7 @@ esac
 # if we require to be in a git repository.
 git_dir_init () {
 	GIT_DIR=$(git rev-parse --git-dir) || exit
-	if [ -z "$SUBDIRECTORY_OK" ]
+	if [ -z "${SUBDIRECTORY_OK-}" ]
 	then
 		test -z "$(git rev-parse --show-cdup)" || {
 			exit=$?
@@ -381,7 +381,7 @@ git_dir_init () {
 	: "${GIT_OBJECT_DIRECTORY="$(git rev-parse --git-path objects)"}"
 }
 
-if test -z "$NONGIT_OK"
+if test -z "${NONGIT_OK-}"
 then
 	git_dir_init
 fi

-- 
2.28.0.820.g0db9d372c0

  ---  >8  ---