Re: [PATCH] git-submodule.sh: allow running in validate mode without previous update

[Nina Schoetterl-Glausch]

On Sun, 2023-06-18 at 23:20 +0200, Paolo Bonzini wrote:
> The call to git-submodule.sh done in configure may happen without a
> previous checkout of the roms/SLOF submodule, or even without a
> previous run of the script.
> 
> So, handle creating a .git-submodule-status file even in validate
> mode.  If git is absent, ensure that all passed directories exists
> (because you should be in a fresh untar and will not have stale
> arguments to git-submodule.sh) but do no other checks.  If git
> is present, ensure that .git-submodule-status contains an entry
> for all submodules passed on the command line.
> 
> With this change, "ignore" mode is not needed anymore.
> 
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Fixes: b11f9bd96f4 ("configure: move SLOF submodule handling to 
> pc-bios/s390-ccw", 2023-06-06)
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure                |  2 +-
>  scripts/git-submodule.sh | 72 ++++++++++++++++++++++------------------
>  2 files changed, 41 insertions(+), 33 deletions(-)
> 
> diff --git a/configure b/configure
> index 86363a7e508..2b41c49c0d1 100755
> --- a/configure
> +++ b/configure
> @@ -758,7 +758,7 @@ done
>  
>  if ! test -e "$source_path/.git"
>  then
> -    git_submodules_action="ignore"
> +    git_submodules_action="validate"
>  fi
>  
>  # test for any invalid configuration combinations
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> index 11fad2137cd..c33d8fe4cac 100755
> --- a/scripts/git-submodule.sh
> +++ b/scripts/git-submodule.sh
> @@ -9,13 +9,22 @@ command=$1
>  shift
>  maybe_modules="$@"
>  
> -# if not running in a git checkout, do nothing
> -test "$command" = "ignore" && exit 0
> -
> +test -z "$maybe_modules" && exit 0
>  test -z "$GIT" && GIT=$(command -v git)
>  
>  cd "$(dirname "$0")/.."
>  
> +no_git_error=
> +if test -n "$maybe_modules" && ! test -e ".git"; then
> +    no_git_error='no git checkout exists'
> +elif test -n "$maybe_modules" && test -z "$GIT"; then
> +    no_git_error='git binary not found'
> +fi

No need to test -n "$maybe_modules" if you exit early above.

> +
> +is_git() {
> +    test -z "$no_git_error"
> +}
> +
>  update_error() {
>      echo "$0: $*"
>      echo
> @@ -34,7 +43,7 @@ update_error() {
>  }
>  
>  validate_error() {
> -    if test "$1" = "validate"; then
> +    if is_git && test "$1" = "validate"; then
>          echo "GIT submodules checkout is out of date, and submodules"
>          echo "configured for validate only. Please run"
>          echo "  scripts/git-submodule.sh update $maybe_modules"
> @@ -51,42 +60,41 @@ check_updated() {
>      test "$CURSTATUS" = "$OLDSTATUS"
>  }
>  
> -if test -n "$maybe_modules" && ! test -e ".git"
> -then
> -    echo "$0: unexpectedly called with submodules but no git checkout exists"
> -    exit 1
> +if is_git; then
> +    test -e $substat || touch $substat
> +    modules=""
> +    for m in $maybe_modules
> +    do
> +        $GIT submodule status $m 1> /dev/null 2>&1
> +        if test $? = 0
> +        then
> +            modules="$modules $m"
> +            grep $m $substat > /dev/null 2>&1 || $GIT submodule status 
> $module >> $substat
> +        else
> +            echo "warn: ignoring non-existent submodule $m"

What is the rational for ignoring non-existing submodules, i.e. how do the 
arguments to
the script go stale as you say in the patch description?
I'm asking because the fedora spec file initializes a new git repo in order to 
apply
patches so the script exits with 0.
Nothing that cannot be worked around ofc.

> +        fi
> +    done
> +else
> +    modules=$maybe_modules
>  fi
>  
> -if test -n "$maybe_modules" && test -z "$GIT"
> -then
> -    echo "$0: unexpectedly called with submodules but git binary not found"
> -    exit 1
> -fi
> -
> -modules=""
> -for m in $maybe_modules
> -do
> -    $GIT submodule status $m 1> /dev/null 2>&1
> -    if test $? = 0
> -    then
> -        modules="$modules $m"
> -    else
> -        echo "warn: ignoring non-existent submodule $m"
> -    fi
> -done
> -
>  case "$command" in
>  status|validate)
> -    test -f "$substat" || validate_error "$command"
> -    test -z "$maybe_modules" && exit 0
>      for module in $modules; do
> -        check_updated $module || validate_error "$command"
> +        if is_git; then
> +            check_updated $module || validate_error "$command"
> +        elif ! test -d $module; then

archive-source.sh creates an empty directory for e.g. roms/SLOF,
so this check succeeds even if the submodule sources are unavailable.
Something like

        elif ! test -d $module || test -z "$(ls -A "$module")"; then

works.

> +            echo "$0: sources not available for $module and $no_git_error"
> +            validate_error "$command"
> +        fi
>      done
> -    exit 0
>      ;;
> +
>  update)
> -    test -e $substat || touch $substat
> -    test -z "$maybe_modules" && exit 0
> +    is_git || {
> +        echo "$0: unexpectedly called with submodules but $no_git_error"
> +        exit 1
> +    }
>  
>      $GIT submodule update --init $modules 1>/dev/null
>      test $? -ne 0 && update_error "failed to update modules"