Various improvements/bugfixes to submodule handling

Some users can't run a bare 'git' command, due to need for a transparent proxying solution such as 'tsocks'. This adds an argument to configure to let users specify such a thing:

  ./configure --with-git="tsocks git"

The submodule script is also updated to give the user a hint about using this flag, if we fail to checkout modules.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 Makefile                 |  4 ++--
 configure                |  5 +++++
 scripts/git-submodule.sh | 29 ++++++++++++++++++++++++-----
 3 files changed, 31 insertions(+), 7 deletions(-)

There are cases where users do VPATH builds with the source directory being on a read-only volume. In such a case they have to manually run the command 'git-submodule.sh ...modules...' ahead of time. When checking for status we should not then write into the source dir.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 scripts/git-submodule.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Some people building QEMU use VPATH builds where the source directory is on a read-only volume. In such a case 'scripts/git-submodules.sh update' will always fail and users are required to run it manually themselves on their original writable source directory.

While this is already supported, it is nice to give users a command line flag to configure to permanently disable automatic submodule updates, as it means they won't get hard to diagnose failures from git-submodules.sh at an arbitrary later date.

This patch thus introduces a flag '--disable-git-update' which will prevent 'make' from ever running 'scripts/git-submodules.sh update'. It will still run the 'status' command to determine if a submodule update is needed, but when it does this it'll simply stop and print a message instructing the developer what todo. eg

$ ./configure  --target-list=x86_64-softmmu --disable-git-update
...snip...

$ make
  GEN     config-host.h
  GEN     trace/generated-tcg-tracers.h
  GEN     trace/generated-helpers-wrappers.h
  GEN     trace/generated-helpers.h
  GEN     trace/generated-helpers.c
  GEN     module_block.h

GIT submodule checkout is out of date. Please run
scripts/git-submodule.sh update ui/keycodemapdb
from the source directory checkout /home/berrange/src/virt/qemu

make: *** [Makefile:31: git-submodule-update] Error 1

Signed-off-by Daniel P. Berrange <[email protected]>

---
 Makefile                 | 11 +++++++++++
 configure                |  7 +++++++
 scripts/git-submodule.sh |  9 +++++++++
 3 files changed, 27 insertions(+)

If going back in time in git history, across a commit that introduces a new submodule, the 'git-submodule.sh' script will fail, causing rebuild to fail.

This is because config-host.mak contains a GIT_SUBMODULES variable that lists a submodule that only exists in the later commit. config-host.mak won't get repopulated until config.status is invoked, but make won't get this far due to the submodule error.

This change makes 'git-submodule.sh' check whether each module is known to git and drops any which are not present. A warning message will be printed when any submodule is dropped in this manner.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 scripts/git-submodule.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh +11 -0

 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"
 

Quoted 31 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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
 

scripts/git-submodule.sh do · line 48

Quoted 2 lines

scripts/git-submodule.sh +1 -1

-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" = ""

Quoted 38 lines · Alexey Kardashevskiy

Quoted 28 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh do · line 48

Quoted 5 lines

scripts/git-submodule.sh +1 -1

@@ -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:

Quoted 1 line

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.

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

Quoted 39 lines · Alexey Kardashevskiy

Quoted 38 lines · Alexey Kardashevskiy

Quoted 28 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh do · line 48

Quoted 5 lines

scripts/git-submodule.sh +1 -1

@@ -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:

Quoted 8 lines

Quoted 1 line

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"

Quoted 1 line

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

Quoted 55 lines · Daniel P. Berrange

Quoted 39 lines · Alexey Kardashevskiy

Quoted 38 lines · Alexey Kardashevskiy

Quoted 28 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh do · line 48

Quoted 5 lines

scripts/git-submodule.sh +1 -1

@@ -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:

Quoted 8 lines

Quoted 1 line

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"

Quoted 1 line

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

Quoted 4 lines

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?

Quoted 41 lines · Alexey Kardashevskiy

Quoted 39 lines · Alexey Kardashevskiy

Quoted 28 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh do · line 48

Quoted 5 lines

scripts/git-submodule.sh +1 -1

@@ -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" = ""

Quoted 6 lines

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"

Quoted 1 line

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

Quoted 39 lines · Alexey Kardashevskiy

Quoted 38 lines · Alexey Kardashevskiy

Quoted 28 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh do · line 48

Quoted 5 lines

scripts/git-submodule.sh +1 -1

@@ -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:

Quoted 8 lines

Quoted 1 line

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"

Quoted 1 line

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

Quoted 55 lines · Daniel P. Berrange

Quoted 39 lines · Alexey Kardashevskiy

Quoted 38 lines · Alexey Kardashevskiy

Quoted 28 lines · Daniel P. Berrange

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 <[email protected]>

---
 scripts/git-submodule.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

scripts/git-submodule.sh +0 -6

index 2857fc57c4..90376a7dae 100755
@@ -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

scripts/git-submodule.sh do · line 48

Quoted 5 lines

scripts/git-submodule.sh +1 -1

@@ -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:

Quoted 8 lines

Quoted 1 line

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"

Quoted 1 line

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

Quoted 3 lines

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?

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

Quoted 2 lines · Daniel P. Berrange

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

Can I ask you to write up a bit for the release notes at
https://wiki.qemu.org/ChangeLog/2.11#Build_Information
covering what users need to know about the new submodule handling, please?

Quoted 8 lines · Peter Maydell

Quoted 2 lines · Daniel P. Berrange

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

Can I ask you to write up a bit for the release notes at
https://wiki.qemu.org/ChangeLog/2.11#Build_Information
covering what users need to know about the new submodule handling, please?

Certainly, will do that.

Perhaps have something in the general wiki pages about building QEMU, and link to it from the changelog.

Quoted 10 lines · Daniel P. Berrange

Quoted 8 lines · Peter Maydell

Quoted 2 lines · Daniel P. Berrange

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

Can I ask you to write up a bit for the release notes at
https://wiki.qemu.org/ChangeLog/2.11#Build_Information
covering what users need to know about the new submodule handling, please?

Quoted 4 lines

Certainly, will do that.

Perhaps have something in the general wiki pages about building QEMU, and link to it from the changelog.

I've created this now:

https://wiki.qemu.org/Documentation/GitSubmodules

and linked it from here:

https://wiki.qemu.org/ChangeLog/2.11#Build_Information

and a couple of other places in the wiki.

Regards,
Daniel

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

v2: https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg06272.html

Changed in v3:

- Avoid use of mktemp
- Use '=' instead of '==' (bashism) in conditional tests
- Fix problem with short-circuiting for empty submodule list
- Ensure help message is displayed on all errors

Daniel P. Berrange (5):
build: allow setting a custom GIT binary for transparent proxying
build: don't create temporary files in source dir
build: allow automatic git submodule updates to be disabled
build: don't fail if given a git submodule which does not exist
build: delay check for empty git submodule list

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

v2: https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg06272.html

Changed in v3:

- Avoid use of mktemp
- Use '=' instead of '==' (bashism) in conditional tests
- Fix problem with short-circuiting for empty submodule list
- Ensure help message is displayed on all errors

Daniel P. Berrange (5):
build: allow setting a custom GIT binary for transparent proxying
build: don't create temporary files in source dir
build: allow automatic git submodule updates to be disabled
build: don't fail if given a git submodule which does not exist
build: delay check for empty git submodule list

 Makefile                 | 15 ++++++++--
 configure                | 12 ++++++++
 scripts/git-submodule.sh | 72 ++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 83 insertions(+), 16 deletions(-)

Quoted 19 lines · Daniel P. Berrange

This short series of patches improves the reliability of the submodule handling to cope with various dev env scenarios that have since emerged

v2: https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg06272.html

Changed in v3:

- Avoid use of mktemp
- Use '=' instead of '==' (bashism) in conditional tests
- Fix problem with short-circuiting for empty submodule list
- Ensure help message is displayed on all errors

Daniel P. Berrange (5):
build: allow setting a custom GIT binary for transparent proxying
build: don't create temporary files in source dir
build: allow automatic git submodule updates to be disabled
build: don't fail if given a git submodule which does not exist
build: delay check for empty git submodule list

Quoted 4 lines

 Makefile                 | 15 ++++++++--
 configure                | 12 ++++++++
 scripts/git-submodule.sh | 72 ++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 83 insertions(+), 16 deletions(-)

It is getting more complicated.

So, same setup,
git checkout v2.10.0
./configure --target-list=ppc64-softmmu
make

All good.
Then:

git checkout git-submodule (upstream + this patchset) ./configure --target-list=ppc64-softmmu

I see an error and educational page about --disable-git-update, good. ./configure --target-list=ppc64-softmmu --disable-git-update make

Now I get:

GIT submodule checkout is out of date. Please run
scripts/git-submodule.sh update ui/keycodemapdb dtc capstone
from the source directory checkout /home/aik/p/qemu

Good, as I wanted.
I do this on the server:
scripts/git-submodule.sh update ui/keycodemapdb dtc capstone

and repeat "make" on the build machine but get the very same error. Then I add "set -x" to scripts/git-submodule.sh and see:

+ substat=.git-submodule-status
+ command=status
+ shift
+ maybe_modules=ui/keycodemapdb dtc capstone
+ test -z git
+ modules=
+ git submodule status ui/keycodemapdb
+ test 0 = 0
+ modules= ui/keycodemapdb
+ git submodule status dtc
+ test 0 = 0
+ modules= ui/keycodemapdb dtc
+ git submodule status capstone
+ test 0 = 0
+ modules= ui/keycodemapdb dtc capstone
+ test -n ui/keycodemapdb dtc capstone
+ test -e .git
+ test -z ui/keycodemapdb dtc capstone
+ test -f .git-submodule-status
+ git submodule status ui/keycodemapdb dtc capstone
+ CURSTATUS=-22ead3e0bfdb87516656453336160e0a37b066bf capstone 558cd81bdd432769b59bff01240c44f82cfb1a9d dtc (v1.4.4) 10739aa26051a5d49d88132604539d3ed085e72e ui/keycodemapdb (heads/master)

+ cat .git-submodule-status
+ OLDSTATUS= 558cd81bdd432769b59bff01240c44f82cfb1a9d dtc (v1.4.4) 10739aa26051a5d49d88132604539d3ed085e72e ui/keycodemapdb (heads/master)

+ test -22ead3e0bfdb87516656453336160e0a37b066bf capstone 558cd81bdd432769b59bff01240c44f82cfb1a9d dtc (v1.4.4) 10739aa26051a5d49d88132604539d3ed085e72e ui/keycodemapdb (heads/master) =
558cd81bdd432769b59bff01240c44f82cfb1a9d dtc (v1.4.4)
10739aa26051a5d49d88132604539d3ed085e72e ui/keycodemapdb (heads/master)

+ exit 1

What is that new module - capstone - about? It is not in the status file:

[vpl1 qemu]$ cat .git-submodule-status
558cd81bdd432769b59bff01240c44f82cfb1a9d dtc (v1.4.4) 10739aa26051a5d49d88132604539d3ed085e72e ui/keycodemapdb (heads/master)