We short circuit the git submodule update when passed an empty module list.
This accidentally causes the 'status' command to write to the status file. The
test needs to be delayed into the individual commands to avoid this premature
writing of the status file.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index 2857fc57c4..90376a7dae 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -33,12 +33,6 @@ error() {
     exit 1
 }
 
-if test -z "$maybe_modules"
-then
-    test -e $substat || touch $substat
-    exit 0
-fi
-
 modules=""
 for m in $maybe_modules
 do
@@ -51,7 +45,7 @@ do
     fi
 done
 
-if ! test -e ".git"
+if test -n "$maybe_modules" && ! test -e ".git"
 then
     echo "$0: unexpectedly called with submodules but no git checkout exists"
     exit 1
@@ -59,6 +53,11 @@ fi
 
 case "$command" in
 status)
+    if test -z "$maybe_modules"
+    then
+         test -s ${substat} && exit 1 || exit 0
+    fi
+
     test -f "$substat" || exit 1
     CURSTATUS=`$GIT submodule status $modules`
     OLDSTATUS=`cat $substat`
@@ -66,6 +65,12 @@ status)
     exit $?
     ;;
 update)
+    if test -z "$maybe_modules"
+    then
+        test -e $substat || touch $substat
+        exit 0
+    fi
+
     $GIT submodule update --init $modules 1>/dev/null
     test $? -ne 0 && error "failed to update modules"
 
-- 
2.13.6

On 11/03/2017 10:33 AM, Daniel P. Berrange wrote:
> We short circuit the git submodule update when passed an empty module list.
> This accidentally causes the 'status' command to write to the status file. The
> test needs to be delayed into the individual commands to avoid this premature
> writing of the status file.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  scripts/git-submodule.sh | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 04/11/17 02:33, Daniel P. Berrange wrote:
> We short circuit the git submodule update when passed an empty module list.
> This accidentally causes the 'status' command to write to the status file. The
> test needs to be delayed into the individual commands to avoid this premature
> writing of the status file.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  scripts/git-submodule.sh | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> index 2857fc57c4..90376a7dae 100755
> --- a/scripts/git-submodule.sh
> +++ b/scripts/git-submodule.sh
> @@ -33,12 +33,6 @@ error() {
>      exit 1
>  }
>  
> -if test -z "$maybe_modules"
> -then
> -    test -e $substat || touch $substat
> -    exit 0
> -fi
> -
>  modules=""
>  for m in $maybe_modules
>  do
> @@ -51,7 +45,7 @@ do
>      fi
>  done
>  
> -if ! test -e ".git"
> +if test -n "$maybe_modules" && ! test -e ".git"


$GIT_DIR should be checked too imho:

if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""




>  then
>      echo "$0: unexpectedly called with submodules but no git checkout exists"
>      exit 1
> @@ -59,6 +53,11 @@ fi
>  
>  case "$command" in
>  status)
> +    if test -z "$maybe_modules"
> +    then
> +         test -s ${substat} && exit 1 || exit 0
> +    fi
> +
>      test -f "$substat" || exit 1
>      CURSTATUS=`$GIT submodule status $modules`
>      OLDSTATUS=`cat $substat`
> @@ -66,6 +65,12 @@ status)
>      exit $?
>      ;;
>  update)
> +    if test -z "$maybe_modules"
> +    then
> +        test -e $substat || touch $substat
> +        exit 0
> +    fi
> +
>      $GIT submodule update --init $modules 1>/dev/null
>      test $? -ne 0 && error "failed to update modules"
>  
> 


-- 
Alexey

On 07/11/17 15:12, Alexey Kardashevskiy wrote:
> On 04/11/17 02:33, Daniel P. Berrange wrote:
>> We short circuit the git submodule update when passed an empty module list.
>> This accidentally causes the 'status' command to write to the status file. The
>> test needs to be delayed into the individual commands to avoid this premature
>> writing of the status file.
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  scripts/git-submodule.sh | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
>> index 2857fc57c4..90376a7dae 100755
>> --- a/scripts/git-submodule.sh
>> +++ b/scripts/git-submodule.sh
>> @@ -33,12 +33,6 @@ error() {
>>      exit 1
>>  }
>>  
>> -if test -z "$maybe_modules"
>> -then
>> -    test -e $substat || touch $substat
>> -    exit 0
>> -fi
>> -
>>  modules=""
>>  for m in $maybe_modules
>>  do
>> @@ -51,7 +45,7 @@ do
>>      fi
>>  done
>>  
>> -if ! test -e ".git"
>> +if test -n "$maybe_modules" && ! test -e ".git"
> 
> 
> $GIT_DIR should be checked too imho:
> 
> if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""

Uff. Just this won't help, it is assumed everywhere that the ".git" folder
exists :(

I was hoping to use "git --work-dir=" for the source tree to have a full
complete source tree to compile on a build machine but
"./scripts/git-submodule.sh status" is always called, no matter what.

With this new and improved git-submodule.sh I have to have a separate
source folder per a building machine as different machines may or may not
have a required "dtc" installed so the correct "git submodule status" will
differ from machine to machine which is quite annoying and not really
necessary in everyday life.

As for now, "configure" tries to use the system libfdt and only if it is
not found, goes for git submodule.

I'd really love to have some way to disable git activities in Makefile...

-- 
Alexey

On Tue, Nov 07, 2017 at 04:13:27PM +1100, Alexey Kardashevskiy wrote:
> On 07/11/17 15:12, Alexey Kardashevskiy wrote:
> > On 04/11/17 02:33, Daniel P. Berrange wrote:
> >> We short circuit the git submodule update when passed an empty module list.
> >> This accidentally causes the 'status' command to write to the status file. The
> >> test needs to be delayed into the individual commands to avoid this premature
> >> writing of the status file.
> >>
> >> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >> ---
> >>  scripts/git-submodule.sh | 19 ++++++++++++-------
> >>  1 file changed, 12 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> >> index 2857fc57c4..90376a7dae 100755
> >> --- a/scripts/git-submodule.sh
> >> +++ b/scripts/git-submodule.sh
> >> @@ -33,12 +33,6 @@ error() {
> >>      exit 1
> >>  }
> >>  
> >> -if test -z "$maybe_modules"
> >> -then
> >> -    test -e $substat || touch $substat
> >> -    exit 0
> >> -fi
> >> -
> >>  modules=""
> >>  for m in $maybe_modules
> >>  do
> >> @@ -51,7 +45,7 @@ do
> >>      fi
> >>  done
> >>  
> >> -if ! test -e ".git"
> >> +if test -n "$maybe_modules" && ! test -e ".git"
> > 
> > 
> > $GIT_DIR should be checked too imho:
> > 
> > if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""
> 
> Uff. Just this won't help, it is assumed everywhere that the ".git" folder
> exists :(
> 
> I was hoping to use "git --work-dir=" for the source tree to have a full
> complete source tree to compile on a build machine but
> "./scripts/git-submodule.sh status" is always called, no matter what.

If you need to pass extra args to 'git' this is supported already by
this changeset. Just create a wrapper script containing whatever args
you need to give git, and use "./configure --with-git=your-wrapper"

> I'd really love to have some way to disable git activities in Makefile...

I don't want to add that. We don't want to ever build if we detect that
the submodules are at a different version to what we expect as it would
cause obscure hard to diagnose build problems

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On 07/11/17 21:12, Daniel P. Berrange wrote:
> On Tue, Nov 07, 2017 at 04:13:27PM +1100, Alexey Kardashevskiy wrote:
>> On 07/11/17 15:12, Alexey Kardashevskiy wrote:
>>> On 04/11/17 02:33, Daniel P. Berrange wrote:
>>>> We short circuit the git submodule update when passed an empty module list.
>>>> This accidentally causes the 'status' command to write to the status file. The
>>>> test needs to be delayed into the individual commands to avoid this premature
>>>> writing of the status file.
>>>>
>>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>>> ---
>>>>  scripts/git-submodule.sh | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
>>>> index 2857fc57c4..90376a7dae 100755
>>>> --- a/scripts/git-submodule.sh
>>>> +++ b/scripts/git-submodule.sh
>>>> @@ -33,12 +33,6 @@ error() {
>>>>      exit 1
>>>>  }
>>>>  
>>>> -if test -z "$maybe_modules"
>>>> -then
>>>> -    test -e $substat || touch $substat
>>>> -    exit 0
>>>> -fi
>>>> -
>>>>  modules=""
>>>>  for m in $maybe_modules
>>>>  do
>>>> @@ -51,7 +45,7 @@ do
>>>>      fi
>>>>  done
>>>>  
>>>> -if ! test -e ".git"
>>>> +if test -n "$maybe_modules" && ! test -e ".git"
>>>
>>>
>>> $GIT_DIR should be checked too imho:
>>>
>>> if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""
>>
>> Uff. Just this won't help, it is assumed everywhere that the ".git" folder
>> exists :(
>>
>> I was hoping to use "git --work-dir=" for the source tree to have a full
>> complete source tree to compile on a build machine but
>> "./scripts/git-submodule.sh status" is always called, no matter what.
> 
> If you need to pass extra args to 'git' this is supported already by
> this changeset. Just create a wrapper script containing whatever args
> you need to give git, and use "./configure --with-git=your-wrapper"
> 
>> I'd really love to have some way to disable git activities in Makefile...
> 
> I don't want to add that. We don't want to ever build if we detect that
> the submodules are at a different version to what we expect as it would
> cause obscure hard to diagnose build problems
> 

I gave you an example with one build machine having dtc and another not
having it - your system breaks. What is the proper solution to this?



-- 
Alexey

On Wed, Nov 08, 2017 at 09:58:11AM +1100, Alexey Kardashevskiy wrote:
> On 07/11/17 21:12, Daniel P. Berrange wrote:
> > On Tue, Nov 07, 2017 at 04:13:27PM +1100, Alexey Kardashevskiy wrote:
> >> On 07/11/17 15:12, Alexey Kardashevskiy wrote:
> >>> On 04/11/17 02:33, Daniel P. Berrange wrote:
> >>>> We short circuit the git submodule update when passed an empty module list.
> >>>> This accidentally causes the 'status' command to write to the status file. The
> >>>> test needs to be delayed into the individual commands to avoid this premature
> >>>> writing of the status file.
> >>>>
> >>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >>>> ---
> >>>>  scripts/git-submodule.sh | 19 ++++++++++++-------
> >>>>  1 file changed, 12 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> >>>> index 2857fc57c4..90376a7dae 100755
> >>>> --- a/scripts/git-submodule.sh
> >>>> +++ b/scripts/git-submodule.sh
> >>>> @@ -33,12 +33,6 @@ error() {
> >>>>      exit 1
> >>>>  }
> >>>>  
> >>>> -if test -z "$maybe_modules"
> >>>> -then
> >>>> -    test -e $substat || touch $substat
> >>>> -    exit 0
> >>>> -fi
> >>>> -
> >>>>  modules=""
> >>>>  for m in $maybe_modules
> >>>>  do
> >>>> @@ -51,7 +45,7 @@ do
> >>>>      fi
> >>>>  done
> >>>>  
> >>>> -if ! test -e ".git"
> >>>> +if test -n "$maybe_modules" && ! test -e ".git"
> >>>
> >>>
> >>> $GIT_DIR should be checked too imho:
> >>>
> >>> if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""
> >>
> >> Uff. Just this won't help, it is assumed everywhere that the ".git" folder
> >> exists :(
> >>
> >> I was hoping to use "git --work-dir=" for the source tree to have a full
> >> complete source tree to compile on a build machine but
> >> "./scripts/git-submodule.sh status" is always called, no matter what.
> > 
> > If you need to pass extra args to 'git' this is supported already by
> > this changeset. Just create a wrapper script containing whatever args
> > you need to give git, and use "./configure --with-git=your-wrapper"
> > 
> >> I'd really love to have some way to disable git activities in Makefile...
> > 
> > I don't want to add that. We don't want to ever build if we detect that
> > the submodules are at a different version to what we expect as it would
> > cause obscure hard to diagnose build problems
> > 
> 
> I gave you an example with one build machine having dtc and another not
> having it - your system breaks. What is the proper solution to this?

You could create separate source directories for each system you're building
on. This won't take up extra disk space if you clone extra source dirs from
your first checkout using --reference (or even --shared - but see caveats
in git manpage for that)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On 07/11/17 21:12, Daniel P. Berrange wrote:
> On Tue, Nov 07, 2017 at 04:13:27PM +1100, Alexey Kardashevskiy wrote:
>> On 07/11/17 15:12, Alexey Kardashevskiy wrote:
>>> On 04/11/17 02:33, Daniel P. Berrange wrote:
>>>> We short circuit the git submodule update when passed an empty module list.
>>>> This accidentally causes the 'status' command to write to the status file. The
>>>> test needs to be delayed into the individual commands to avoid this premature
>>>> writing of the status file.
>>>>
>>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>>> ---
>>>>  scripts/git-submodule.sh | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
>>>> index 2857fc57c4..90376a7dae 100755
>>>> --- a/scripts/git-submodule.sh
>>>> +++ b/scripts/git-submodule.sh
>>>> @@ -33,12 +33,6 @@ error() {
>>>>      exit 1
>>>>  }
>>>>  
>>>> -if test -z "$maybe_modules"
>>>> -then
>>>> -    test -e $substat || touch $substat
>>>> -    exit 0
>>>> -fi
>>>> -
>>>>  modules=""
>>>>  for m in $maybe_modules
>>>>  do
>>>> @@ -51,7 +45,7 @@ do
>>>>      fi
>>>>  done
>>>>  
>>>> -if ! test -e ".git"
>>>> +if test -n "$maybe_modules" && ! test -e ".git"
>>>
>>>
>>> $GIT_DIR should be checked too imho:
>>>
>>> if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""
>>
>> Uff. Just this won't help, it is assumed everywhere that the ".git" folder
>> exists :(
>>
>> I was hoping to use "git --work-dir=" for the source tree to have a full
>> complete source tree to compile on a build machine but
>> "./scripts/git-submodule.sh status" is always called, no matter what.
> 
> If you need to pass extra args to 'git' this is supported already by


Missed that. No, I do not need extra args to git, I need configure and
others to stop assuming that the .git folder always exists instead of
calling $(GIT) to know that. "--work-dir" and other things can be
configured via git means or/and environment variables.



> this changeset. Just create a wrapper script containing whatever args
> you need to give git, and use "./configure --with-git=your-wrapper"
> 
>> I'd really love to have some way to disable git activities in Makefile...
> 
> I don't want to add that. We don't want to ever build if we detect that
> the submodules are at a different version to what we expect as it would
> cause obscure hard to diagnose build problems




-- 
Alexey

On 07/11/17 21:12, Daniel P. Berrange wrote:
> On Tue, Nov 07, 2017 at 04:13:27PM +1100, Alexey Kardashevskiy wrote:
>> On 07/11/17 15:12, Alexey Kardashevskiy wrote:
>>> On 04/11/17 02:33, Daniel P. Berrange wrote:
>>>> We short circuit the git submodule update when passed an empty module list.
>>>> This accidentally causes the 'status' command to write to the status file. The
>>>> test needs to be delayed into the individual commands to avoid this premature
>>>> writing of the status file.
>>>>
>>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>>> ---
>>>>  scripts/git-submodule.sh | 19 ++++++++++++-------
>>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
>>>> index 2857fc57c4..90376a7dae 100755
>>>> --- a/scripts/git-submodule.sh
>>>> +++ b/scripts/git-submodule.sh
>>>> @@ -33,12 +33,6 @@ error() {
>>>>      exit 1
>>>>  }
>>>>  
>>>> -if test -z "$maybe_modules"
>>>> -then
>>>> -    test -e $substat || touch $substat
>>>> -    exit 0
>>>> -fi
>>>> -
>>>>  modules=""
>>>>  for m in $maybe_modules
>>>>  do
>>>> @@ -51,7 +45,7 @@ do
>>>>      fi
>>>>  done
>>>>  
>>>> -if ! test -e ".git"
>>>> +if test -n "$maybe_modules" && ! test -e ".git"
>>>
>>>
>>> $GIT_DIR should be checked too imho:
>>>
>>> if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""
>>
>> Uff. Just this won't help, it is assumed everywhere that the ".git" folder
>> exists :(
>>
>> I was hoping to use "git --work-dir=" for the source tree to have a full
>> complete source tree to compile on a build machine but
>> "./scripts/git-submodule.sh status" is always called, no matter what.
> 
> If you need to pass extra args to 'git' this is supported already by
> this changeset. Just create a wrapper script containing whatever args
> you need to give git, and use "./configure --with-git=your-wrapper"
> 
>> I'd really love to have some way to disable git activities in Makefile...
> 
> I don't want to add that. We don't want to ever build if we detect that
> the submodules are at a different version to what we expect as it would
> cause obscure hard to diagnose build problems


btw how is it expected to build from source tarball, without any .git
directories present, for example with rpmbuild?



-- 
Alexey

On Wed, Nov 08, 2017 at 07:28:31PM +1100, Alexey Kardashevskiy wrote:
> On 07/11/17 21:12, Daniel P. Berrange wrote:
> > On Tue, Nov 07, 2017 at 04:13:27PM +1100, Alexey Kardashevskiy wrote:
> >> On 07/11/17 15:12, Alexey Kardashevskiy wrote:
> >>> On 04/11/17 02:33, Daniel P. Berrange wrote:
> >>>> We short circuit the git submodule update when passed an empty module list.
> >>>> This accidentally causes the 'status' command to write to the status file. The
> >>>> test needs to be delayed into the individual commands to avoid this premature
> >>>> writing of the status file.
> >>>>
> >>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >>>> ---
> >>>>  scripts/git-submodule.sh | 19 ++++++++++++-------
> >>>>  1 file changed, 12 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> >>>> index 2857fc57c4..90376a7dae 100755
> >>>> --- a/scripts/git-submodule.sh
> >>>> +++ b/scripts/git-submodule.sh
> >>>> @@ -33,12 +33,6 @@ error() {
> >>>>      exit 1
> >>>>  }
> >>>>  
> >>>> -if test -z "$maybe_modules"
> >>>> -then
> >>>> -    test -e $substat || touch $substat
> >>>> -    exit 0
> >>>> -fi
> >>>> -
> >>>>  modules=""
> >>>>  for m in $maybe_modules
> >>>>  do
> >>>> @@ -51,7 +45,7 @@ do
> >>>>      fi
> >>>>  done
> >>>>  
> >>>> -if ! test -e ".git"
> >>>> +if test -n "$maybe_modules" && ! test -e ".git"
> >>>
> >>>
> >>> $GIT_DIR should be checked too imho:
> >>>
> >>> if test -n "$maybe_modules" && ! test -e ".git" && test "$GIT_DIR" = ""
> >>
> >> Uff. Just this won't help, it is assumed everywhere that the ".git" folder
> >> exists :(
> >>
> >> I was hoping to use "git --work-dir=" for the source tree to have a full
> >> complete source tree to compile on a build machine but
> >> "./scripts/git-submodule.sh status" is always called, no matter what.
> > 
> > If you need to pass extra args to 'git' this is supported already by
> > this changeset. Just create a wrapper script containing whatever args
> > you need to give git, and use "./configure --with-git=your-wrapper"
> > 
> >> I'd really love to have some way to disable git activities in Makefile...
> > 
> > I don't want to add that. We don't want to ever build if we detect that
> > the submodules are at a different version to what we expect as it would
> > cause obscure hard to diagnose build problems
> 
> 
> btw how is it expected to build from source tarball, without any .git
> directories present, for example with rpmbuild?

All submodules are bundled into the tar.xz, so submodules are never used
when building outside of git.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|