* [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible @ 2021-10-21 17:49 Glenn Washburn 2021-10-21 19:07 ` Glenn Washburn 2021-10-22 16:44 ` Daniel Kiper 0 siblings, 2 replies; 6+ messages in thread From: Glenn Washburn @ 2021-10-21 17:49 UTC (permalink / raw) To: grub-devel, Daniel Kiper; +Cc: Glenn Washburn 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. [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 fi - git clone $shallow ${GNULIB_URL:-$default_gnulib_url} "$gnulib_path" \ - || cleanup_gnulib trap - 1 2 13 15 fi -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible 2021-10-21 17:49 [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible Glenn Washburn @ 2021-10-21 19:07 ` Glenn Washburn 2021-10-22 16:44 ` Daniel Kiper 1 sibling, 0 replies; 6+ messages in thread From: Glenn Washburn @ 2021-10-21 19:07 UTC (permalink / raw) To: grub-devel, Daniel Kiper On Thu, 21 Oct 2021 12:49:19 -0500 Glenn Washburn <development@efficientek.com> 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, FWIW, the patch I've upstreamed has removed references to GRUB. I don't think its matters here, because we'd have our own patch regardless and it doesn't need to be the same as upstream anyway. > 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. > > [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 Should be "know how deep to go". Daniel, if this is accepted as is, perhaps you could make this change. Glenn > + # 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 > fi > - git clone $shallow ${GNULIB_URL:-$default_gnulib_url} "$gnulib_path" \ > - || cleanup_gnulib > > trap - 1 2 13 15 > fi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible 2021-10-21 17:49 [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible Glenn Washburn 2021-10-21 19:07 ` Glenn Washburn @ 2021-10-22 16:44 ` Daniel Kiper 2021-10-22 21:26 ` Glenn Washburn 1 sibling, 1 reply; 6+ messages in thread From: Daniel Kiper @ 2021-10-22 16:44 UTC (permalink / raw) To: Glenn Washburn; +Cc: grub-devel 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. > > [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. Daniel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible 2021-10-22 16:44 ` Daniel Kiper @ 2021-10-22 21:26 ` Glenn Washburn 2021-10-25 21:35 ` Daniel Kiper 0 siblings, 1 reply; 6+ messages in thread From: Glenn Washburn @ 2021-10-22 21:26 UTC (permalink / raw) To: Daniel Kiper; +Cc: grub-devel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible 2021-10-22 21:26 ` Glenn Washburn @ 2021-10-25 21:35 ` Daniel Kiper 2022-01-08 2:12 ` Glenn Washburn 0 siblings, 1 reply; 6+ messages in thread From: Daniel Kiper @ 2021-10-25 21:35 UTC (permalink / raw) To: Glenn Washburn; +Cc: grub-devel, darren.kenny On Fri, Oct 22, 2021 at 04:26:41PM -0500, Glenn Washburn wrote: > 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 Oh, sorry, I missed this somehow. > 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 want to bump the gnulib version to the latest one. Though first of all we have to upstream a few gnulib issues discovered by the Coverity. Darren, CC-ed, is working on it. > 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. If gnulib upstream folks reject it then I can consider taking this patch into the GRUB. > I'm fine with waiting a few weeks and see what happens upstream. Sounds like a plan. Daniel > Glenn > > [1] https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00052.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible 2021-10-25 21:35 ` Daniel Kiper @ 2022-01-08 2:12 ` Glenn Washburn 0 siblings, 0 replies; 6+ messages in thread From: Glenn Washburn @ 2022-01-08 2:12 UTC (permalink / raw) To: Daniel Kiper; +Cc: grub-devel, darren.kenny On Mon, 25 Oct 2021 23:35:06 +0200 Daniel Kiper <daniel.kiper@oracle.com> wrote: > On Fri, Oct 22, 2021 at 04:26:41PM -0500, Glenn Washburn wrote: > > On Fri, 22 Oct 2021 18:44:15 +0200 > > Daniel Kiper <daniel.kiper@oracle.com> wrote: > > > > > 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 > > Oh, sorry, I missed this somehow. > > > 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 want to bump the gnulib version to the latest one. Though first of all > we have to upstream a few gnulib issues discovered by the Coverity. > Darren, CC-ed, is working on it. What's the current status of 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. > > If gnulib upstream folks reject it then I can consider taking this patch > into the GRUB. > > > I'm fine with waiting a few weeks and see what happens upstream. > > Sounds like a plan. > > Daniel > > > Glenn > > > > [1] https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00052.html Ok its finally been committed to the upstream repo after being missed these past few months. Once/if the Coverity issues are committed we can work on using the latest git revision. Glenn ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-08 2:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-10-21 17:49 [PATCH] bootstrap: When a commit hash is specified, do a shallow fetch if possible Glenn Washburn 2021-10-21 19:07 ` Glenn Washburn 2021-10-22 16:44 ` Daniel Kiper 2021-10-22 21:26 ` Glenn Washburn 2021-10-25 21:35 ` Daniel Kiper 2022-01-08 2:12 ` Glenn Washburn
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.