[Qemu-devel] [PATCH v2 0/4] Various improvements to submodule handling

[Qemu-devel] [PATCH v2 0/4] Various improvements to submodule handling

[Daniel P. Berrange]

I've sent a variety of improvements to submodule handling as separate patches
and its getting slightly confusing. So here is a series with all patches added
together. I've also included a fix that helps us go backwards in time across
the introduction of a submodule.

Daniel P. Berrange (4):
  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

 Makefile                 | 15 ++++++++++--
 configure                | 14 +++++++++++-
 scripts/git-submodule.sh | 59 ++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 76 insertions(+), 12 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH v2 1/4] 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 | 30 +++++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 9372742f86..4c9d0eaef2 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 03547cea6a..65765968f3 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
@@ -1294,6 +1295,8 @@ for opt do
           error_exit "vhost-user isn't available on win32"
       fi
   ;;
+  --with-git=*) git="$optarg"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -5338,6 +5341,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"
@@ -5528,6 +5532,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..c66567d409 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -3,14 +3,19 @@
 # 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() {
+    printf "$0: %s\n" "$*" >&2
+    exit 1
+}
+
 if test -z "$modules"
 then
     test -e $substat || touch $substat
@@ -27,12 +32,27 @@ 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
+    if test $? -ne 0 ; then
+        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
+    fi
+    $GIT submodule status $modules > "${substat}"
+    test $? -ne 0 && error "failed to save git submodule status"
     ;;
 esac
+
+exit 0
-- 
2.13.6

[Qemu-devel] [PATCH v2 2/4] 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...'  manually 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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
index c66567d409..586ff32293 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -31,10 +31,10 @@ 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
+    substat_tmp=$(mktemp)
+    trap "rm -f ${substat_tmp}" EXIT
+    $GIT submodule status $modules > "${substat_tmp}"
+    diff "${substat}" "${substat_tmp}" >/dev/null
     exit $?
     ;;
 update)
-- 
2.13.6

[Qemu-devel] [PATCH v2 3/4] 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                |  9 ++++++++-
 scripts/git-submodule.sh |  9 +++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4c9d0eaef2..cefb75dc8d 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 65765968f3..8a569e1770 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"
@@ -1297,11 +1299,15 @@ 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"
       exit 1
-  ;;
+      ;;
   esac
 done
 
@@ -5534,6 +5540,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 586ff32293..30fd83db55 100755
--- a/scripts/git-submodule.sh
+++ b/scripts/git-submodule.sh
@@ -48,6 +48,15 @@ update)
         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
     fi
     $GIT submodule status $modules > "${substat}"
-- 
2.13.6

[Qemu-devel] [PATCH v2 4/4] 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 30fd83db55..60b3b9bdeb 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
 
@@ -16,12 +16,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

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

[Alexey Kardashevskiy]

On 28/10/17 00:14, 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(-)
> 
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> index 30fd83db55..60b3b9bdeb 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
>  
> @@ -16,12 +16,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


./scripts/git-submodule.sh: 29: test: 0: unexpected operator

This helps:

-    if test $? == 0
+    if test "$?" -eq "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"
> 


-- 
Alexey

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

[Alexey Kardashevskiy]

On 28/10/17 00:14, 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 | 30 +++++++++++++++++++++++++-----
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9372742f86..4c9d0eaef2 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 03547cea6a..65765968f3 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
> @@ -1294,6 +1295,8 @@ for opt do
>            error_exit "vhost-user isn't available on win32"
>        fi
>    ;;
> +  --with-git=*) git="$optarg"
> +  ;;
>    *)
>        echo "ERROR: unknown option $opt"
>        echo "Try '$0 --help' for more information"
> @@ -5338,6 +5341,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"
> @@ -5528,6 +5532,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..c66567d409 100755
> --- a/scripts/git-submodule.sh
> +++ b/scripts/git-submodule.sh
> @@ -3,14 +3,19 @@
>  # 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() {
> +    printf "$0: %s\n" "$*" >&2
> +    exit 1
> +}
> +
>  if test -z "$modules"
>  then
>      test -e $substat || touch $substat
> @@ -27,12 +32,27 @@ 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
> +    if test $? -ne 0 ; then
> +        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
> +    fi
> +    $GIT submodule status $modules > "${substat}"
> +    test $? -ne 0 && error "failed to save git submodule status"


The way I am testing it - I simply delete .git-submodule-status (I used to
change it but deleting works as well) and then I get:

./scripts/git-submodule.sh: 74: ./scripts/git-submodule.sh: cannot create
.git-submodule-status: Read-only file system

because "git submodule update" returns 0 (as everything is up to date) but
updating status fails. Which is fine, I would just like to get a better
message as even after few days of reading this script, I do not remember in
what order I should pass submodules to scripts/git-submodule.sh. Yeah, I
can find it in output but even the name of script to run does not stick to
my brain :(

Something like this:

-    test $? -ne 0 && error "failed to save git submodule status"
+    test $? -ne 0 && error "\"$GIT submodule status $modules\" failed to
save git submodule status"



>      ;;
>  esac
> +
> +exit 0
> 


-- 
Alexey

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

[Eric Blake]

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

On 10/27/2017 03:14 PM, 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...'  manually ahead of time. When checking for

One of the two "manually" can be removed (either one, and the sentence
still makes sense)

> status we should not then write into the source dir.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  scripts/git-submodule.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> index c66567d409..586ff32293 100755
> --- a/scripts/git-submodule.sh
> +++ b/scripts/git-submodule.sh
> @@ -31,10 +31,10 @@ 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
> +    substat_tmp=$(mktemp)

Is mktemp portable enough?  Hopefully so.

-- 
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 v2 3/4] build: allow automatic git submodule updates to be disabled

[Eric Blake]

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

On 10/27/2017 03:14 PM, 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.
> 

> @@ -1297,11 +1299,15 @@ for opt do
>    ;;
>    --with-git=*) git="$optarg"
>    ;;
> +  --enable-git-update) git_update=yes
> +  ;;
> +  --disable-git-update) git_update=no
> +  ;;

Consistent, which makes...

>    *)
>        echo "ERROR: unknown option $opt"
>        echo "Try '$0 --help' for more information"
>        exit 1
> -  ;;
> +      ;;

...this an unrelated whitespace change?


-- 
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 v2 0/4] Various improvements to submodule handling

[Eric Blake]

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

On 10/27/2017 03:14 PM, Daniel P. Berrange wrote:
> I've sent a variety of improvements to submodule handling as separate patches
> and its getting slightly confusing. So here is a series with all patches added
> together. I've also included a fix that helps us go backwards in time across
> the introduction of a submodule.
> 
> Daniel P. Berrange (4):
>   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

I have some minor comments on a couple of individual patches, but for
the series:
Reviewed-by: Eric Blake <eblake@redhat.com>

> 
>  Makefile                 | 15 ++++++++++--
>  configure                | 14 +++++++++++-
>  scripts/git-submodule.sh | 59 ++++++++++++++++++++++++++++++++++++++++--------
>  3 files changed, 76 insertions(+), 12 deletions(-)
> 

-- 
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 v2 4/4] build: don't fail if given a git submodule which does not exist

[Eric Blake]

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

On 10/28/2017 03:27 AM, Alexey Kardashevskiy wrote:
> On 28/10/17 00:14, 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.
>>

>> +do
>> +    $GIT submodule status $m 1> /dev/null 2>&1
>> +    if test $? == 0
> 
> 
> ./scripts/git-submodule.sh: 29: test: 0: unexpected operator

Ah, right, == is a bashism.

> 
> This helps:
> 
> -    if test $? == 0
> +    if test "$?" -eq "0"

Or 'if test $? = 0'

-- 
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 v2 2/4] build: don't create temporary files in source dir

[Alexey Kardashevskiy]

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

On 28/10/17 16:25, Eric Blake wrote:
> On 10/27/2017 03:14 PM, 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...'  manually ahead of time. When checking for
> 
> One of the two "manually" can be removed (either one, and the sentence
> still makes sense)
> 
>> status we should not then write into the source dir.
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  scripts/git-submodule.sh | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
>> index c66567d409..586ff32293 100755
>> --- a/scripts/git-submodule.sh
>> +++ b/scripts/git-submodule.sh
>> @@ -31,10 +31,10 @@ 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
>> +    substat_tmp=$(mktemp)
> 
> Is mktemp portable enough?  Hopefully so.

It could also be some fixed file name in the build directory.


-- 
Alexey


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

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

[Daniel P. Berrange]

On Sat, Oct 28, 2017 at 12:53:50PM +1100, Alexey Kardashevskiy wrote:
> On 28/10/17 00:14, 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 | 30 +++++++++++++++++++++++++-----
> >  3 files changed, 32 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 9372742f86..4c9d0eaef2 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 03547cea6a..65765968f3 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
> > @@ -1294,6 +1295,8 @@ for opt do
> >            error_exit "vhost-user isn't available on win32"
> >        fi
> >    ;;
> > +  --with-git=*) git="$optarg"
> > +  ;;
> >    *)
> >        echo "ERROR: unknown option $opt"
> >        echo "Try '$0 --help' for more information"
> > @@ -5338,6 +5341,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"
> > @@ -5528,6 +5532,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..c66567d409 100755
> > --- a/scripts/git-submodule.sh
> > +++ b/scripts/git-submodule.sh
> > @@ -3,14 +3,19 @@
> >  # 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() {
> > +    printf "$0: %s\n" "$*" >&2
> > +    exit 1
> > +}
> > +
> >  if test -z "$modules"
> >  then
> >      test -e $substat || touch $substat
> > @@ -27,12 +32,27 @@ 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
> > +    if test $? -ne 0 ; then
> > +        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
> > +    fi
> > +    $GIT submodule status $modules > "${substat}"
> > +    test $? -ne 0 && error "failed to save git submodule status"
> 
> 
> The way I am testing it - I simply delete .git-submodule-status (I used to
> change it but deleting works as well) and then I get:
> 
> ./scripts/git-submodule.sh: 74: ./scripts/git-submodule.sh: cannot create
> .git-submodule-status: Read-only file system
> 
> because "git submodule update" returns 0 (as everything is up to date) but
> updating status fails. Which is fine, I would just like to get a better
> message as even after few days of reading this script, I do not remember in
> what order I should pass submodules to scripts/git-submodule.sh. Yeah, I
> can find it in output but even the name of script to run does not stick to
> my brain :(
> 
> Something like this:
> 
> -    test $? -ne 0 && error "failed to save git submodule status"
> +    test $? -ne 0 && error "\"$GIT submodule status $modules\" failed to
> save git submodule status"

Take a look at the 3rd patch - it now prints out the exact command you
would need to run in the writable-source dir.

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 v2 1/4] build: allow setting a custom GIT binary for transparent proxying

[Alexey Kardashevskiy]

On 29/10/17 07:45, Daniel P. Berrange wrote:
> On Sat, Oct 28, 2017 at 12:53:50PM +1100, Alexey Kardashevskiy wrote:
>> On 28/10/17 00:14, 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 | 30 +++++++++++++++++++++++++-----
>>>  3 files changed, 32 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 9372742f86..4c9d0eaef2 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 03547cea6a..65765968f3 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
>>> @@ -1294,6 +1295,8 @@ for opt do
>>>            error_exit "vhost-user isn't available on win32"
>>>        fi
>>>    ;;
>>> +  --with-git=*) git="$optarg"
>>> +  ;;
>>>    *)
>>>        echo "ERROR: unknown option $opt"
>>>        echo "Try '$0 --help' for more information"
>>> @@ -5338,6 +5341,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"
>>> @@ -5528,6 +5532,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..c66567d409 100755
>>> --- a/scripts/git-submodule.sh
>>> +++ b/scripts/git-submodule.sh
>>> @@ -3,14 +3,19 @@
>>>  # 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() {
>>> +    printf "$0: %s\n" "$*" >&2
>>> +    exit 1
>>> +}
>>> +
>>>  if test -z "$modules"
>>>  then
>>>      test -e $substat || touch $substat
>>> @@ -27,12 +32,27 @@ 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
>>> +    if test $? -ne 0 ; then
>>> +        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
>>> +    fi
>>> +    $GIT submodule status $modules > "${substat}"
>>> +    test $? -ne 0 && error "failed to save git submodule status"
>>
>>
>> The way I am testing it - I simply delete .git-submodule-status (I used to
>> change it but deleting works as well) and then I get:
>>
>> ./scripts/git-submodule.sh: 74: ./scripts/git-submodule.sh: cannot create
>> .git-submodule-status: Read-only file system
>>
>> because "git submodule update" returns 0 (as everything is up to date) but
>> updating status fails. Which is fine, I would just like to get a better
>> message as even after few days of reading this script, I do not remember in
>> what order I should pass submodules to scripts/git-submodule.sh. Yeah, I
>> can find it in output but even the name of script to run does not stick to
>> my brain :(
>>
>> Something like this:
>>
>> -    test $? -ne 0 && error "failed to save git submodule status"
>> +    test $? -ne 0 && error "\"$GIT submodule status $modules\" failed to
>> save git submodule status"
> 
> Take a look at the 3rd patch - it now prints out the exact command you
> would need to run in the writable-source dir.

For the message in 3/4 to show up, I need to 1) know about
--disable-git-update and 2) use it. My testcase is lot more common - I did
not use --disable-git-update, and a strange message about writing read-only
folder appears, exactly like when I started the conversation.


-- 
Alexey

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

[Daniel P. Berrange]

On Sun, Oct 29, 2017 at 12:47:07PM +1100, Alexey Kardashevskiy wrote:
> On 29/10/17 07:45, Daniel P. Berrange wrote:
> > On Sat, Oct 28, 2017 at 12:53:50PM +1100, Alexey Kardashevskiy wrote:
> >> On 28/10/17 00:14, 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 | 30 +++++++++++++++++++++++++-----
> >>>  3 files changed, 32 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/Makefile b/Makefile
> >>> index 9372742f86..4c9d0eaef2 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 03547cea6a..65765968f3 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
> >>> @@ -1294,6 +1295,8 @@ for opt do
> >>>            error_exit "vhost-user isn't available on win32"
> >>>        fi
> >>>    ;;
> >>> +  --with-git=*) git="$optarg"
> >>> +  ;;
> >>>    *)
> >>>        echo "ERROR: unknown option $opt"
> >>>        echo "Try '$0 --help' for more information"
> >>> @@ -5338,6 +5341,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"
> >>> @@ -5528,6 +5532,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..c66567d409 100755
> >>> --- a/scripts/git-submodule.sh
> >>> +++ b/scripts/git-submodule.sh
> >>> @@ -3,14 +3,19 @@
> >>>  # 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() {
> >>> +    printf "$0: %s\n" "$*" >&2
> >>> +    exit 1
> >>> +}
> >>> +
> >>>  if test -z "$modules"
> >>>  then
> >>>      test -e $substat || touch $substat
> >>> @@ -27,12 +32,27 @@ 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
> >>> +    if test $? -ne 0 ; then
> >>> +        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
> >>> +    fi
> >>> +    $GIT submodule status $modules > "${substat}"
> >>> +    test $? -ne 0 && error "failed to save git submodule status"
> >>
> >>
> >> The way I am testing it - I simply delete .git-submodule-status (I used to
> >> change it but deleting works as well) and then I get:
> >>
> >> ./scripts/git-submodule.sh: 74: ./scripts/git-submodule.sh: cannot create
> >> .git-submodule-status: Read-only file system
> >>
> >> because "git submodule update" returns 0 (as everything is up to date) but
> >> updating status fails. Which is fine, I would just like to get a better
> >> message as even after few days of reading this script, I do not remember in
> >> what order I should pass submodules to scripts/git-submodule.sh. Yeah, I
> >> can find it in output but even the name of script to run does not stick to
> >> my brain :(
> >>
> >> Something like this:
> >>
> >> -    test $? -ne 0 && error "failed to save git submodule status"
> >> +    test $? -ne 0 && error "\"$GIT submodule status $modules\" failed to
> >> save git submodule status"
> > 
> > Take a look at the 3rd patch - it now prints out the exact command you
> > would need to run in the writable-source dir.
> 
> For the message in 3/4 to show up, I need to 1) know about
> --disable-git-update and 2) use it. My testcase is lot more common - I did
> not use --disable-git-update, and a strange message about writing read-only
> folder appears, exactly like when I started the conversation.

That isn't the behaviour I get with this patch series applied. I made my
source dir readonly, and then tried a VPATH build and got the expected
messages

  ...snip....
  GIT     ui/keycodemapdb
error: could not lock config file .git/config: Permission denied
error: could not lock config file .git/config: Permission denied
fatal: Failed to register url for submodule path 'ui/keycodemapdb'

Unable to automatically checkout GIT submodules ' ui/keycodemapdb'.
If you require use of an alternative GIT binary (for example to
enable use of a transparent proxy), then please specify it by
running configure by with the '--with-git' argument. e.g.

 $ ./configure --with-git='tsocks git'

Alternatively you may disable automatic GIT submodule checkout
with:

 $ ./configure --disable-git-update'

and then manually update submodules prior to running make, with:

 $ scripts/git-sbumodule.sh update  ui/keycodemapdb

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

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 v2 1/4] build: allow setting a custom GIT binary for transparent proxying

[Alexey Kardashevskiy]

On 29/10/17 18:57, Daniel P. Berrange wrote:
> On Sun, Oct 29, 2017 at 12:47:07PM +1100, Alexey Kardashevskiy wrote:
>> On 29/10/17 07:45, Daniel P. Berrange wrote:
>>> On Sat, Oct 28, 2017 at 12:53:50PM +1100, Alexey Kardashevskiy wrote:
>>>> On 28/10/17 00:14, 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 | 30 +++++++++++++++++++++++++-----
>>>>>  3 files changed, 32 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/Makefile b/Makefile
>>>>> index 9372742f86..4c9d0eaef2 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 03547cea6a..65765968f3 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
>>>>> @@ -1294,6 +1295,8 @@ for opt do
>>>>>            error_exit "vhost-user isn't available on win32"
>>>>>        fi
>>>>>    ;;
>>>>> +  --with-git=*) git="$optarg"
>>>>> +  ;;
>>>>>    *)
>>>>>        echo "ERROR: unknown option $opt"
>>>>>        echo "Try '$0 --help' for more information"
>>>>> @@ -5338,6 +5341,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"
>>>>> @@ -5528,6 +5532,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..c66567d409 100755
>>>>> --- a/scripts/git-submodule.sh
>>>>> +++ b/scripts/git-submodule.sh
>>>>> @@ -3,14 +3,19 @@
>>>>>  # 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() {
>>>>> +    printf "$0: %s\n" "$*" >&2
>>>>> +    exit 1
>>>>> +}
>>>>> +
>>>>>  if test -z "$modules"
>>>>>  then
>>>>>      test -e $substat || touch $substat
>>>>> @@ -27,12 +32,27 @@ 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
>>>>> +    if test $? -ne 0 ; then
>>>>> +        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
>>>>> +    fi
>>>>> +    $GIT submodule status $modules > "${substat}"
>>>>> +    test $? -ne 0 && error "failed to save git submodule status"
>>>>
>>>>
>>>> The way I am testing it - I simply delete .git-submodule-status (I used to
>>>> change it but deleting works as well) and then I get:
>>>>
>>>> ./scripts/git-submodule.sh: 74: ./scripts/git-submodule.sh: cannot create
>>>> .git-submodule-status: Read-only file system
>>>>
>>>> because "git submodule update" returns 0 (as everything is up to date) but
>>>> updating status fails. Which is fine, I would just like to get a better
>>>> message as even after few days of reading this script, I do not remember in
>>>> what order I should pass submodules to scripts/git-submodule.sh. Yeah, I
>>>> can find it in output but even the name of script to run does not stick to
>>>> my brain :(
>>>>
>>>> Something like this:
>>>>
>>>> -    test $? -ne 0 && error "failed to save git submodule status"
>>>> +    test $? -ne 0 && error "\"$GIT submodule status $modules\" failed to
>>>> save git submodule status"
>>>
>>> Take a look at the 3rd patch - it now prints out the exact command you
>>> would need to run in the writable-source dir.
>>
>> For the message in 3/4 to show up, I need to 1) know about
>> --disable-git-update and 2) use it. My testcase is lot more common - I did
>> not use --disable-git-update, and a strange message about writing read-only
>> folder appears, exactly like when I started the conversation.
> 
> That isn't the behaviour I get with this patch series applied. I made my
> source dir readonly, and then tried a VPATH build and got the expected
> messages

I run ./scripts/git-submodule.sh on a server (where source directory is
writeable), them "rm .git-submodule-status", then run "Makefile" on a build
machine (which has the source directory mounted as read-only). I basically
recreate the situation where I was when I just discovered this brand new
./scripts/git-submodule.sh.

> 
>   ...snip....
>   GIT     ui/keycodemapdb
> error: could not lock config file .git/config: Permission denied
> error: could not lock config file .git/config: Permission denied
> fatal: Failed to register url for submodule path 'ui/keycodemapdb'


I added "set -x" to the script and this is what I got:

make: Entering directory '/home/aik/pbuild/qemu-fstn1-ppc64'
+ substat=.git-submodule-status
+ command=status
+ shift
+ maybe_modules=ui/keycodemapdb dtc
+ test -z git
+ test -z ui/keycodemapdb dtc
+ modules=
+ git submodule status ui/keycodemapdb
+ test 0 -eq 0
+ modules= ui/keycodemapdb
+ git submodule status dtc
+ test 0 -eq 0
+ modules= ui/keycodemapdb dtc
+ test -e .git
+ test -f .git-submodule-status
+ exit 1
  GIT     ui/keycodemapdb dtc
+ substat=.git-submodule-status
+ command=update
+ shift
+ maybe_modules=ui/keycodemapdb dtc
+ test -z git
+ test -z ui/keycodemapdb dtc
+ modules=
+ git submodule status ui/keycodemapdb
+ test 0 -eq 0
+ modules= ui/keycodemapdb
+ git submodule status dtc
+ test 0 -eq 0
+ modules= ui/keycodemapdb dtc
+ test -e .git
+ git submodule update --init ui/keycodemapdb dtc
+ test 0 -ne 0
./scripts/git-submodule.sh: 74: ./scripts/git-submodule.sh: cannot create
.git-submodule-status: Read-only file system
+ git submodule status ui/keycodemapdb dtc
+ test 2 -ne 0
+ error failed to save git submodule status
+ printf ./scripts/git-submodule.sh: %s\n failed to save git submodule status
./scripts/git-submodule.sh: failed to save git submodule status
+ exit 1
Makefile:40: recipe for target 'git-submodule-update' failed
make: *** [git-submodule-update] Error 1
make: *** Waiting for unfinished jobs....
make: Leaving directory '/home/aik/pbuild/qemu-fstn1-ppc64'


"git submodule update" does not fail, why would it - the submodules are up
to date.


> 
> Unable to automatically checkout GIT submodules ' ui/keycodemapdb'.
> If you require use of an alternative GIT binary (for example to
> enable use of a transparent proxy), then please specify it by
> running configure by with the '--with-git' argument. e.g.
> 
>  $ ./configure --with-git='tsocks git'
> 
> Alternatively you may disable automatic GIT submodule checkout
> with:
> 
>  $ ./configure --disable-git-update'
> 
> and then manually update submodules prior to running make, with:
> 
>  $ scripts/git-sbumodule.sh update  ui/keycodemapdb


I know that now, all I am asking is an error message to print exact command
to run...


> 
> make: *** [Makefile:40: git-submodule-update] Error 1
> 
> Regards,
> Daniel
> 


-- 
Alexey

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

[Daniel P. Berrange]

On Mon, Oct 30, 2017 at 01:08:56AM +1100, Alexey Kardashevskiy wrote:
> 
> I run ./scripts/git-submodule.sh on a server (where source directory is
> writeable), them "rm .git-submodule-status", then run "Makefile" on a build
> machine (which has the source directory mounted as read-only). I basically
> recreate the situation where I was when I just discovered this brand new
> ./scripts/git-submodule.sh.

Don't rm the .git-submodule-status. That means 'make' thinks the modules
are out of date.

Just run 'scripts/git-submodules.sh ...mods..' on the writable source
dir, and then run 'make' on the build machine.

> I know that now, all I am asking is an error message to print exact command
> to run...

If you hadn't deleted the .git-submodule-status, it would have worked fine.


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 v2 1/4] build: allow setting a custom GIT binary for transparent proxying

[Alexey Kardashevskiy]

On 30/10/17 03:29, Daniel P. Berrange wrote:
> On Mon, Oct 30, 2017 at 01:08:56AM +1100, Alexey Kardashevskiy wrote:
>>
>> I run ./scripts/git-submodule.sh on a server (where source directory is
>> writeable), them "rm .git-submodule-status", then run "Makefile" on a build
>> machine (which has the source directory mounted as read-only). I basically
>> recreate the situation where I was when I just discovered this brand new
>> ./scripts/git-submodule.sh.
> 
> Don't rm the .git-submodule-status. That means 'make' thinks the modules
> are out of date.
> 
> Just run 'scripts/git-submodules.sh ...mods..' on the writable source
> dir, and then run 'make' on the build machine.
> 
>> I know that now, all I am asking is an error message to print exact command
>> to run...
> 
> If you hadn't deleted the .git-submodule-status, it would have worked fine.

No.

I do this on a server:

[vpl1 qemu]$ git co v2.10.0
At this point no .git-submodule-status is expected/required/exist.

[vpl1 qemu]$ ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/
-j24

Configure succeeds, compiles just fine, it has been working like this for
years.


Now:

[vpl1 qemu]$ git co git-submodule  (this is your stuff)
[vpl1 qemu]$ ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/
-j24
touch: cannot touch ‘.git-submodule-status’: Read-only file system
make: Entering directory `/home/aik/pbuild/qemu-aikhostos2-ppc64'
  GEN     ppc64-softmmu/config-devices.mak.tmp
[...]

./scripts/git-submodule.sh: line 74: .git-submodule-status: Read-only file
system
./scripts/git-submodule.sh: failed to save git submodule status
make: *** [git-submodule-update] Error 1


What, why? Out of nowhere we write to read-only folder and we do not even
bother telling why.

Am I the only one who would be confused by this change?


I only delete that file because I am testing the real situation which I
saw. Looks like nobody else is really testing this :(


-- 
Alexey

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

[Daniel P. Berrange]

On Mon, Oct 30, 2017 at 10:49:01AM +1100, Alexey Kardashevskiy wrote:
> On 30/10/17 03:29, Daniel P. Berrange wrote:
> > On Mon, Oct 30, 2017 at 01:08:56AM +1100, Alexey Kardashevskiy wrote:
> >>
> >> I run ./scripts/git-submodule.sh on a server (where source directory is
> >> writeable), them "rm .git-submodule-status", then run "Makefile" on a build
> >> machine (which has the source directory mounted as read-only). I basically
> >> recreate the situation where I was when I just discovered this brand new
> >> ./scripts/git-submodule.sh.
> > 
> > Don't rm the .git-submodule-status. That means 'make' thinks the modules
> > are out of date.
> > 
> > Just run 'scripts/git-submodules.sh ...mods..' on the writable source
> > dir, and then run 'make' on the build machine.
> > 
> >> I know that now, all I am asking is an error message to print exact command
> >> to run...
> > 
> > If you hadn't deleted the .git-submodule-status, it would have worked fine.
> 
> No.
> 
> I do this on a server:
> 
> [vpl1 qemu]$ git co v2.10.0
> At this point no .git-submodule-status is expected/required/exist.
> 
> [vpl1 qemu]$ ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/
> -j24
> 
> Configure succeeds, compiles just fine, it has been working like this for
> years.
> 
> 
> Now:
> 
> [vpl1 qemu]$ git co git-submodule  (this is your stuff)
> [vpl1 qemu]$ ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/
> -j24
> touch: cannot touch ‘.git-submodule-status’: Read-only file system
> make: Entering directory `/home/aik/pbuild/qemu-aikhostos2-ppc64'
>   GEN     ppc64-softmmu/config-devices.mak.tmp
> [...]
> 
> ./scripts/git-submodule.sh: line 74: .git-submodule-status: Read-only file
> system
> ./scripts/git-submodule.sh: failed to save git submodule status
> make: *** [git-submodule-update] Error 1

Oh I see, the git submodules list is empty because you have a previous
built source tree, so its shortcircuiting the extra check I added. Tihs
is easy enough to address


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 v2 1/4] build: allow setting a custom GIT binary for transparent proxying

[Alexey Kardashevskiy]

On 30/10/17 18:52, Daniel P. Berrange wrote:
> On Mon, Oct 30, 2017 at 10:49:01AM +1100, Alexey Kardashevskiy wrote:
>> On 30/10/17 03:29, Daniel P. Berrange wrote:
>>> On Mon, Oct 30, 2017 at 01:08:56AM +1100, Alexey Kardashevskiy wrote:
>>>>
>>>> I run ./scripts/git-submodule.sh on a server (where source directory is
>>>> writeable), them "rm .git-submodule-status", then run "Makefile" on a build
>>>> machine (which has the source directory mounted as read-only). I basically
>>>> recreate the situation where I was when I just discovered this brand new
>>>> ./scripts/git-submodule.sh.
>>>
>>> Don't rm the .git-submodule-status. That means 'make' thinks the modules
>>> are out of date.
>>>
>>> Just run 'scripts/git-submodules.sh ...mods..' on the writable source
>>> dir, and then run 'make' on the build machine.
>>>
>>>> I know that now, all I am asking is an error message to print exact command
>>>> to run...
>>>
>>> If you hadn't deleted the .git-submodule-status, it would have worked fine.
>>
>> No.
>>
>> I do this on a server:
>>
>> [vpl1 qemu]$ git co v2.10.0
>> At this point no .git-submodule-status is expected/required/exist.
>>
>> [vpl1 qemu]$ ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/
>> -j24
>>
>> Configure succeeds, compiles just fine, it has been working like this for
>> years.
>>
>>
>> Now:
>>
>> [vpl1 qemu]$ git co git-submodule  (this is your stuff)
>> [vpl1 qemu]$ ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/
>> -j24
>> touch: cannot touch ‘.git-submodule-status’: Read-only file system
>> make: Entering directory `/home/aik/pbuild/qemu-aikhostos2-ppc64'
>>   GEN     ppc64-softmmu/config-devices.mak.tmp
>> [...]
>>
>> ./scripts/git-submodule.sh: line 74: .git-submodule-status: Read-only file
>> system
>> ./scripts/git-submodule.sh: failed to save git submodule status
>> make: *** [git-submodule-update] Error 1
> 
> Oh I see, the git submodules list is empty because you have a previous
> built source tree, so its shortcircuiting the extra check I added. Tihs
> is easy enough to address


btw why is the name "git-submodule.sh", not update-submodule.sh or
update-git-submodule.sh on a par with update-linux-headers.sh?




-- 
Alexey