[Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[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

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

-- 
2.13.6

[Qemu-devel] [PATCH v3 1/5] build: allow setting a custom GIT binary for transparent proxying

[Daniel P. Berrange]

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 <berrange@redhat.com>
---
 Makefile                 |  4 ++--
 configure                |  5 +++++
 scripts/git-submodule.sh | 29 ++++++++++++++++++++++++-----
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index ec73acfa9a..d218f8f3c8 100644
--- a/Makefile
+++ b/Makefile
@@ -21,14 +21,14 @@ git-submodule-update:
 ifeq (0,$(MAKELEVEL))
   git_module_status := $(shell \
     cd '$(SRC_PATH)' && \
-    ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
+    GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
     echo $$?; \
   )
 
 ifeq (1,$(git_module_status))
 git-submodule-update:
 	$(call quiet-command, \
-          (cd $(SRC_PATH) && ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
+          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
           "GIT","$(GIT_SUBMODULES)")
 endif
 endif
diff --git a/configure b/configure
index 285d123dbf..28ff481fe1 100755
--- a/configure
+++ b/configure
@@ -271,6 +271,7 @@ then
 else
     git_submodules=""
 fi
+git="git"
 
 # Don't accept a target_list environment variable.
 unset target_list
@@ -1303,6 +1304,8 @@ for opt do
   ;;
   --enable-capstone=system) capstone="system"
   ;;
+  --with-git=*) git="$optarg"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -5400,6 +5403,7 @@ echo "local state directory   queried at runtime"
 echo "Windows SDK       $win_sdk"
 fi
 echo "Source path       $source_path"
+echo "GIT binary        $git"
 echo "GIT submodules    $git_submodules"
 echo "C compiler        $cc"
 echo "Host C compiler   $host_cc"
@@ -5591,6 +5595,7 @@ echo "extra_cxxflags=$EXTRA_CXXFLAGS" >> $config_host_mak
 echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
 echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
 echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
+echo "GIT=$git" >> $config_host_mak
 echo "GIT_SUBMODULES=$git_submodules" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index 08932a35f0..039f782d81 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -3,14 +3,27 @@
 # This code is licensed under the GPL version 2 or later.  See
 # the COPYING file in the top-level directory.
 
-set -e
-
 substat=".git-submodule-status"
 
 command=$1
 shift
 modules="$@"
 
+test -z "$GIT" && GIT=git
+
+error() {
+    echo "$0: $*"
+    echo
+    echo "Unable to automatically checkout GIT submodules '$modules'."
+    echo "If you require use of an alternative GIT binary (for example to"
+    echo "enable use of a transparent proxy), then please specify it by"
+    echo "running configure by with the '--with-git' argument. e.g."
+    echo
+    echo " $ ./configure --with-git='tsocks git'"
+    echo
+    exit 1
+}
+
 if test -z "$modules"
 then
     test -e $substat || touch $substat
@@ -27,12 +40,18 @@ case "$command" in
 status)
     test -f "$substat" || exit 1
     trap "rm -f ${substat}.tmp" EXIT
-    git submodule status $modules > "${substat}.tmp"
+    $GIT submodule status $modules > "${substat}.tmp"
+    test $? -ne 0 && error "failed to query git submodule status"
     diff "${substat}" "${substat}.tmp" >/dev/null
     exit $?
     ;;
 update)
-    git submodule update --init $modules 1>/dev/null
-    git submodule status $modules > "${substat}"
+    $GIT submodule update --init $modules 1>/dev/null
+    test $? -ne 0 && error "failed to update modules"
+
+    $GIT submodule status $modules > "${substat}"
+    test $? -ne 0 && error "failed to save git submodule status" >&2
     ;;
 esac
+
+exit 0
-- 
2.13.6

[Qemu-devel] [PATCH v3 2/5] build: don't create temporary files in source dir

[Daniel P. Berrange]

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 <berrange@redhat.com>
---
 scripts/git-submodule.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index 039f782d81..21909ea413 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -39,10 +39,9 @@ fi
 case "$command" in
 status)
     test -f "$substat" || exit 1
-    trap "rm -f ${substat}.tmp" EXIT
-    $GIT submodule status $modules > "${substat}.tmp"
-    test $? -ne 0 && error "failed to query git submodule status"
-    diff "${substat}" "${substat}.tmp" >/dev/null
+    CURSTATUS=`$GIT submodule status $modules`
+    OLDSTATUS=`cat $substat`
+    test "$CURSTATUS" = "$OLDSTATUS"
     exit $?
     ;;
 update)
-- 
2.13.6

[Qemu-devel] [PATCH v3 3/5] build: allow automatic git submodule updates to be disabled

[Daniel P. Berrange]

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 <berrange@redhat.com>
---
 Makefile                 | 11 +++++++++++
 configure                |  7 +++++++
 scripts/git-submodule.sh |  9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/Makefile b/Makefile
index d218f8f3c8..a92d7b8e14 100644
--- a/Makefile
+++ b/Makefile
@@ -26,12 +26,23 @@ ifeq (0,$(MAKELEVEL))
   )
 
 ifeq (1,$(git_module_status))
+ifeq (no,$(GIT_UPDATE))
+git-submodule-update:
+	$(call quiet-command, \
+            echo && \
+            echo "GIT submodule checkout is out of date. Please run" && \
+            echo "  scripts/git-submodule.sh update $(GIT_SUBMODULES)" && \
+            echo "from the source directory checkout $(SRC_PATH)" && \
+            echo && \
+            exit 1)
+else
 git-submodule-update:
 	$(call quiet-command, \
           (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
           "GIT","$(GIT_SUBMODULES)")
 endif
 endif
+endif
 
 .git-submodule-status: git-submodule-update config-host.mak
 
diff --git a/configure b/configure
index 28ff481fe1..a24b62e71d 100755
--- a/configure
+++ b/configure
@@ -267,8 +267,10 @@ stack_protector=""
 
 if test -e "$source_path/.git"
 then
+    git_update=yes
     git_submodules="ui/keycodemapdb"
 else
+    git_update=no
     git_submodules=""
 fi
 git="git"
@@ -1306,6 +1308,10 @@ for opt do
   ;;
   --with-git=*) git="$optarg"
   ;;
+  --enable-git-update) git_update=yes
+  ;;
+  --disable-git-update) git_update=no
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -5597,6 +5603,7 @@ echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
 echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
 echo "GIT=$git" >> $config_host_mak
 echo "GIT_SUBMODULES=$git_submodules" >> $config_host_mak
+echo "GIT_UPDATE=$git_update" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
 
diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index 21909ea413..63ace6fc55 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -21,6 +21,15 @@ error() {
     echo
     echo " $ ./configure --with-git='tsocks git'"
     echo
+    echo "Alternatively you may disable automatic GIT submodule checkout"
+    echo "with:"
+    echo
+    echo " $ ./configure --disable-git-update'"
+    echo
+    echo "and then manually update submodules prior to running make, with:"
+    echo
+    echo " $ scripts/git-sbumodule.sh update $modules"
+    echo
     exit 1
 }
 
-- 
2.13.6

[Qemu-devel] [PATCH v3 4/5] build: don't fail if given a git submodule which does not exist

[Daniel P. Berrange]

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 <berrange@redhat.com>
---
 scripts/git-submodule.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index 63ace6fc55..2857fc57c4 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -7,7 +7,7 @@ substat=".git-submodule-status"
 
 command=$1
 shift
-modules="$@"
+maybe_modules="$@"
 
 test -z "$GIT" && GIT=git
 
@@ -33,12 +33,24 @@ error() {
     exit 1
 }
 
-if test -z "$modules"
+if test -z "$maybe_modules"
 then
     test -e $substat || touch $substat
     exit 0
 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-existant submodule $m"
+    fi
+done
+
 if ! test -e ".git"
 then
     echo "$0: unexpectedly called with submodules but no git checkout exists"
-- 
2.13.6

[Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[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 <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

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Peter Maydell]

On 3 November 2017 at 15:33, Daniel P. Berrange <berrange@redhat.com> wrote:
> 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?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 1/5] build: allow setting a custom GIT binary for transparent proxying

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On 11/03/2017 10:33 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  Makefile                 |  4 ++--
>  configure                |  5 +++++
>  scripts/git-submodule.sh | 29 ++++++++++++++++++++++++-----
>  3 files changed, 31 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v3 2/5] build: don't create temporary files in source dir

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On 11/03/2017 10:33 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  scripts/git-submodule.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v3 3/5] build: allow automatic git submodule updates to be disabled

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

On 11/03/2017 10:33 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  Makefile                 | 11 +++++++++++
>  configure                |  7 +++++++
>  scripts/git-submodule.sh |  9 +++++++++
>  3 files changed, 27 insertions(+)
> 

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v3 4/5] build: don't fail if given a git submodule which does not exist

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

On 11/03/2017 10:33 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  scripts/git-submodule.sh | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 

> +do
> +    $GIT submodule status $m 1> /dev/null 2>&1
> +    if test $? = 0
> +    then
> +        modules="$modules $m"
> +    else
> +        echo "warn: ignoring non-existant submodule $m"

s/existant/existent/

With the typo fix,
Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Philippe Mathieu-Daudé]

On 11/03/2017 12:33 PM, Daniel P. Berrange wrote:
> 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

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Alexey Kardashevskiy]

On 04/11/17 02:33, Daniel P. Berrange wrote:
> 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(-)


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)



-- 
Alexey

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Alexey Kardashevskiy]

On 06/11/17 11:22, Alexey Kardashevskiy wrote:
> On 04/11/17 02:33, Daniel P. Berrange wrote:
>> 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(-)
> 
> 
> 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)

Ufff, that was inaccurate me, sorry for the noise. This is a real thing:


[vpl1 qemu]$ git submodule status capstone
 22ead3e0bfdb87516656453336160e0a37b066bf capstone (3.0.5-rc3-14-g22ead3e0)
[vpl1 qemu]$ ssh garrison2 git -C p/qemu submodule status capstone
 22ead3e0bfdb87516656453336160e0a37b066bf capstone (3.0.5-rc3-14-g22ead3e)


So now I have to add this to .gitconfig on all build machines:

[core]
        abbrev=10

as "git submodule status" simply calls "git describe" which does not take
"--abbrev". Well, ok :(



-- 
Alexey

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Daniel P. Berrange]

On Fri, Nov 03, 2017 at 06:03:55PM +0000, Peter Maydell wrote:
> On 3 November 2017 at 15:33, Daniel P. Berrange <berrange@redhat.com> wrote:
> > 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.

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

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Daniel P. Berrange]

On Mon, Nov 06, 2017 at 10:44:01AM +0000, Daniel P. Berrange wrote:
> On Fri, Nov 03, 2017 at 06:03:55PM +0000, Peter Maydell wrote:
> > On 3 November 2017 at 15:33, Daniel P. Berrange <berrange@redhat.com> wrote:
> > > 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.

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

Re: [Qemu-devel] [PATCH v3 0/5] Various improvements/bugfixes to submodule handling

[Alexey Kardashevskiy]

On 07/11/17 01:30, Daniel P. Berrange wrote:
> On Mon, Nov 06, 2017 at 10:44:01AM +0000, Daniel P. Berrange wrote:
>> On Fri, Nov 03, 2017 at 06:03:55PM +0000, Peter Maydell wrote:
>>> On 3 November 2017 at 15:33, Daniel P. Berrange <berrange@redhat.com> wrote:
>>>> 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.
> 
> I've created this now:
> 
> https://wiki.qemu.org/Documentation/GitSubmodules

This could probably also mention something like this workaround if things
do not work as expected:

git config --global --replace-all core.abbrev 8

Thanks.


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


-- 
Alexey

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Alexey Kardashevskiy]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Alexey Kardashevskiy]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Daniel P. Berrange]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Alexey Kardashevskiy]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Alexey Kardashevskiy]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Alexey Kardashevskiy]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Daniel P. Berrange]

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

Re: [Qemu-devel] [PATCH v3 5/5] build: delay check for empty git submodule list

[Daniel P. Berrange]

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