diff mbox series

[v1,04/14] archive-source.sh: Clone the submodules locally

Message ID 20190125140017.6092-5-alex.bennee@linaro.org
State New
Headers show
Series testing/next (binfmt_misc, vm-build and BSD CI) | expand

Commit Message

Alex Bennée Jan. 25, 2019, 2 p.m. UTC
From: Philippe Mathieu-Daudé <philmd@redhat.com>


We cloned the QEMU repository from the local storage. Since the
submodules are also available there, clone them too. This is
quicker and reduce network use.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 scripts/archive-source.sh | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

-- 
2.17.1

Comments

Daniel P. Berrangé Jan. 30, 2019, 3:11 p.m. UTC | #1
On Fri, Jan 25, 2019 at 02:00:07PM +0000, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>

> 

> We cloned the QEMU repository from the local storage. Since the

> submodules are also available there, clone them too. This is

> quicker and reduce network use.


FYI there's another attempt at solving it here:

  https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg07710.html

though that has some problems, so I prefer what you've done
here.  There's still scope for replacing the git ls-tree + tar
command with a git archive command, but that's tangential to
the problem of reducing network usage.

> 

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  scripts/archive-source.sh | 9 ++++-----

>  1 file changed, 4 insertions(+), 5 deletions(-)

> 

> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh

> index 6eed2a29bd..ce31be67b0 100755

> --- a/scripts/archive-source.sh

> +++ b/scripts/archive-source.sh

> @@ -38,6 +38,10 @@ else

>  fi

>  git clone --shared . "$vroot_dir"

>  test $? -ne 0 && error "failed to clone into '$vroot_dir'"

> +for sm in $submodules; do

> +    git clone --shared "$sm" "$vroot_dir/$sm"

> +    test $? -ne 0 && error "failed to clone submodule $sm"

> +done


I don't think this is reliable. Notice the message further up
where we define $submodules

  # We want a predictable list of submodules for builds, that is
  # independent of what the developer currently has initialized
  # in their checkout, because the build environment is completely
  # different to the host OS.


IOW, if the developers host build does not require 'dtc', then
the 'dtc' directory will be an empty directory, not a git
submodule checkout. At which point "git clone" has nothing
available.

Second, even if 'dtc' is checked out, we can't assume that
it is checked out at the right commit hash - 'git clone' will
match whatever is currently checked out.

So this needs to be wrapped

   if test -d "$sm/.git"
   then
       git clone --shared "$sm" "$vroot_dir/$sm"
       test $? -ne 0 && error "failed to clone submodule $sm"
   fi


>  

>  cd "$vroot_dir"

>  test $? -ne 0 && error "failed to change into '$vroot_dir'"

> @@ -45,11 +49,6 @@ test $? -ne 0 && error "failed to change into '$vroot_dir'"

>  git checkout $HEAD

>  test $? -ne 0 && error "failed to checkout $HEAD revision"

>  

> -for sm in $submodules; do

> -    git submodule update --init $sm

> -    test $? -ne 0 && error "failed to init submodule $sm"

> -done


This should not be deleted. It ensures that the checkout we
just cloned gets moved to the correct HEAD. It might still
use the network, but that is genuinely neccessary in some
cases. At least the network usage will be minimized if the
developers host already had an updated submodule set.

> -

>  if test -n "$submodules"; then

>      {

>          git ls-files || error "git ls-files failed"



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 :|
diff mbox series

Patch

diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 6eed2a29bd..ce31be67b0 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -38,6 +38,10 @@  else
 fi
 git clone --shared . "$vroot_dir"
 test $? -ne 0 && error "failed to clone into '$vroot_dir'"
+for sm in $submodules; do
+    git clone --shared "$sm" "$vroot_dir/$sm"
+    test $? -ne 0 && error "failed to clone submodule $sm"
+done
 
 cd "$vroot_dir"
 test $? -ne 0 && error "failed to change into '$vroot_dir'"
@@ -45,11 +49,6 @@  test $? -ne 0 && error "failed to change into '$vroot_dir'"
 git checkout $HEAD
 test $? -ne 0 && error "failed to checkout $HEAD revision"
 
-for sm in $submodules; do
-    git submodule update --init $sm
-    test $? -ne 0 && error "failed to init submodule $sm"
-done
-
 if test -n "$submodules"; then
     {
         git ls-files || error "git ls-files failed"