autogen.sh: Restore --no-git (avoid git submodule update)
diff mbox series

Message ID 20200602154745.15054-1-ian.jackson@eu.citrix.com
State New
Headers show
Series
  • autogen.sh: Restore --no-git (avoid git submodule update)
Related show

Commit Message

Ian Jackson June 2, 2020, 3:47 p.m. UTC
Prior to 2621d48f005a "gnulib: delete all gnulib integration",
one could pass ./autogen.sh --no-git to prevent the libvirt build
system from running git submodule update.

This feature is needed by systems like the Xen Project CI which want
to explicitly control the revisions of every tree.  These will
typically arrange to initialise the submodules check out the right
version of everything, and then expect the build system not to mess
with it any more.

Despite to the old documentation comments referring only to gnulib,
the --no-git feature is required not only because of gnulib but also
because of the other submodule, src/keycodemapdb.

(And in any case, even if it were no longer required because all the
submodules were removed, it ought ideally to have been retained as a
no-op for compaibility reasons.)

So restore the --no-git feature.

Because of the way the argument parsing of autogen.sh works, it is
easiest to recognise this option only if it comes first.  This works
for the Xen Project CI, which has always passed this option first.

If something else is using this option (and hasn't introduced a
different workaround in the meantime), not in the first position,
then perhaps a more sophisticated approach will be needed.  But I
think this will do for now.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 autogen.sh | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Pavel Hrdina June 3, 2020, 10:31 a.m. UTC | #1
On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> one could pass ./autogen.sh --no-git to prevent the libvirt build
> system from running git submodule update.
> 
> This feature is needed by systems like the Xen Project CI which want
> to explicitly control the revisions of every tree.  These will
> typically arrange to initialise the submodules check out the right
> version of everything, and then expect the build system not to mess
> with it any more.

To be honest I don't understand why would anyone want to keep track of
all submodules of all projects for any CI and update it manually every
time the upstream project changes these submodules. Sounds like a lot
of unnecessary work but maybe I'm missing something.

> Despite to the old documentation comments referring only to gnulib,
> the --no-git feature is required not only because of gnulib but also
> because of the other submodule, src/keycodemapdb.
> 
> (And in any case, even if it were no longer required because all the
> submodules were removed, it ought ideally to have been retained as a
> no-op for compaibility reasons.)

Well, we will break a lot more by switching to Meson build system where
everyone building libvirt will have to change their scripts as it will
not be compatible at all.

Pavel

> So restore the --no-git feature.
> 
> Because of the way the argument parsing of autogen.sh works, it is
> easiest to recognise this option only if it comes first.  This works
> for the Xen Project CI, which has always passed this option first.
> 
> If something else is using this option (and hasn't introduced a
> different workaround in the meantime), not in the first position,
> then perhaps a more sophisticated approach will be needed.  But I
> think this will do for now.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  autogen.sh | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/autogen.sh b/autogen.sh
> index 4e1bbceb0a..1c98273452 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -1,5 +1,10 @@
>  #!/bin/sh
>  # Run this to generate all the initial makefiles, etc.
> +#
> +# THe following options must come first.  All other or subsequent
> +# arguments are passed to configure:
> +#   --no-git   skip `git submodule update --init`
> +
>  test -n "$srcdir" || srcdir=$(dirname "$0")
>  test -n "$srcdir" || srcdir=.
>  
> @@ -13,7 +18,11 @@ cd "$srcdir"
>      exit 1
>  }
>  
> -git submodule update --init || exit 1
> +if [ "x$1" = x--no-git ]; then
> +	shift
> +else
> +	git submodule update --init || exit 1
> +fi
>  
>  autoreconf --verbose --force --install || exit 1
>  
> -- 
> 2.11.0
>
Daniel P. Berrangé June 3, 2020, 10:37 a.m. UTC | #2
On Wed, Jun 03, 2020 at 12:31:09PM +0200, Pavel Hrdina wrote:
> On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> > Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> > one could pass ./autogen.sh --no-git to prevent the libvirt build
> > system from running git submodule update.
> > 
> > This feature is needed by systems like the Xen Project CI which want
> > to explicitly control the revisions of every tree.  These will
> > typically arrange to initialise the submodules check out the right
> > version of everything, and then expect the build system not to mess
> > with it any more.
> 
> To be honest I don't understand why would anyone want to keep track of
> all submodules of all projects for any CI and update it manually every
> time the upstream project changes these submodules. Sounds like a lot
> of unnecessary work but maybe I'm missing something.

Two possible scenarios that I think are valid

 - The CI job script does not have network access
 - The CI job script sees the source dir as read-only

In both cases the CI harness would have to initialize the submodule
before runing the CI job.

I don't know if this is what Xen CI is hitting, but I think the
request is reasonable none the less.

Both Jenkins and GitLab CI have option to make the harness init
submodules prior to the job running.

> > Despite to the old documentation comments referring only to gnulib,
> > the --no-git feature is required not only because of gnulib but also
> > because of the other submodule, src/keycodemapdb.
> > 
> > (And in any case, even if it were no longer required because all the
> > submodules were removed, it ought ideally to have been retained as a
> > no-op for compaibility reasons.)
> 
> Well, we will break a lot more by switching to Meson build system where
> everyone building libvirt will have to change their scripts as it will
> not be compatible at all.

Yes, but I think that's tangential, as the two above reasons will
still apply, and Meson will cope with having submodules pre-initialized.


Regards,
Daniel
Pavel Hrdina June 3, 2020, 11:34 a.m. UTC | #3
On Wed, Jun 03, 2020 at 11:37:08AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 03, 2020 at 12:31:09PM +0200, Pavel Hrdina wrote:
> > On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> > > Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> > > one could pass ./autogen.sh --no-git to prevent the libvirt build
> > > system from running git submodule update.
> > > 
> > > This feature is needed by systems like the Xen Project CI which want
> > > to explicitly control the revisions of every tree.  These will
> > > typically arrange to initialise the submodules check out the right
> > > version of everything, and then expect the build system not to mess
> > > with it any more.
> > 
> > To be honest I don't understand why would anyone want to keep track of
> > all submodules of all projects for any CI and update it manually every
> > time the upstream project changes these submodules. Sounds like a lot
> > of unnecessary work but maybe I'm missing something.
> 
> Two possible scenarios that I think are valid
> 
>  - The CI job script does not have network access
>  - The CI job script sees the source dir as read-only
> 
> In both cases the CI harness would have to initialize the submodule
> before runing the CI job.
> 
> I don't know if this is what Xen CI is hitting, but I think the
> request is reasonable none the less.
> 
> Both Jenkins and GitLab CI have option to make the harness init
> submodules prior to the job running.

My point was that running 'git submodule update --init' will not change
anything if all submodules are updated to the correct revision and if
the repository was pre-created with submodules and is read-only when the
test is executed the git command will not fail as well and everything
will be fine.

If the submodules are not updated to the correct revision then it will
fail and notify the CI users that something is broken.

There should not be any need to disable this explicitly unless you want
to build libvirt with different revisions of submodules.

> > > Despite to the old documentation comments referring only to gnulib,
> > > the --no-git feature is required not only because of gnulib but also
> > > because of the other submodule, src/keycodemapdb.
> > > 
> > > (And in any case, even if it were no longer required because all the
> > > submodules were removed, it ought ideally to have been retained as a
> > > no-op for compaibility reasons.)
> > 
> > Well, we will break a lot more by switching to Meson build system where
> > everyone building libvirt will have to change their scripts as it will
> > not be compatible at all.
> 
> Yes, but I think that's tangential, as the two above reasons will
> still apply, and Meson will cope with having submodules pre-initialized.

Yes, these are unrelated and I just wanted to point out that
compaibility reasons are in most cases not valid to changes to build
system or moving files around in git repository and so on.

Pavel
Ian Jackson June 3, 2020, 12:53 p.m. UTC | #4
Pavel Hrdina writes ("Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)"):
> There should not be any need to disable this explicitly unless you want
> to build libvirt with different revisions of submodules.

The Xen Project CI has a cross-tree bisector.  It is capable of
automatically selecting revisions, including revisions of submodules,
for testing.

It therefore needs to be able to build with different revisions of
submodules, indeed.

Thanks,
Ian.
Ian Jackson June 3, 2020, 1:28 p.m. UTC | #5
Pavel Hrdina writes ("Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)"):
> To be honest I don't understand why would anyone want to keep track of
> all submodules of all projects for any CI and update it manually every
> time the upstream project changes these submodules. Sounds like a lot
> of unnecessary work but maybe I'm missing something.

Maybe I should answer this.  The short answer is that this can be done
entirely automatically.

> Well, we will break a lot more by switching to Meson build system where
> everyone building libvirt will have to change their scripts as it will
> not be compatible at all.

When that occurs we'll have to change our build rune, of course.

Our CI system (which does bisection and stable tracking and so on)
will wants to build old versions, so it'll have to have both build
runes and look for something in-tree to tell the two methods apart.

Thanks,
Ian.
Pavel Hrdina June 3, 2020, 2:21 p.m. UTC | #6
On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> one could pass ./autogen.sh --no-git to prevent the libvirt build
> system from running git submodule update.
> 
> This feature is needed by systems like the Xen Project CI which want
> to explicitly control the revisions of every tree.  These will
> typically arrange to initialise the submodules check out the right
> version of everything, and then expect the build system not to mess
> with it any more.
> 
> Despite to the old documentation comments referring only to gnulib,
> the --no-git feature is required not only because of gnulib but also
> because of the other submodule, src/keycodemapdb.
> 
> (And in any case, even if it were no longer required because all the
> submodules were removed, it ought ideally to have been retained as a
> no-op for compaibility reasons.)
> 
> So restore the --no-git feature.
> 
> Because of the way the argument parsing of autogen.sh works, it is
> easiest to recognise this option only if it comes first.  This works
> for the Xen Project CI, which has always passed this option first.
> 
> If something else is using this option (and hasn't introduced a
> different workaround in the meantime), not in the first position,
> then perhaps a more sophisticated approach will be needed.  But I
> think this will do for now.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  autogen.sh | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/autogen.sh b/autogen.sh
> index 4e1bbceb0a..1c98273452 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -1,5 +1,10 @@
>  #!/bin/sh
>  # Run this to generate all the initial makefiles, etc.
> +#
> +# THe following options must come first.  All other or subsequent
> +# arguments are passed to configure:
> +#   --no-git   skip `git submodule update --init`
> +
>  test -n "$srcdir" || srcdir=$(dirname "$0")
>  test -n "$srcdir" || srcdir=.
>  
> @@ -13,7 +18,11 @@ cd "$srcdir"
>      exit 1
>  }
>  
> -git submodule update --init || exit 1
> +if [ "x$1" = x--no-git ]; then
> +	shift
> +else
> +	git submodule update --init || exit 1

I changed the TAB into spaces.

> +fi
>  
>  autoreconf --verbose --force --install || exit 1

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

and pushed.
Ian Jackson June 3, 2020, 3:41 p.m. UTC | #7
Pavel Hrdina writes ("Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)"):
> On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> > -git submodule update --init || exit 1
> > +if [ "x$1" = x--no-git ]; then
> > +	shift
> > +else
> > +	git submodule update --init || exit 1
> 
> I changed the TAB into spaces.

Ah, sorry for not following the right style.

> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
> 
> and pushed.

Thanks :-).

Ian.

Patch
diff mbox series

diff --git a/autogen.sh b/autogen.sh
index 4e1bbceb0a..1c98273452 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -1,5 +1,10 @@ 
 #!/bin/sh
 # Run this to generate all the initial makefiles, etc.
+#
+# THe following options must come first.  All other or subsequent
+# arguments are passed to configure:
+#   --no-git   skip `git submodule update --init`
+
 test -n "$srcdir" || srcdir=$(dirname "$0")
 test -n "$srcdir" || srcdir=.
 
@@ -13,7 +18,11 @@  cd "$srcdir"
     exit 1
 }
 
-git submodule update --init || exit 1
+if [ "x$1" = x--no-git ]; then
+	shift
+else
+	git submodule update --init || exit 1
+fi
 
 autoreconf --verbose --force --install || exit 1