All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] configure: actually disable 'git_update' mode with --disable-git-update
       [not found] ` <20200716102213.2931209-1-ddstreet@canonical.com>
@ 2020-07-24  9:55   ` Michael Tokarev
  2020-07-29 19:58     ` [PATCH] " Dan Streetman
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Tokarev @ 2020-07-24  9:55 UTC (permalink / raw)
  To: Dan Streetman, qemu-trivial
  Cc: Rafael David Tinoco, Christian Ehrhardt, qemu-devel

Hi!

Dan, can you please resend this with To: qemu-devel@, maybe Cc: qemu-trivial@?
It isn't exactly trivial and it needs some review from the developers.

Thanks,

/mjt

16.07.2020 13:22, Dan Streetman wrote:
> The --disable-git-update configure param sets git_update=no, but
> some later checks only look for the .git dir. This changes the
> --enable-git-update to set git_update=yes but also fail if it
> does not find a .git dir. Then all the later checks for the .git
> dir can just be changed to a check for $git_update = "yes".
> 
> Also update the Makefile to skip the 'git_update' checks if it has
> been disabled.
> 
> This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
> also keep the source code in git, but do not want to enable the
> 'git_update' mode; with the current code, that's not possible even
> if the downstream package specifies --disable-git-update.
> 
> Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> ---
> Changes in v2:
>  - change GIT_UPDATE default to "" if no .git found
>  - change Makefile to skip git update checks if GIT_UPDATE=no
> 
>  Makefile  | 15 +++++++++------
>  configure | 21 +++++++++++++--------
>  2 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 32345c610e..9bcd3f9af4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -25,6 +25,8 @@ git-submodule-update:
>  
>  .PHONY: git-submodule-update
>  
> +# If --disable-git-update specified, skip these git checks
> +ifneq (no,$(GIT_UPDATE))
>  git_module_status := $(shell \
>    cd '$(SRC_PATH)' && \
>    GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
> @@ -32,7 +34,12 @@ git_module_status := $(shell \
>  )
>  
>  ifeq (1,$(git_module_status))
> -ifeq (no,$(GIT_UPDATE))
> +ifeq (yes,$(GIT_UPDATE))
> +git-submodule-update:
> +	$(call quiet-command, \
> +          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
> +          "GIT","$(GIT_SUBMODULES)")
> +else
>  git-submodule-update:
>  	$(call quiet-command, \
>              echo && \
> @@ -41,11 +48,7 @@ git-submodule-update:
>              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
>  
> diff --git a/configure b/configure
> index b751c853f5..eefda62210 100755
> --- a/configure
> +++ b/configure
> @@ -318,7 +318,7 @@ then
>      git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
>      git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
>  else
> -    git_update=no
> +    git_update=""
>      git_submodules=""
>  
>      if ! test -f "$source_path/ui/keycodemapdb/README"
> @@ -1603,7 +1603,12 @@ for opt do
>    ;;
>    --with-git=*) git="$optarg"
>    ;;
> -  --enable-git-update) git_update=yes
> +  --enable-git-update)
> +      git_update=yes
> +      if test ! -e "$source_path/.git"; then
> +          echo "ERROR: cannot --enable-git-update without .git"
> +          exit 1
> +      fi
>    ;;
>    --disable-git-update) git_update=no
>    ;;
> @@ -2017,7 +2022,7 @@ fi
>  # Consult white-list to determine whether to enable werror
>  # by default.  Only enable by default for git builds
>  if test -z "$werror" ; then
> -    if test -e "$source_path/.git" && \
> +    if test "$git_update" = "yes" && \
>          { test "$linux" = "yes" || test "$mingw32" = "yes"; }; then
>          werror="yes"
>      else
> @@ -4418,10 +4423,10 @@ EOF
>      fdt=system
>    else
>        # have GIT checkout, so activate dtc submodule
> -      if test -e "${source_path}/.git" ; then
> +      if test "$git_update" = "yes" ; then
>            git_submodules="${git_submodules} dtc"
>        fi
> -      if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
> +      if test -d "${source_path}/dtc/libfdt" || test "$git_update" = "yes" ; then
>            fdt=git
>            mkdir -p dtc
>            if [ "$pwd_is_source_path" != "y" ] ; then
> @@ -5391,7 +5396,7 @@ case "$capstone" in
>    "" | yes)
>      if $pkg_config capstone; then
>        capstone=system
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test "$git_update" = "yes" ; then
>        capstone=git
>      elif test -e "${source_path}/capstone/Makefile" ; then
>        capstone=internal
> @@ -6447,7 +6452,7 @@ case "$slirp" in
>    "" | yes)
>      if $pkg_config slirp; then
>        slirp=system
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test "$git_update" = "yes" ; then
>        slirp=git
>      elif test -e "${source_path}/slirp/Makefile" ; then
>        slirp=internal
> @@ -6809,7 +6814,7 @@ if test "$cpu" = "s390x" ; then
>      roms="$roms s390-ccw"
>      # SLOF is required for building the s390-ccw firmware on s390x,
>      # since it is using the libnet code from SLOF for network booting.
> -    if test -e "${source_path}/.git" ; then
> +    if test "$git_update" = "yes" ; then
>        git_submodules="${git_submodules} roms/SLOF"
>      fi
>    fi
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-07-24  9:55   ` [PATCH v2] configure: actually disable 'git_update' mode with --disable-git-update Michael Tokarev
@ 2020-07-29 19:58     ` Dan Streetman
  2020-09-13 18:57       ` [PATCH resend] " Dan Streetman
  2020-09-22 16:34       ` [PATCH] " Daniel P. Berrangé
  0 siblings, 2 replies; 10+ messages in thread
From: Dan Streetman @ 2020-07-29 19:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dan Streetman, qemu-trivial, Michael Tokarev,
	Rafael David Tinoco, Christian Ehrhardt

The --disable-git-update configure param sets git_update=no, but
some later checks only look for the .git dir. This changes the
--enable-git-update to set git_update=yes but also fail if it
does not find a .git dir. Then all the later checks for the .git
dir can just be changed to a check for $git_update = "yes".

Also update the Makefile to skip the 'git_update' checks if it has
been disabled.

This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
also keep the source code in git, but do not want to enable the
'git_update' mode; with the current code, that's not possible even
if the downstream package specifies --disable-git-update.

Signed-off-by: Dan Streetman <ddstreet@canonical.com>
---
Note this is a rebased resend of a previous email to qemu-trivial:
https://lists.nongnu.org/archive/html/qemu-trivial/2020-07/msg00180.html

 Makefile  | 15 +++++++++------
 configure | 21 +++++++++++++--------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index c2120d8d48..42550ae086 100644
--- a/Makefile
+++ b/Makefile
@@ -25,6 +25,8 @@ git-submodule-update:
 
 .PHONY: git-submodule-update
 
+# If --disable-git-update specified, skip these git checks
+ifneq (no,$(GIT_UPDATE))
 git_module_status := $(shell \
   cd '$(SRC_PATH)' && \
   GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
@@ -32,7 +34,12 @@ git_module_status := $(shell \
 )
 
 ifeq (1,$(git_module_status))
-ifeq (no,$(GIT_UPDATE))
+ifeq (yes,$(GIT_UPDATE))
+git-submodule-update:
+	$(call quiet-command, \
+          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
+          "GIT","$(GIT_SUBMODULES)")
+else
 git-submodule-update:
 	$(call quiet-command, \
             echo && \
@@ -41,11 +48,7 @@ git-submodule-update:
             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
 
diff --git a/configure b/configure
index 2acc4d1465..e7a241e971 100755
--- a/configure
+++ b/configure
@@ -318,7 +318,7 @@ then
     git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
     git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
 else
-    git_update=no
+    git_update=""
     git_submodules=""
 
     if ! test -f "$source_path/ui/keycodemapdb/README"
@@ -1598,7 +1598,12 @@ for opt do
   ;;
   --with-git=*) git="$optarg"
   ;;
-  --enable-git-update) git_update=yes
+  --enable-git-update)
+      git_update=yes
+      if test ! -e "$source_path/.git"; then
+          echo "ERROR: cannot --enable-git-update without .git"
+          exit 1
+      fi
   ;;
   --disable-git-update) git_update=no
   ;;
@@ -2011,7 +2016,7 @@ fi
 # Consult white-list to determine whether to enable werror
 # by default.  Only enable by default for git builds
 if test -z "$werror" ; then
-    if test -e "$source_path/.git" && \
+    if test "$git_update" = "yes" && \
         { test "$linux" = "yes" || test "$mingw32" = "yes"; }; then
         werror="yes"
     else
@@ -4412,10 +4417,10 @@ EOF
     fdt=system
   else
       # have GIT checkout, so activate dtc submodule
-      if test -e "${source_path}/.git" ; then
+      if test "$git_update" = "yes" ; then
           git_submodules="${git_submodules} dtc"
       fi
-      if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
+      if test -d "${source_path}/dtc/libfdt" || test "$git_update" = "yes" ; then
           fdt=git
           mkdir -p dtc
           if [ "$pwd_is_source_path" != "y" ] ; then
@@ -5385,7 +5390,7 @@ case "$capstone" in
   "" | yes)
     if $pkg_config capstone; then
       capstone=system
-    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+    elif test "$git_update" = "yes" ; then
       capstone=git
     elif test -e "${source_path}/capstone/Makefile" ; then
       capstone=internal
@@ -6414,7 +6419,7 @@ case "$slirp" in
   "" | yes)
     if $pkg_config slirp; then
       slirp=system
-    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+    elif test "$git_update" = "yes" ; then
       slirp=git
     elif test -e "${source_path}/slirp/Makefile" ; then
       slirp=internal
@@ -6776,7 +6781,7 @@ if test "$cpu" = "s390x" ; then
     roms="$roms s390-ccw"
     # SLOF is required for building the s390-ccw firmware on s390x,
     # since it is using the libnet code from SLOF for network booting.
-    if test -e "${source_path}/.git" ; then
+    if test "$git_update" = "yes" ; then
       git_submodules="${git_submodules} roms/SLOF"
     fi
   fi
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH resend] configure: actually disable 'git_update' mode with --disable-git-update
  2020-07-29 19:58     ` [PATCH] " Dan Streetman
@ 2020-09-13 18:57       ` Dan Streetman
  2020-09-13 21:09         ` Philippe Mathieu-Daudé
  2020-09-22 16:34       ` [PATCH] " Daniel P. Berrangé
  1 sibling, 1 reply; 10+ messages in thread
From: Dan Streetman @ 2020-09-13 18:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dan Streetman, qemu-trivial, Michael Tokarev,
	Rafael David Tinoco, Christian Ehrhardt

The --disable-git-update configure param sets git_update=no, but
some later checks only look for the .git dir. This changes the
--enable-git-update to set git_update=yes but also fail if it
does not find a .git dir. Then all the later checks for the .git
dir can just be changed to a check for $git_update = "yes".

Also update the Makefile to skip the 'git_update' checks if it has
been disabled.

This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
also keep the source code in git, but do not want to enable the
'git_update' mode; with the current code, that's not possible even
if the downstream package specifies --disable-git-update.

Signed-off-by: Dan Streetman <ddstreet@canonical.com>
---
Resend; this was sent twice before:
https://lists.nongnu.org/archive/html/qemu-trivial/2020-07/msg00180.html
https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg08243.html

Makefile  | 15 +++++++++------
 configure | 23 ++++++++++++++---------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 2ed19310cf7..712aaf8b53b 100644
--- a/Makefile
+++ b/Makefile
@@ -38,6 +38,8 @@ git-submodule-update:
 
 .PHONY: git-submodule-update
 
+# If --disable-git-update specified, skip these git checks
+ifneq (no,$(GIT_UPDATE))
 git_module_status := $(shell \
   cd '$(SRC_PATH)' && \
   GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
@@ -45,7 +47,12 @@ git_module_status := $(shell \
 )
 
 ifeq (1,$(git_module_status))
-ifeq (no,$(GIT_UPDATE))
+ifeq (yes,$(GIT_UPDATE))
+git-submodule-update:
+	$(call quiet-command, \
+          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
+          "GIT","$(GIT_SUBMODULES)")
+else
 git-submodule-update:
 	$(call quiet-command, \
             echo && \
@@ -54,11 +61,7 @@ git-submodule-update:
             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
 
diff --git a/configure b/configure
index 4231d56bcc0..2e0e2adc587 100755
--- a/configure
+++ b/configure
@@ -346,7 +346,7 @@ then
     git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
     git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
 else
-    git_update=no
+    git_update=""
     git_submodules=""
 
     if ! test -f "$source_path/ui/keycodemapdb/README"
@@ -1577,7 +1577,12 @@ for opt do
   ;;
   --with-git=*) git="$optarg"
   ;;
-  --enable-git-update) git_update=yes
+  --enable-git-update)
+      git_update=yes
+      if test ! -e "$source_path/.git"; then
+          echo "ERROR: cannot --enable-git-update without .git"
+          exit 1
+      fi
   ;;
   --disable-git-update) git_update=no
   ;;
@@ -1974,7 +1979,7 @@ python="$python -B"
 if test -z "$meson"; then
     if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.55.1; then
         meson=meson
-    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+    elif test $git_update = 'yes' ; then
         meson=git
     elif test -e "${source_path}/meson/meson.py" ; then
         meson=internal
@@ -2052,7 +2057,7 @@ fi
 # Consult white-list to determine whether to enable werror
 # by default.  Only enable by default for git builds
 if test -z "$werror" ; then
-    if test -e "$source_path/.git" && \
+    if test "$git_update" = "yes" && \
         { test "$linux" = "yes" || test "$mingw32" = "yes"; }; then
         werror="yes"
     else
@@ -4175,10 +4180,10 @@ EOF
     fdt=system
   else
       # have GIT checkout, so activate dtc submodule
-      if test -e "${source_path}/.git" ; then
+      if test "$git_update" = "yes" ; then
           git_submodules="${git_submodules} dtc"
       fi
-      if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
+      if test -d "${source_path}/dtc/libfdt" || test "$git_update" = "yes" ; then
           fdt=git
           mkdir -p dtc
           fdt_cflags="-I${source_path}/dtc/libfdt"
@@ -5126,7 +5131,7 @@ case "$capstone" in
   "" | yes)
     if $pkg_config capstone; then
       capstone=system
-    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+    elif test "$git_update" = "yes" ; then
       capstone=git
     elif test -e "${source_path}/capstone/Makefile" ; then
       capstone=internal
@@ -6126,7 +6131,7 @@ case "$slirp" in
   "" | yes)
     if $pkg_config slirp; then
       slirp=system
-    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+    elif test "$git_update" = "yes" ; then
       slirp=git
     elif test -e "${source_path}/slirp/Makefile" ; then
       slirp=internal
@@ -6460,7 +6465,7 @@ if test "$cpu" = "s390x" ; then
     roms="$roms s390-ccw"
     # SLOF is required for building the s390-ccw firmware on s390x,
     # since it is using the libnet code from SLOF for network booting.
-    if test -e "${source_path}/.git" ; then
+    if test "$git_update" = "yes" ; then
       git_submodules="${git_submodules} roms/SLOF"
     fi
   fi
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH resend] configure: actually disable 'git_update' mode with --disable-git-update
  2020-09-13 18:57       ` [PATCH resend] " Dan Streetman
@ 2020-09-13 21:09         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-13 21:09 UTC (permalink / raw)
  To: Dan Streetman, qemu-devel
  Cc: qemu-trivial, Daniel P . Berrange, Michael Tokarev,
	Rafael David Tinoco, Christian Ehrhardt

Cc'ing Daniel as this is related to 'GIT submodules' which
he maintains:

$ scripts/get_maintainer.pl -f scripts/git-submodule.sh
"Daniel P. Berrangé" <berrange@redhat.com> (odd fixer:GIT submodules)

On 9/13/20 8:57 PM, Dan Streetman wrote:
> The --disable-git-update configure param sets git_update=no, but
> some later checks only look for the .git dir. This changes the
> --enable-git-update to set git_update=yes but also fail if it
> does not find a .git dir. Then all the later checks for the .git
> dir can just be changed to a check for $git_update = "yes".
> 
> Also update the Makefile to skip the 'git_update' checks if it has
> been disabled.
> 
> This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
> also keep the source code in git, but do not want to enable the
> 'git_update' mode; with the current code, that's not possible even
> if the downstream package specifies --disable-git-update.
> 
> Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> ---
> Resend; this was sent twice before:
> https://lists.nongnu.org/archive/html/qemu-trivial/2020-07/msg00180.html

Which was v2,

> https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg08243.html

probably v3,

so this is v4.

Please don't post new patches as reply to previous versions,
because they'll likely get unnoticed (for developers who use
threaded view of their mail folder).

> 
> Makefile  | 15 +++++++++------
>  configure | 23 ++++++++++++++---------
>  2 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2ed19310cf7..712aaf8b53b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,6 +38,8 @@ git-submodule-update:
>  
>  .PHONY: git-submodule-update
>  
> +# If --disable-git-update specified, skip these git checks
> +ifneq (no,$(GIT_UPDATE))
>  git_module_status := $(shell \
>    cd '$(SRC_PATH)' && \
>    GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
> @@ -45,7 +47,12 @@ git_module_status := $(shell \
>  )
>  
>  ifeq (1,$(git_module_status))
> -ifeq (no,$(GIT_UPDATE))
> +ifeq (yes,$(GIT_UPDATE))
> +git-submodule-update:
> +	$(call quiet-command, \
> +          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
> +          "GIT","$(GIT_SUBMODULES)")
> +else
>  git-submodule-update:
>  	$(call quiet-command, \
>              echo && \
> @@ -54,11 +61,7 @@ git-submodule-update:
>              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
>  
> diff --git a/configure b/configure
> index 4231d56bcc0..2e0e2adc587 100755
> --- a/configure
> +++ b/configure
> @@ -346,7 +346,7 @@ then
>      git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
>      git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
>  else
> -    git_update=no
> +    git_update=""
>      git_submodules=""
>  
>      if ! test -f "$source_path/ui/keycodemapdb/README"
> @@ -1577,7 +1577,12 @@ for opt do
>    ;;
>    --with-git=*) git="$optarg"
>    ;;
> -  --enable-git-update) git_update=yes
> +  --enable-git-update)
> +      git_update=yes
> +      if test ! -e "$source_path/.git"; then
> +          echo "ERROR: cannot --enable-git-update without .git"
> +          exit 1
> +      fi
>    ;;
>    --disable-git-update) git_update=no
>    ;;
> @@ -1974,7 +1979,7 @@ python="$python -B"
>  if test -z "$meson"; then
>      if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.55.1; then
>          meson=meson
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test $git_update = 'yes' ; then
>          meson=git
>      elif test -e "${source_path}/meson/meson.py" ; then
>          meson=internal
> @@ -2052,7 +2057,7 @@ fi
>  # Consult white-list to determine whether to enable werror
>  # by default.  Only enable by default for git builds
>  if test -z "$werror" ; then
> -    if test -e "$source_path/.git" && \
> +    if test "$git_update" = "yes" && \
>          { test "$linux" = "yes" || test "$mingw32" = "yes"; }; then
>          werror="yes"
>      else
> @@ -4175,10 +4180,10 @@ EOF
>      fdt=system
>    else
>        # have GIT checkout, so activate dtc submodule
> -      if test -e "${source_path}/.git" ; then
> +      if test "$git_update" = "yes" ; then
>            git_submodules="${git_submodules} dtc"
>        fi
> -      if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
> +      if test -d "${source_path}/dtc/libfdt" || test "$git_update" = "yes" ; then
>            fdt=git
>            mkdir -p dtc
>            fdt_cflags="-I${source_path}/dtc/libfdt"
> @@ -5126,7 +5131,7 @@ case "$capstone" in
>    "" | yes)
>      if $pkg_config capstone; then
>        capstone=system
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test "$git_update" = "yes" ; then
>        capstone=git
>      elif test -e "${source_path}/capstone/Makefile" ; then
>        capstone=internal
> @@ -6126,7 +6131,7 @@ case "$slirp" in
>    "" | yes)
>      if $pkg_config slirp; then
>        slirp=system
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test "$git_update" = "yes" ; then
>        slirp=git
>      elif test -e "${source_path}/slirp/Makefile" ; then
>        slirp=internal
> @@ -6460,7 +6465,7 @@ if test "$cpu" = "s390x" ; then
>      roms="$roms s390-ccw"
>      # SLOF is required for building the s390-ccw firmware on s390x,
>      # since it is using the libnet code from SLOF for network booting.
> -    if test -e "${source_path}/.git" ; then
> +    if test "$git_update" = "yes" ; then
>        git_submodules="${git_submodules} roms/SLOF"
>      fi
>    fi
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-07-29 19:58     ` [PATCH] " Dan Streetman
  2020-09-13 18:57       ` [PATCH resend] " Dan Streetman
@ 2020-09-22 16:34       ` Daniel P. Berrangé
  2020-10-01  1:28         ` Dan Streetman
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-22 16:34 UTC (permalink / raw)
  To: Dan Streetman
  Cc: Michael Tokarev, qemu-devel, Christian Ehrhardt, Rafael David Tinoco

On Wed, Jul 29, 2020 at 03:58:29PM -0400, Dan Streetman wrote:
> The --disable-git-update configure param sets git_update=no, but
> some later checks only look for the .git dir. This changes the
> --enable-git-update to set git_update=yes but also fail if it
> does not find a .git dir. Then all the later checks for the .git
> dir can just be changed to a check for $git_update = "yes".
> 
> Also update the Makefile to skip the 'git_update' checks if it has
> been disabled.
> 
> This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
> also keep the source code in git, but do not want to enable the
> 'git_update' mode; with the current code, that's not possible even
> if the downstream package specifies --disable-git-update.

Lets recap the original intended behaviour

 1. User building from master qemu.git or a fork
     a) git_update=yes (default)
         - Always sync submodules to required commit

     b) git_update=no  (--disable-git-update)
         - Never sync submodules, user is responsible for sync
	 - Validate submodules are at correct commit and fail if not.

 2. User building from tarball
     - Never do anything git related at all


Your change is removing the validation from  1.b). This is not desirable
in general, because if a user has done a git pull and failed to sync the
submodules, they are liable to get obscure, hard to diagnose errors later
in the build process. This puts a burden on the user and maintainers who
have to waste time diagnosing such problems.



> Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> ---
> Note this is a rebased resend of a previous email to qemu-trivial:
> https://lists.nongnu.org/archive/html/qemu-trivial/2020-07/msg00180.html

NB, I'm removing qemu-trivial, because I don't think this patch
qualifies as trivial.


> 
>  Makefile  | 15 +++++++++------
>  configure | 21 +++++++++++++--------
>  2 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c2120d8d48..42550ae086 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -25,6 +25,8 @@ git-submodule-update:
>  
>  .PHONY: git-submodule-update
>  
> +# If --disable-git-update specified, skip these git checks
> +ifneq (no,$(GIT_UPDATE))
>  git_module_status := $(shell \
>    cd '$(SRC_PATH)' && \
>    GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
> @@ -32,7 +34,12 @@ git_module_status := $(shell \
>  )
>  
>  ifeq (1,$(git_module_status))
> -ifeq (no,$(GIT_UPDATE))
> +ifeq (yes,$(GIT_UPDATE))
> +git-submodule-update:
> +	$(call quiet-command, \
> +          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
> +          "GIT","$(GIT_SUBMODULES)")
> +else
>  git-submodule-update:
>  	$(call quiet-command, \
>              echo && \
> @@ -41,11 +48,7 @@ git-submodule-update:
>              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
>  
> diff --git a/configure b/configure
> index 2acc4d1465..e7a241e971 100755
> --- a/configure
> +++ b/configure
> @@ -318,7 +318,7 @@ then
>      git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
>      git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
>  else
> -    git_update=no
> +    git_update=""
>      git_submodules=""
>  
>      if ! test -f "$source_path/ui/keycodemapdb/README"
> @@ -1598,7 +1598,12 @@ for opt do
>    ;;
>    --with-git=*) git="$optarg"
>    ;;
> -  --enable-git-update) git_update=yes
> +  --enable-git-update)
> +      git_update=yes
> +      if test ! -e "$source_path/.git"; then
> +          echo "ERROR: cannot --enable-git-update without .git"
> +          exit 1
> +      fi
>    ;;
>    --disable-git-update) git_update=no
>    ;;
> @@ -2011,7 +2016,7 @@ fi
>  # Consult white-list to determine whether to enable werror
>  # by default.  Only enable by default for git builds
>  if test -z "$werror" ; then
> -    if test -e "$source_path/.git" && \
> +    if test "$git_update" = "yes" && \
>          { test "$linux" = "yes" || test "$mingw32" = "yes"; }; then
>          werror="yes"
>      else
> @@ -4412,10 +4417,10 @@ EOF
>      fdt=system
>    else
>        # have GIT checkout, so activate dtc submodule
> -      if test -e "${source_path}/.git" ; then
> +      if test "$git_update" = "yes" ; then
>            git_submodules="${git_submodules} dtc"
>        fi
> -      if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
> +      if test -d "${source_path}/dtc/libfdt" || test "$git_update" = "yes" ; then
>            fdt=git
>            mkdir -p dtc
>            if [ "$pwd_is_source_path" != "y" ] ; then
> @@ -5385,7 +5390,7 @@ case "$capstone" in
>    "" | yes)
>      if $pkg_config capstone; then
>        capstone=system
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test "$git_update" = "yes" ; then
>        capstone=git
>      elif test -e "${source_path}/capstone/Makefile" ; then
>        capstone=internal
> @@ -6414,7 +6419,7 @@ case "$slirp" in
>    "" | yes)
>      if $pkg_config slirp; then
>        slirp=system
> -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> +    elif test "$git_update" = "yes" ; then
>        slirp=git
>      elif test -e "${source_path}/slirp/Makefile" ; then
>        slirp=internal
> @@ -6776,7 +6781,7 @@ if test "$cpu" = "s390x" ; then
>      roms="$roms s390-ccw"
>      # SLOF is required for building the s390-ccw firmware on s390x,
>      # since it is using the libnet code from SLOF for network booting.
> -    if test -e "${source_path}/.git" ; then
> +    if test "$git_update" = "yes" ; then
>        git_submodules="${git_submodules} roms/SLOF"
>      fi
>    fi
> -- 
> 2.25.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 :|



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-09-22 16:34       ` [PATCH] " Daniel P. Berrangé
@ 2020-10-01  1:28         ` Dan Streetman
  2020-10-02 13:11           ` Daniel P. Berrangé
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Streetman @ 2020-10-01  1:28 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Michael Tokarev, qemu-devel, Christian Ehrhardt, Rafael David Tinoco

On Tue, Sep 22, 2020 at 12:34 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jul 29, 2020 at 03:58:29PM -0400, Dan Streetman wrote:
> > The --disable-git-update configure param sets git_update=no, but
> > some later checks only look for the .git dir. This changes the
> > --enable-git-update to set git_update=yes but also fail if it
> > does not find a .git dir. Then all the later checks for the .git
> > dir can just be changed to a check for $git_update = "yes".
> >
> > Also update the Makefile to skip the 'git_update' checks if it has
> > been disabled.
> >
> > This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
> > also keep the source code in git, but do not want to enable the
> > 'git_update' mode; with the current code, that's not possible even
> > if the downstream package specifies --disable-git-update.
>
> Lets recap the original intended behaviour
>
>  1. User building from master qemu.git or a fork
>      a) git_update=yes (default)
>          - Always sync submodules to required commit
>
>      b) git_update=no  (--disable-git-update)
>          - Never sync submodules, user is responsible for sync
>          - Validate submodules are at correct commit and fail if not.
>
>  2. User building from tarball
>      - Never do anything git related at all
>
>
> Your change is removing the validation from  1.b).

Would you accept adding a --disable-git-submodules-check so downstream
packagers can keep the source in git, but avoid the submodule
validation?
Or do you have another suggestion for handling this?

Michael, Christian, do you have any suggestions or preference for how
this should be handled in Debian and Ubuntu?

> This is not desirable
> in general, because if a user has done a git pull and failed to sync the
> submodules, they are liable to get obscure, hard to diagnose errors later
> in the build process. This puts a burden on the user and maintainers who
> have to waste time diagnosing such problems.

Yep, this problem causes the same obscure, hard to diagnose errors
later in the build with downstream packages, but only if the .git dir
is present, because the build process is subtly changed in that case.

>
>
>
> > Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> > ---
> > Note this is a rebased resend of a previous email to qemu-trivial:
> > https://lists.nongnu.org/archive/html/qemu-trivial/2020-07/msg00180.html
>
> NB, I'm removing qemu-trivial, because I don't think this patch
> qualifies as trivial.
>
>
> >
> >  Makefile  | 15 +++++++++------
> >  configure | 21 +++++++++++++--------
> >  2 files changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index c2120d8d48..42550ae086 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -25,6 +25,8 @@ git-submodule-update:
> >
> >  .PHONY: git-submodule-update
> >
> > +# If --disable-git-update specified, skip these git checks
> > +ifneq (no,$(GIT_UPDATE))
> >  git_module_status := $(shell \
> >    cd '$(SRC_PATH)' && \
> >    GIT="$(GIT)" ./scripts/git-submodule.sh status $(GIT_SUBMODULES); \
> > @@ -32,7 +34,12 @@ git_module_status := $(shell \
> >  )
> >
> >  ifeq (1,$(git_module_status))
> > -ifeq (no,$(GIT_UPDATE))
> > +ifeq (yes,$(GIT_UPDATE))
> > +git-submodule-update:
> > +     $(call quiet-command, \
> > +          (cd $(SRC_PATH) && GIT="$(GIT)" ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \
> > +          "GIT","$(GIT_SUBMODULES)")
> > +else
> >  git-submodule-update:
> >       $(call quiet-command, \
> >              echo && \
> > @@ -41,11 +48,7 @@ git-submodule-update:
> >              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
> >
> > diff --git a/configure b/configure
> > index 2acc4d1465..e7a241e971 100755
> > --- a/configure
> > +++ b/configure
> > @@ -318,7 +318,7 @@ then
> >      git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
> >      git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
> >  else
> > -    git_update=no
> > +    git_update=""
> >      git_submodules=""
> >
> >      if ! test -f "$source_path/ui/keycodemapdb/README"
> > @@ -1598,7 +1598,12 @@ for opt do
> >    ;;
> >    --with-git=*) git="$optarg"
> >    ;;
> > -  --enable-git-update) git_update=yes
> > +  --enable-git-update)
> > +      git_update=yes
> > +      if test ! -e "$source_path/.git"; then
> > +          echo "ERROR: cannot --enable-git-update without .git"
> > +          exit 1
> > +      fi
> >    ;;
> >    --disable-git-update) git_update=no
> >    ;;
> > @@ -2011,7 +2016,7 @@ fi
> >  # Consult white-list to determine whether to enable werror
> >  # by default.  Only enable by default for git builds
> >  if test -z "$werror" ; then
> > -    if test -e "$source_path/.git" && \
> > +    if test "$git_update" = "yes" && \
> >          { test "$linux" = "yes" || test "$mingw32" = "yes"; }; then
> >          werror="yes"
> >      else
> > @@ -4412,10 +4417,10 @@ EOF
> >      fdt=system
> >    else
> >        # have GIT checkout, so activate dtc submodule
> > -      if test -e "${source_path}/.git" ; then
> > +      if test "$git_update" = "yes" ; then
> >            git_submodules="${git_submodules} dtc"
> >        fi
> > -      if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then
> > +      if test -d "${source_path}/dtc/libfdt" || test "$git_update" = "yes" ; then
> >            fdt=git
> >            mkdir -p dtc
> >            if [ "$pwd_is_source_path" != "y" ] ; then
> > @@ -5385,7 +5390,7 @@ case "$capstone" in
> >    "" | yes)
> >      if $pkg_config capstone; then
> >        capstone=system
> > -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> > +    elif test "$git_update" = "yes" ; then
> >        capstone=git
> >      elif test -e "${source_path}/capstone/Makefile" ; then
> >        capstone=internal
> > @@ -6414,7 +6419,7 @@ case "$slirp" in
> >    "" | yes)
> >      if $pkg_config slirp; then
> >        slirp=system
> > -    elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> > +    elif test "$git_update" = "yes" ; then
> >        slirp=git
> >      elif test -e "${source_path}/slirp/Makefile" ; then
> >        slirp=internal
> > @@ -6776,7 +6781,7 @@ if test "$cpu" = "s390x" ; then
> >      roms="$roms s390-ccw"
> >      # SLOF is required for building the s390-ccw firmware on s390x,
> >      # since it is using the libnet code from SLOF for network booting.
> > -    if test -e "${source_path}/.git" ; then
> > +    if test "$git_update" = "yes" ; then
> >        git_submodules="${git_submodules} roms/SLOF"
> >      fi
> >    fi
> > --
> > 2.25.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 :|
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-10-01  1:28         ` Dan Streetman
@ 2020-10-02 13:11           ` Daniel P. Berrangé
  2020-10-02 14:25             ` Peter Maydell
  2020-10-16 20:51             ` Dan Streetman
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-10-02 13:11 UTC (permalink / raw)
  To: Dan Streetman
  Cc: Michael Tokarev, qemu-devel, Christian Ehrhardt, Rafael David Tinoco

On Wed, Sep 30, 2020 at 09:28:54PM -0400, Dan Streetman wrote:
> On Tue, Sep 22, 2020 at 12:34 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Jul 29, 2020 at 03:58:29PM -0400, Dan Streetman wrote:
> > > The --disable-git-update configure param sets git_update=no, but
> > > some later checks only look for the .git dir. This changes the
> > > --enable-git-update to set git_update=yes but also fail if it
> > > does not find a .git dir. Then all the later checks for the .git
> > > dir can just be changed to a check for $git_update = "yes".
> > >
> > > Also update the Makefile to skip the 'git_update' checks if it has
> > > been disabled.
> > >
> > > This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
> > > also keep the source code in git, but do not want to enable the
> > > 'git_update' mode; with the current code, that's not possible even
> > > if the downstream package specifies --disable-git-update.
> >
> > Lets recap the original intended behaviour
> >
> >  1. User building from master qemu.git or a fork
> >      a) git_update=yes (default)
> >          - Always sync submodules to required commit
> >
> >      b) git_update=no  (--disable-git-update)
> >          - Never sync submodules, user is responsible for sync
> >          - Validate submodules are at correct commit and fail if not.
> >
> >  2. User building from tarball
> >      - Never do anything git related at all
> >
> >
> > Your change is removing the validation from  1.b).
> 
> Would you accept adding a --disable-git-submodules-check so downstream
> packagers can keep the source in git, but avoid the submodule
> validation?

It feels like the current option shouldn't be a boolean, rather
a tri-state    --with-git-submodules=[update|validate|ignore]

> Or do you have another suggestion for handling this?

Assuming you're just using git for conveniently applying local
downstream patches, you don't need the git repo to exist once
getting to the build stage. IOW just delete the .git dir after
applying patches before running a build.


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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-10-02 13:11           ` Daniel P. Berrangé
@ 2020-10-02 14:25             ` Peter Maydell
  2020-10-02 14:44               ` Rafael David Tinoco
  2020-10-16 20:51             ` Dan Streetman
  1 sibling, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2020-10-02 14:25 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Dan Streetman, Michael Tokarev, QEMU Developers,
	Christian Ehrhardt, Rafael David Tinoco

On Fri, 2 Oct 2020 at 14:13, Daniel P. Berrangé <berrange@redhat.com> wrote:
> Assuming you're just using git for conveniently applying local
> downstream patches, you don't need the git repo to exist once
> getting to the build stage. IOW just delete the .git dir after
> applying patches before running a build.

...then what do you do if the build fails and you want to
edit/update the patch before retrying? "Blow away your .git
tree every time you build and reconstitute it somehow later"
doesn't seem like a very friendly thing to require...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-10-02 14:25             ` Peter Maydell
@ 2020-10-02 14:44               ` Rafael David Tinoco
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael David Tinoco @ 2020-10-02 14:44 UTC (permalink / raw)
  To: Peter Maydell, Daniel P. Berrangé
  Cc: Dan Streetman, Michael Tokarev, QEMU Developers, Christian Ehrhardt


>>  Assuming you're just using git for conveniently applying local
>>  downstream patches, you don't need the git repo to exist once
>>  getting to the build stage. IOW just delete the .git dir after
>>  applying patches before running a build.
>
>...then what do you do if the build fails and you want to
>edit/update the patch before retrying? "Blow away your .git
>tree every time you build and reconstitute it somehow later"
>doesn't seem like a very friendly thing to require...

+1. This option is disconnected with sustaining engineering reality 
IMHO: tons of interactive rebases, adding and dropping patches, 
re-orderings - so previous existing patches can allow the new ones (or 
even existing ones) to become clean cherry-picks - in between patch sets 
being worked on, bisections before continuing all this, etc.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] configure: actually disable 'git_update' mode with --disable-git-update
  2020-10-02 13:11           ` Daniel P. Berrangé
  2020-10-02 14:25             ` Peter Maydell
@ 2020-10-16 20:51             ` Dan Streetman
  1 sibling, 0 replies; 10+ messages in thread
From: Dan Streetman @ 2020-10-16 20:51 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Michael Tokarev, qemu-devel, Christian Ehrhardt, Rafael David Tinoco

On Fri, Oct 2, 2020 at 9:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Sep 30, 2020 at 09:28:54PM -0400, Dan Streetman wrote:
> > On Tue, Sep 22, 2020 at 12:34 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Wed, Jul 29, 2020 at 03:58:29PM -0400, Dan Streetman wrote:
> > > > The --disable-git-update configure param sets git_update=no, but
> > > > some later checks only look for the .git dir. This changes the
> > > > --enable-git-update to set git_update=yes but also fail if it
> > > > does not find a .git dir. Then all the later checks for the .git
> > > > dir can just be changed to a check for $git_update = "yes".
> > > >
> > > > Also update the Makefile to skip the 'git_update' checks if it has
> > > > been disabled.
> > > >
> > > > This is needed because downstream packagers, e.g. Debian, Ubuntu, etc,
> > > > also keep the source code in git, but do not want to enable the
> > > > 'git_update' mode; with the current code, that's not possible even
> > > > if the downstream package specifies --disable-git-update.
> > >
> > > Lets recap the original intended behaviour
> > >
> > >  1. User building from master qemu.git or a fork
> > >      a) git_update=yes (default)
> > >          - Always sync submodules to required commit
> > >
> > >      b) git_update=no  (--disable-git-update)
> > >          - Never sync submodules, user is responsible for sync
> > >          - Validate submodules are at correct commit and fail if not.
> > >
> > >  2. User building from tarball
> > >      - Never do anything git related at all
> > >
> > >
> > > Your change is removing the validation from  1.b).
> >
> > Would you accept adding a --disable-git-submodules-check so downstream
> > packagers can keep the source in git, but avoid the submodule
> > validation?
>
> It feels like the current option shouldn't be a boolean, rather
> a tri-state    --with-git-submodules=[update|validate|ignore]
>

Ok, I updated the patch to do that:
https://lists.nongnu.org/archive/html/qemu-devel/2020-10/msg04799.html

> > Or do you have another suggestion for handling this?
>
> Assuming you're just using git for conveniently applying local
> downstream patches, you don't need the git repo to exist once
> getting to the build stage. IOW just delete the .git dir after
> applying patches before running a build.
>
>
> 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 :|
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-16 20:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200715205013.2367760-1-ddstreet@canonical.com>
     [not found] ` <20200716102213.2931209-1-ddstreet@canonical.com>
2020-07-24  9:55   ` [PATCH v2] configure: actually disable 'git_update' mode with --disable-git-update Michael Tokarev
2020-07-29 19:58     ` [PATCH] " Dan Streetman
2020-09-13 18:57       ` [PATCH resend] " Dan Streetman
2020-09-13 21:09         ` Philippe Mathieu-Daudé
2020-09-22 16:34       ` [PATCH] " Daniel P. Berrangé
2020-10-01  1:28         ` Dan Streetman
2020-10-02 13:11           ` Daniel P. Berrangé
2020-10-02 14:25             ` Peter Maydell
2020-10-02 14:44               ` Rafael David Tinoco
2020-10-16 20:51             ` Dan Streetman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.