diff mbox series

[v2] fetch2: Ensure a valid symlink in `PREMIRRORS` case when using shallow cloning

Message ID 20250521163311.2745308-1-stefan-koch@siemens.com
State Not Applicable
Headers show
Series [v2] fetch2: Ensure a valid symlink in `PREMIRRORS` case when using shallow cloning | expand

Commit Message

Koch, Stefan May 21, 2025, 4:33 p.m. UTC
- Since `ud.path` contains in that case the `PREMIRRORS` prefix path,
  this change ensures that a correct symlink is set up.

Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
---
 lib/bb/fetch2/__init__.py | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Richard Purdie May 22, 2025, 11:12 a.m. UTC | #1
On Wed, 2025-05-21 at 18:33 +0200, Stefan Koch wrote:
> - Since `ud.path` contains in that case the `PREMIRRORS` prefix path,
>   this change ensures that a correct symlink is set up.
> 
> Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
> ---
>  lib/bb/fetch2/__init__.py | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 2de4f4f8c..052505456 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -1117,7 +1117,12 @@ def try_mirror_url(fetch, origud, ud, ld, check = False):
>                      origud.method.build_mirror_data(origud, ld)
>              return origud.localpath
>          # Otherwise the result is a local file:// and we symlink to it
> +        # This could also be a link to a shallow archive
>          ensure_symlink(ud.localpath, origud.localpath)
> +        # When using shallow mode, add a symlink to the original fullshallow
> +        # path to ensure a valid symlink even in the `PREMIRRORS` case
> +        if getattr(ud, 'shallow', False) and not os.path.exists(origud.fullshallow):
> +            ensure_symlink(ud.localpath, origud.fullshallow)
>          update_stamp(origud, ld)
>          return ud.localpath

I guess the one problem here is that we're teaching a general function
in __init__ a fetcher specific issue :/.

Ideally we probably should have some fetcher specific function calls
for this to preserve the independence of the parent methods...

Cheers,

Richard
Koch, Stefan May 22, 2025, 11:24 a.m. UTC | #2
On Thu, 2025-05-22 at 12:12 +0100, Richard Purdie wrote:
> On Wed, 2025-05-21 at 18:33 +0200, Stefan Koch wrote:
> > - Since `ud.path` contains in that case the `PREMIRRORS` prefix
> > path,
> >   this change ensures that a correct symlink is set up.
> > 
> > Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
> > ---
> >  lib/bb/fetch2/__init__.py | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> > index 2de4f4f8c..052505456 100644
> > --- a/lib/bb/fetch2/__init__.py
> > +++ b/lib/bb/fetch2/__init__.py
> > @@ -1117,7 +1117,12 @@ def try_mirror_url(fetch, origud, ud, ld,
> > check = False):
> >                      origud.method.build_mirror_data(origud, ld)
> >              return origud.localpath
> >          # Otherwise the result is a local file:// and we symlink
> > to it
> > +        # This could also be a link to a shallow archive
> >          ensure_symlink(ud.localpath, origud.localpath)
> > +        # When using shallow mode, add a symlink to the original
> > fullshallow
> > +        # path to ensure a valid symlink even in the `PREMIRRORS`
> > case
> > +        if getattr(ud, 'shallow', False) and not
> > os.path.exists(origud.fullshallow):
> > +            ensure_symlink(ud.localpath, origud.fullshallow)
> >          update_stamp(origud, ld)
> >          return ud.localpath
> 
> I guess the one problem here is that we're teaching a general
> function
> in __init__ a fetcher specific issue :/.
> 
I thought so, too.

Anyway, I have decided for the symlink solution for two reasons:
- the PREMIRRORS logic is handled overall within __init__
- there is already one symlink created

> Ideally we probably should have some fetcher specific function calls
> for this to preserve the independence of the parent methods...
Another option I had thought about is to store the origud.path
somewhere and reuse it in the git fetcher. Then the archive could be
saved with the correct name instead of adding an additional symlink.

Question here: Is this free of another side-effects?
> 
> Cheers,
> 
> Richard

Regards

Stefan
Richard Purdie May 22, 2025, 11:32 a.m. UTC | #3
On Thu, 2025-05-22 at 11:24 +0000, Koch, Stefan wrote:
> On Thu, 2025-05-22 at 12:12 +0100, Richard Purdie wrote:
> > On Wed, 2025-05-21 at 18:33 +0200, Stefan Koch wrote:
> > > - Since `ud.path` contains in that case the `PREMIRRORS` prefix
> > > path,
> > >   this change ensures that a correct symlink is set up.
> > > 
> > > Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
> > > ---
> > >  lib/bb/fetch2/__init__.py | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/lib/bb/fetch2/__init__.py
> > > b/lib/bb/fetch2/__init__.py
> > > index 2de4f4f8c..052505456 100644
> > > --- a/lib/bb/fetch2/__init__.py
> > > +++ b/lib/bb/fetch2/__init__.py
> > > @@ -1117,7 +1117,12 @@ def try_mirror_url(fetch, origud, ud, ld,
> > > check = False):
> > >                      origud.method.build_mirror_data(origud, ld)
> > >              return origud.localpath
> > >          # Otherwise the result is a local file:// and we symlink
> > > to it
> > > +        # This could also be a link to a shallow archive
> > >          ensure_symlink(ud.localpath, origud.localpath)
> > > +        # When using shallow mode, add a symlink to the original
> > > fullshallow
> > > +        # path to ensure a valid symlink even in the
> > > `PREMIRRORS`
> > > case
> > > +        if getattr(ud, 'shallow', False) and not
> > > os.path.exists(origud.fullshallow):
> > > +            ensure_symlink(ud.localpath, origud.fullshallow)
> > >          update_stamp(origud, ld)
> > >          return ud.localpath
> > 
> > I guess the one problem here is that we're teaching a general
> > function
> > in __init__ a fetcher specific issue :/.
> > 
> I thought so, too.
> 
> Anyway, I have decided for the symlink solution for two reasons:
> - the PREMIRRORS logic is handled overall within __init__
> - there is already one symlink created
> 
> > Ideally we probably should have some fetcher specific function
> > calls
> > for this to preserve the independence of the parent methods...
> Another option I had thought about is to store the origud.path
> somewhere and reuse it in the git fetcher. Then the archive could be
> saved with the correct name instead of adding an additional symlink.
> 
> Question here: Is this free of another side-effects?
> 

The trouble is that doesn't nest, you can have mirrors of mirrors or
mirrors. That can be handled with a chain of symlinks.

I think the right solution here is a specific new method for fetchers
which is called after premirrors to put links in place.

The default would be:

ensure_symlink(ud.localpath, origud.localpath)

however the git fetcher would append the shallow logic.

Something like:

FetchMethod()
    def update_mirror_links(ud, origud):
        ensure_symlink(ud.localpath, origud.localpath)

Git(FetchMethod):
    def update_mirror_links(ud, origud):
        super().update_mirror_links(ud, origud)
        if ud,shallow and not os.path.exists(origud.fullshallow):
            ensure_symlink(ud.localpath, origud.fullshallow)


Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 2de4f4f8c..052505456 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1117,7 +1117,12 @@  def try_mirror_url(fetch, origud, ud, ld, check = False):
                     origud.method.build_mirror_data(origud, ld)
             return origud.localpath
         # Otherwise the result is a local file:// and we symlink to it
+        # This could also be a link to a shallow archive
         ensure_symlink(ud.localpath, origud.localpath)
+        # When using shallow mode, add a symlink to the original fullshallow
+        # path to ensure a valid symlink even in the `PREMIRRORS` case
+        if getattr(ud, 'shallow', False) and not os.path.exists(origud.fullshallow):
+            ensure_symlink(ud.localpath, origud.fullshallow)
         update_stamp(origud, ld)
         return ud.localpath