| Message ID | 20250521163311.2745308-1-stefan-koch@siemens.com |
|---|---|
| State | Accepted, archived |
| Commit | 37ed18e45aa17406162efc5ee3ddb2d6b33d07b9 |
| Headers | show |
| Series | [v2] fetch2: Ensure a valid symlink in `PREMIRRORS` case when using shallow cloning | expand |
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
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
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
On Thu, 2025-05-22 at 12:32 +0100, Richard Purdie wrote: > > 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) > > > > I'll send the patchset in version 3. That will contain a similar logic and a selftest case. As expected: When applying that patch to the current `master` without applying the other patches of the patchset it will fail. When applying all patches it will succeed. > > Cheers, > > > > Richard Regards -- Stefan Koch Siemens AG www.siemens.com -- Stefan Koch Siemens AG www.siemens.com
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
- 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(+)