Re: [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible

[Glenn Washburn]

On Fri, 22 Oct 2021 18:44:15 +0200
Daniel Kiper <daniel.kiper@oracle.com> wrote:

> On Thu, Oct 21, 2021 at 12:49:19PM -0500, Glenn Washburn wrote:
> > The gnulib sources are large but more importantly have lots of changes. So
> > initial checkout of the repository can take a long time when network or
> > cpu resources are limited. The later is especially acute in a non-KVM QEMU
> > virtual machine (which can take 40+ minutes compared to <30 seconds with
> > this change[1]). The problem is specific to how GRUB uses gnulib, which is
> > by pegging the desired checkout to a specific git revision. In this case,
> > git can not do a shallow clone by using the --depth option because git does
> > not know ahead of time how deep the revision is from the tip. So git must
> > clone the whole repository.
> >
> > However, there is an alternate method that requires support from the git
> > server[2], namely by asking for a specific commit on fetch. Refactor to use
> > fetch and fallback to fetching the entire repository if fetching by commit
> > hash fails.
> >
> > Currently the git server hosting the official gnulib git repository does not
> > support fetch by commit hash[3]. However, there are mirrors which do support
> > this[4], and can be specified by setting the $GNULIB_URL.

This paragraph is now out of date, the git server configuration change
happened much more quickly than I expected. So before committing it
should be removed along with the last two references.

> >
> > [1] https://savannah.nongnu.org/support/index.php?110553#comment1
> > [2] https://stackoverflow.com/a/3489576/2108011
> > [3] https://savannah.nongnu.org/support/index.php?110553
> > [4] https://github.com/coreutils/gnulib
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  bootstrap | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/bootstrap b/bootstrap
> > index 5b08e7e2d..914f911f8 100755
> > --- a/bootstrap
> > +++ b/bootstrap
> > @@ -665,9 +665,21 @@ if $use_gnulib; then
> >        shallow=
> >        if test -z "$GNULIB_REVISION"; then
> >          git clone -h 2>&1 | grep -- --depth > /dev/null && 
> > shallow='--depth 2'
> > +        git clone $shallow ${GNULIB_URL:-$default_gnulib_url} 
> > "$gnulib_path" \
> > +          || cleanup_gnulib
> > +      else
> > +        git fetch -h 2>&1 | grep -- --depth > /dev/null && 
> > shallow='--depth 2'
> > +        mkdir -p "$gnulib_path"
> > +        git -C "$gnulib_path" init
> > +        git -C "$gnulib_path" remote add origin 
> > ${GNULIB_URL:-$default_gnulib_url}
> > +        # Can not do a shallow fetch if fetch by commit hash fails because 
> > we
> > +        # do not know how the to go to get to $GNULIB_REVISION, so we must 
> > get
> > +        # all commits.
> > +        git -C "$gnulib_path" fetch $shallow origin "$GNULIB_REVISION" \
> > +          || git -C "$gnulib_path" fetch origin \
> > +          || cleanup_gnulib
> > +        git -C "$gnulib_path" reset --hard FETCH_HEAD
> 
> Is this hunk from gnulib upstream? If not I would prefer if you upstream
> it into gnulib. We should avoid custom patches for imported code as much
> as possible.

I agree in general. As mentioned in a follow up reply to this patch, I
did submit the patch upstream[1]. The only difference is the commit
message. It has not been responded to yet. However, even once accepted
upstream we'll have to cherry-pick it to the GRUB repo. Are you
thinking we'd need to get all changes to bootstrap at that time? It
doesn't look like there's anything that would be that useful (except
maybe using the https repo url). I'm curious how you'd like to handle
this.

I don't think the issue of a custom patch in this instance is a problem
though. Bootstrap very rarely changes, even in upstream (<10 changes in
the last 2 years). So there I'm not concerned about having to maintain
a lot of custom patches on top of it (like the situation of many
distros vis-a-vis GRUB). So I think if its not accepted upstream (I
don't see why it wouldn't be), I think we should use in GRUB because it
provides significant reduction in bandwith and a drastic reduction in
time to checkout on a qemu VM (but also significant not on a VM, 5min
vs 1min) depending on the CPU resources available. This is especially
useful for a CI/testing system that is frequently run.

I'm fine with waiting a few weeks and see what happens upstream.

Glenn

[1] https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00052.html