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

Richard Purdie
2020-03-11T11:38:45Z

On Wed, 2020-03-11 at 11:31 +0000, Paul Barker wrote:
> 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?

Nearly. The "operation" here is similar to "download" or "unpack" but
amounts to "make a mirror copy". Should the fetcher have such a method,
which would then have the fetcher implementation details in the
fetchers themselves?

Cheers,

Richard