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

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

[Jędrzej Dudkiewicz]

I'm running git 2.28.0 on Cygwin. I have a script with:

set -aeu

at the beginning (as all scripts should have). In the script I'm trying to run:

git submodule init

When running via "bash -x script.sh", I get:

+ git submodule init
++ basename /usr/libexec/git-core/git-submodule
++ sed -e 's/-/ /'
+ dashless='git submodule'
+ USAGE='[--quiet] [--cached]
   or: git submodule [--quiet] add [-b <branch>] [-f|--force] [--name
<name>] [--reference <repository>] [--] <repository> [<path>]
   or: git submodule [--quiet] status [--cached] [--recursive] [--] [<path>...]
   or: git submodule [--quiet] init [--] [<path>...]
   or: git submodule [--quiet] deinit [-f|--force] (--all| [--] <path>...)
   or: git submodule [--quiet] update [--init] [--remote]
[-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase]
[--[no-]recommend-shallow] [--reference <repository>] [--recursive]
[--[no-]single-branch] [--] [<path>...]
   or: git submodule [--quiet] set-branch (--default|--branch
<branch>) [--] <path>
   or: git submodule [--quiet] set-url [--] <path> <newurl>
   or: git submodule [--quiet] summary [--cached|--files]
[--summary-limit <n>] [commit] [--] [<path>...]
   or: git submodule [--quiet] foreach [--recursive] <command>
   or: git submodule [--quiet] sync [--recursive] [--] [<path>...]
   or: git submodule [--quiet] absorbgitdirs [--] [<path>...]'
+ OPTIONS_SPEC=
+ SUBDIRECTORY_OK=Yes
+ . git-sh-setup
++ unset CDPATH
++ IFS='
'
+++ git --exec-path
++ . /usr/libexec/git-core/git-sh-i18n
+++ TEXTDOMAIN=git
+++ export TEXTDOMAIN
/usr/libexec/git-core/git-sh-i18n: line 10: GIT_TEXTDOMAINDIR: unbound variable

(TL;DR: This basically boils down to GIT_TEXTDOMAINDIR being unbound,
i.e. uninitialized/non-existant.)

When I exported GIT_TEXTDOMAINDIR with bogus value from my script
before calling "git submodule init":

export GIT_TEXTDOMAINDIR=/bogus/value

I got (only error shown):

/usr/libexec/git-core/git-sh-i18n: line 20: GIT_TEST_GETTEXT_POISON:
unbound variable

I set it to `false`, restarted and then got:

/usr/libexec/git-core/git-sh-i18n: line 28:
GIT_INTERNAL_GETTEXT_TEST_FALLBACKS: unbound variable

I set it to empty value, restarted and then got:

/usr/libexec/git-core/git-sh-setup: line 91: LONG_USAGE: unbound variable

This is when I gave up.

Same script run with git 2.21.0 on Linux has no problems, but I
haven't compare differences between two versions, so I don't know why
it stopped working.
-- 
Jędrzej Dudkiewicz

I really hate this damn machine, I wish that they would sell it.
It never does just what I want, but only what I tell it.

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

[Adam Dinwoodie]

Hi Jędrzej,

On Fri, 11 Sep 2020 at 09:20, Jędrzej Dudkiewicz wrote:
>
> I'm running git 2.28.0 on Cygwin. I have a script with:
>
> set -aeu
>
> at the beginning (as all scripts should have). In the script I'm trying to run:
>
> git submodule init
>
> When running via "bash -x script.sh", I get:
>
> + git submodule init
> ++ basename /usr/libexec/git-core/git-submodule
> ++ sed -e 's/-/ /'
> + dashless='git submodule'
> + USAGE='[--quiet] [--cached]
>    or: git submodule [--quiet] add [-b <branch>] [-f|--force] [--name
> <name>] [--reference <repository>] [--] <repository> [<path>]
>    or: git submodule [--quiet] status [--cached] [--recursive] [--] [<path>...]
>    or: git submodule [--quiet] init [--] [<path>...]
>    or: git submodule [--quiet] deinit [-f|--force] (--all| [--] <path>...)
>    or: git submodule [--quiet] update [--init] [--remote]
> [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase]
> [--[no-]recommend-shallow] [--reference <repository>] [--recursive]
> [--[no-]single-branch] [--] [<path>...]
>    or: git submodule [--quiet] set-branch (--default|--branch
> <branch>) [--] <path>
>    or: git submodule [--quiet] set-url [--] <path> <newurl>
>    or: git submodule [--quiet] summary [--cached|--files]
> [--summary-limit <n>] [commit] [--] [<path>...]
>    or: git submodule [--quiet] foreach [--recursive] <command>
>    or: git submodule [--quiet] sync [--recursive] [--] [<path>...]
>    or: git submodule [--quiet] absorbgitdirs [--] [<path>...]'
> + OPTIONS_SPEC=
> + SUBDIRECTORY_OK=Yes
> + . git-sh-setup
> ++ unset CDPATH
> ++ IFS='
> '
> +++ git --exec-path
> ++ . /usr/libexec/git-core/git-sh-i18n
> +++ TEXTDOMAIN=git
> +++ export TEXTDOMAIN
> /usr/libexec/git-core/git-sh-i18n: line 10: GIT_TEXTDOMAINDIR: unbound variable
> <snip>

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. It looks like perhaps you're `source`ing or dotting in the
`git-submodule` script? Whatever it is, I think the issue is down to
the way you're using `git submodule`, rather than a problem with Git
or the Cygwin Git build.

I  did just try to reproduce the problem using Git v2.28.0-1 on
Cygwin, and wasn't able to:

```
$ cat test.sh
set -aeu
rm -rf testdir
mkdir testdir
cd testdir
git init
git submodule init

$ bash -x test.sh
+ set -aeu
+ rm -rf testdir
+ mkdir testdir
+ cd testdir
+ git init
Initialized empty Git repository in /home/adam/testdir/.git/
+ git submodule init

$ echo $?
0
```

HTH

Adam

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

[Jędrzej Dudkiewicz]

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. 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).
-- 
Jędrzej Dudkiewicz

I really hate this damn machine, I wish that they would sell it.
It never does just what I want, but only what I tell it.

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

[SZEDER Gábor]

On Fri, Sep 11, 2020 at 01:46:43PM +0200, Jędrzej Dudkiewicz wrote:
> 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  ---

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

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

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

Is it and should it?

git-sh-setup was meant to be an implementation detail for our own
scripts and we know don't use "-u -e".  We never cared about
backward compatibility for such use by end-users when we made any
update to the git-sh-setup scriptlet.  We freely changed existing
features and squatted on good names for variables and functions we
used in it, because it is designed as a private helper library.

Having said that, we do protect from end-user misconfiguration like
exporting CDPATH, and protecting ourselves from exporting SHELLOPTS
is not something I would oppose.

Thanks.

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

[SZEDER Gábor]

On Fri, Sep 11, 2020 at 12:07:51PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > 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'.
> 
> Is it and should it?
> 
> git-sh-setup was meant to be an implementation detail for our own
> scripts and we know don't use "-u -e".  We never cared about
> backward compatibility for such use by end-users when we made any
> update to the git-sh-setup scriptlet.  We freely changed existing
> features and squatted on good names for variables and functions we
> used in it, because it is designed as a private helper library.

It has a manpage that it installed by 'make install-doc', and 'man
git' advertises it, so I use it in most of my git-foo shell scripts,
e.g. for 'require_clean_work_tree'.

> Having said that, we do protect from end-user misconfiguration like
> exporting CDPATH, and protecting ourselves from exporting SHELLOPTS
> is not something I would oppose.
> 
> Thanks.