Re: [PATCH 1/5] archiver.bbclass: Handle gitsm URLs in the mirror archiver

[Paul Barker <pbarker@konsulko.com>]

On Tue, 10 Mar 2020 23:16:38 +0000
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> On Mon, 2020-03-09 at 14:21 +0000, Paul Barker wrote:
> > To fully archive a `gitsm://` entry in SRC_URI we need to also capture
> > the submodules recursively. If shallow mirror tarballs are found, they
> > must be temporarily extracted so that the submodules can be determined.
> > 
> > Signed-off-by: Paul Barker <pbarker@konsulko.com>
> > ---
> >  meta/classes/archiver.bbclass | 31 ++++++++++++++++++++++++++-----
> >  1 file changed, 26 insertions(+), 5 deletions(-)
> > 
> > diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
> > index 013195df7d..fef7ad4f62 100644
> > --- a/meta/classes/archiver.bbclass
> > +++ b/meta/classes/archiver.bbclass
> > @@ -306,7 +306,7 @@ python do_ar_configured() {
> >  }
> >  
> >  python do_ar_mirror() {
> > -    import subprocess
> > +    import shutil, subprocess, tempfile
> >  
> >      src_uri = (d.getVar('SRC_URI') or '').split()
> >      if len(src_uri) == 0:
> > @@ -337,12 +337,10 @@ python do_ar_mirror() {
> >  
> >      bb.utils.mkdirhier(destdir)
> >  
> > -    fetcher = bb.fetch2.Fetch(src_uri, d)
> > -
> > -    for url in fetcher.urls:
> > +    def archive_url(fetcher, url):
> >          if is_excluded(url):
> >              bb.note('Skipping excluded url: %s' % (url))
> > -            continue
> > +            return
> >  
> >          bb.note('Archiving url: %s' % (url))
> >          ud = fetcher.ud[url]
> > @@ -376,6 +374,29 @@ python do_ar_mirror() {
> >          bb.note('Copying source mirror')
> >          cmd = 'cp -fpPRH %s %s' % (localpath, destdir)
> >          subprocess.check_call(cmd, shell=True)
> > +
> > +        if url.startswith('gitsm://'):
> > +            def archive_submodule(ud, url, module, modpath, workdir, d):
> > +                url += ";bareclone=1;nobranch=1"
> > +                newfetch = bb.fetch2.Fetch([url], d, cache=False)
> > +
> > +                for url in newfetch.urls:
> > +                    archive_url(newfetch, url)
> > +
> > +            # If we're using a shallow mirror tarball it needs to be unpacked
> > +            # temporarily so that we can examine the .gitmodules file
> > +            if ud.shallow and os.path.exists(ud.fullshallow) and ud.method.need_update(ud, d):
> > +                tmpdir = tempfile.mkdtemp(dir=d.getVar("DL_DIR"))
> > +                subprocess.check_call("tar -xzf %s" % ud.fullshallow, cwd=tmpdir, shell=True)
> > +                ud.method.process_submodules(ud, tmpdir, archive_submodule, d)
> > +                shutil.rmtree(tmpdir)
> > +            else:
> > +                ud.method.process_submodules(ud, ud.clonedir, archive_submodule, d)
> > +
> > +    fetcher = bb.fetch2.Fetch(src_uri, d, cache=False)
> > +
> > +    for url in fetcher.urls:
> > +        archive_url(fetcher, url)
> >  }  
> 
> I can't help feeling that this is basically a sign the fetcher is
> broken.
> 
> What should really happen here is that there should be a method in the
> fetcher we call into.
> 
> Instead we're teaching code how to hack around the fetcher. Would it be
> possible to add some API we could call into here and maintain integrity
> of the fetcher API?

This is gitsm-specific so the process_submodules method is probably the
correct fetcher API. We need to call back into an archiver-supplied function
for each submodule that is found.

I guess process_submodules could do the temporary unpacking of the shallow
archive and then this code would be simplified. Is that what you had in mind?

-- 
Paul Barker
Konsulko Group