diff mbox series

[bitbake-devel,3/3] fetch2/git.py: add comment in try_premirrors

Message ID 20240201033100.3617421-3-Qi.Chen@windriver.com
State New
Headers show
Series [bitbake-devel,1/3] fetch2/git.py: fix a corner case in try_premirror | expand

Commit Message

ChenQi Feb. 1, 2024, 3:31 a.m. UTC 
From: Chen Qi <Qi.Chen@windriver.com>

The purpose of ensuring 'incremental fetch' is not easy to see from
the codes. So add comments to explain this.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 bitbake/lib/bb/fetch2/git.py | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Sam Liddicott Feb. 1, 2024, 11:15 a.m. UTC | #1 
This reminds me of a case while using a kirkstone branch, where a git
submodule was the same repo but a different branch as the parent module,
and the fetcher was not able to complete even without mirrors.

It was a go project where vendor modules were kept in a branch whose name
was vendor/ prefixed to the branch of the main project. This allowed the
module dependencies to be regulated (and not fetched during compile) while
still being associated with the project branch without tainting the project
branch or history - but it meant that the fetcher would hang.

Would these changes address that?

Sam

On Thu, 1 Feb 2024 at 03:31, Chen Qi via lists.openembedded.org <Qi.Chen=
windriver.com@lists.openembedded.org> wrote:

> From: Chen Qi <Qi.Chen@windriver.com>
>
> The purpose of ensuring 'incremental fetch' is not easy to see from
> the codes. So add comments to explain this.
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/git.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
> index 322a9366da..fa8c597ea2 100644
> --- a/bitbake/lib/bb/fetch2/git.py
> +++ b/bitbake/lib/bb/fetch2/git.py
> @@ -361,6 +361,11 @@ class Git(FetchMethod):
>          # we need to try premirrors first as using upstream is destined
> to fail.
>          if not trusted_network(d, ud.url):
>              return True
> +        # the following check is to ensure incremental fetch in
> downloads, this is
> +        # because the premirror might be old and does not contain the new
> rev required,
> +        # and this will cause a total removal and new clone. So if we can
> reach to
> +        # network, we prefer upstream over premirror, though the
> premirror might contain
> +        # the new rev.
>          if os.path.exists(ud.clonedir):
>              return False
>          return True
> --
> 2.42.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15818):
> https://lists.openembedded.org/g/bitbake-devel/message/15818
> Mute This Topic: https://lists.openembedded.org/mt/104090273/7497305
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> sam@liddicott.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
ChenQi Feb. 2, 2024, 4:24 a.m. UTC | #2 
On 2/1/24 19:15, Sam Liddicott wrote:
> This reminds me of a case while using a kirkstone branch, where a git 
> submodule was the same repo but a different branch as the parent 
> module, and the fetcher was not able to complete even without mirrors.

I think the problem you encountered is a different one. If you have a 
reliable way to reproduce the issue (try poky master first), then you 
can file a bug in yocto bugzilla.

P.S.

1. Some projects delete branches and I once encountered this. Did you 
manually check that all revs are still available in upstream for those 
submodules.

2. You can try using meta-virtualization/scripts/oe-go-mod-autogen.py 
when you work with go mod recipes, though your recipe's repo seems to 
handle go modules in a custom way and you may not need this.

Regards,

Qi

>
> It was a go project where vendor modules were kept in a branch whose 
> name was vendor/ prefixed to the branch of the main project. This 
> allowed the module dependencies to be regulated (and not fetched 
> during compile) while still being associated with the project branch 
> without tainting the project branch or history - but it meant that the 
> fetcher would hang.
>
> Would these changes address that?
>
> Sam
>
> On Thu, 1 Feb 2024 at 03:31, Chen Qi via lists.openembedded.org 
> <https://urldefense.com/v3/__http://lists.openembedded.org__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAe6eDotE$> 
> <Qi.Chen=windriver.com@lists.openembedded.org> wrote:
>
>     From: Chen Qi <Qi.Chen@windriver.com>
>
>     The purpose of ensuring 'incremental fetch' is not easy to see from
>     the codes. So add comments to explain this.
>
>     Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>     ---
>      bitbake/lib/bb/fetch2/git.py
>     <https://urldefense.com/v3/__http://git.py__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAPZ807pk$>
>     | 5 +++++
>      1 file changed, 5 insertions(+)
>
>     diff --git a/bitbake/lib/bb/fetch2/git.py
>     <https://urldefense.com/v3/__http://git.py__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAPZ807pk$>
>     b/bitbake/lib/bb/fetch2/git.py
>     <https://urldefense.com/v3/__http://git.py__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAPZ807pk$>
>     index 322a9366da..fa8c597ea2 100644
>     --- a/bitbake/lib/bb/fetch2/git.py
>     <https://urldefense.com/v3/__http://git.py__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAPZ807pk$>
>     +++ b/bitbake/lib/bb/fetch2/git.py
>     <https://urldefense.com/v3/__http://git.py__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAPZ807pk$>
>     @@ -361,6 +361,11 @@ class Git(FetchMethod):
>              # we need to try premirrors first as using upstream is
>     destined to fail.
>              if not trusted_network(d, ud.url):
>                  return True
>     +        # the following check is to ensure incremental fetch in
>     downloads, this is
>     +        # because the premirror might be old and does not contain
>     the new rev required,
>     +        # and this will cause a total removal and new clone. So
>     if we can reach to
>     +        # network, we prefer upstream over premirror, though the
>     premirror might contain
>     +        # the new rev.
>              if os.path.exists(ud.clonedir):
>                  return False
>              return True
>     -- 
>     2.42.0
>
>
>     -=-=-=-=-=-=-=-=-=-=-=-
>     Links: You receive all messages sent to this group.
>     View/Reply Online (#15818):
>     https://lists.openembedded.org/g/bitbake-devel/message/15818
>     <https://urldefense.com/v3/__https://lists.openembedded.org/g/bitbake-devel/message/15818__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcA8nSAEcE$>
>     Mute This Topic:
>     https://lists.openembedded.org/mt/104090273/7497305
>     <https://urldefense.com/v3/__https://lists.openembedded.org/mt/104090273/7497305__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAWxGg7kI$>
>     Group Owner: bitbake-devel+owner@lists.openembedded.org
>     <mailto:bitbake-devel%2Bowner@lists.openembedded.org>
>     Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub
>     <https://urldefense.com/v3/__https://lists.openembedded.org/g/bitbake-devel/unsub__;!!AjveYdw8EvQ!abkk4w1KDkQAv1NfDTy93YpSD4d1825gTUGc4LYGxrQWW5b6Y-UppkBFI7S3Mz0xK7iiZAcAMfW6SJU$>
>     [sam@liddicott.com]
>     -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 322a9366da..fa8c597ea2 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -361,6 +361,11 @@  class Git(FetchMethod):
         # we need to try premirrors first as using upstream is destined to fail.
         if not trusted_network(d, ud.url):
             return True
+        # the following check is to ensure incremental fetch in downloads, this is
+        # because the premirror might be old and does not contain the new rev required,
+        # and this will cause a total removal and new clone. So if we can reach to
+        # network, we prefer upstream over premirror, though the premirror might contain
+        # the new rev.
         if os.path.exists(ud.clonedir):
             return False
         return True