[PATCH 0/5] Archiver and externalsrc fixes

[PATCH 0/5] Archiver and externalsrc fixes

[Paul Barker]

These changes fix the archival of git submodules, do_deploy_archives
dependencies and externalsrc support for the kernel.

The gitsm archiver fix is independent of the patch sent to bitbake - the
patch here fixes the generation of archives for git submodules, the separate
bitbake patch fixes the use of gitsm shallow mirror tarballs whether
generated by the archiver or just by copying them from DL_DIR.

I know we're in feature freeze but all of these patches fix specific errors
which exist on both zeus and master. I recommend backporting all of these to
zeus once committed to master.

Paul Barker (5):
  archiver.bbclass: Handle gitsm URLs in the mirror archiver
  archiver.bbclass: Make do_deploy_archives a recursive dependency
  kernelsrc.bbclass: Fix externalsrc support
  perf: Fix externalsrc support
  kernel-yocto.bbclass: Support config fragments with externalsrc

 meta/classes/archiver.bbclass     | 35 +++++++++++++++++++++++++------
 meta/classes/kernel-yocto.bbclass |  3 ++-
 meta/classes/kernelsrc.bbclass    |  2 +-
 meta/recipes-kernel/perf/perf.bb  |  2 +-
 4 files changed, 33 insertions(+), 9 deletions(-)

-- 
2.20.1

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

[Paul Barker]

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)
 }
 
 def exclude_useless_paths(tarinfo):
-- 
2.20.1

[PATCH 2/5] archiver.bbclass: Make do_deploy_archives a recursive dependency

[Paul Barker]

To ensure that archives are captured for all dependencies of a typical
bitbake build we add do_deploy_archives to the list of recursive
dependencies of do_build. Without this, archives may be missed for
recipes such as gcc-source which do not create packages or populate a
sysroot.

do_deploy_archives is also added to the recursive dependencies of
do_populate_sdk so that all sources required for an SDK can be captured.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 meta/classes/archiver.bbclass | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
index fef7ad4f62..c11d36d708 100644
--- a/meta/classes/archiver.bbclass
+++ b/meta/classes/archiver.bbclass
@@ -604,7 +604,9 @@ addtask do_ar_configured after do_unpack_and_patch
 addtask do_ar_mirror after do_fetch
 addtask do_dumpdata
 addtask do_ar_recipe
-addtask do_deploy_archives before do_build
+addtask do_deploy_archives
+do_build[recrdeptask] += "do_deploy_archives"
+do_populate_sdk[recrdeptask] += "do_deploy_archives"
 
 python () {
     # Add tasks in the correct order, specifically for linux-yocto to avoid race condition.
-- 
2.20.1

[PATCH 3/5] kernelsrc.bbclass: Fix externalsrc support

[Paul Barker]

When the externalsrc class is used the tasks listed in
SRCTREECOVEREDTASKS are deleted to prevent them being executed. If
externalsrc is used for the kernel then this will include
virtual/kernel:do_patch.

We can depend on do_shared_workdir instead as this will survive when
externalsrc is used.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 meta/classes/kernelsrc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernelsrc.bbclass b/meta/classes/kernelsrc.bbclass
index 675d40ec9a..a951ba3325 100644
--- a/meta/classes/kernelsrc.bbclass
+++ b/meta/classes/kernelsrc.bbclass
@@ -1,7 +1,7 @@
 S = "${STAGING_KERNEL_DIR}"
 deltask do_fetch
 deltask do_unpack
-do_patch[depends] += "virtual/kernel:do_patch"
+do_patch[depends] += "virtual/kernel:do_shared_workdir"
 do_patch[noexec] = "1"
 do_package[depends] += "virtual/kernel:do_populate_sysroot"
 KERNEL_VERSION = "${@get_kernelversion_file("${STAGING_KERNEL_BUILDDIR}")}"
-- 
2.20.1

[PATCH 4/5] perf: Fix externalsrc support

[Paul Barker]

When the externalsrc class is used the tasks listed in
SRCTREECOVEREDTASKS are deleted to prevent them being executed. If
externalsrc is used for the kernel then this will include
virtual/kernel:do_patch.

We can depend on do_shared_workdir instead as this will survive when
externalsrc is used.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 meta/recipes-kernel/perf/perf.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb
index 10a5b88260..e005eb082b 100644
--- a/meta/recipes-kernel/perf/perf.bb
+++ b/meta/recipes-kernel/perf/perf.bb
@@ -52,7 +52,7 @@ export PYTHON_SITEPACKAGES_DIR
 #kernel 3.1+ supports WERROR to disable warnings as errors
 export WERROR = "0"
 
-do_populate_lic[depends] += "virtual/kernel:do_patch"
+do_populate_lic[depends] += "virtual/kernel:do_shared_workdir"
 
 # needed for building the tools/perf Perl binding
 include ${@bb.utils.contains('PACKAGECONFIG', 'scripting', 'perf-perl.inc', '', d)}
-- 
2.20.1

[PATCH 5/5] kernel-yocto.bbclass: Support config fragments with externalsrc

[Paul Barker]

The merging of config fragments is performend in the do_kernel_configme
task and so config fragments will not be supported when this task is
removed from the dependency tree.

kernel-yocto adds additional tasks which may modify the source directory
to SRCTREECOVEREDTASKS so that they are removed when using externalsrc.
However, do_kernel_configme should be safe to use, the only modification
to the source tree is the potential creation of the '.kernel-meta'
directory and the '.metadir' file.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 meta/classes/kernel-yocto.bbclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
index d71ce212a8..6792c9a233 100644
--- a/meta/classes/kernel-yocto.bbclass
+++ b/meta/classes/kernel-yocto.bbclass
@@ -1,5 +1,5 @@
 # remove tasks that modify the source tree in case externalsrc is inherited
-SRCTREECOVEREDTASKS += "do_kernel_configme do_validate_branches do_kernel_configcheck do_kernel_checkout do_fetch do_unpack do_patch"
+SRCTREECOVEREDTASKS += "do_validate_branches do_kernel_configcheck do_kernel_checkout do_fetch do_unpack do_patch"
 PATCH_GIT_USER_EMAIL ?= "kernel-yocto@oe"
 PATCH_GIT_USER_NAME ?= "OpenEmbedded"
 
@@ -325,6 +325,7 @@ do_validate_branches[depends] = "kern-tools-native:do_populate_sysroot"
 do_kernel_configme[depends] += "virtual/${TARGET_PREFIX}binutils:do_populate_sysroot"
 do_kernel_configme[depends] += "virtual/${TARGET_PREFIX}gcc:do_populate_sysroot"
 do_kernel_configme[depends] += "bc-native:do_populate_sysroot bison-native:do_populate_sysroot"
+do_kernel_configme[depends] += "kern-tools-native:do_populate_sysroot"
 do_kernel_configme[dirs] += "${S} ${B}"
 do_kernel_configme() {
 	# translate the kconfig_mode into something that merge_config.sh
-- 
2.20.1

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

[Richard Purdie]

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?

Cheers,

Richard

Re: [PATCH 2/5] archiver.bbclass: Make do_deploy_archives a recursive dependency

[Richard Purdie]

On Mon, 2020-03-09 at 14:21 +0000, Paul Barker wrote:
> To ensure that archives are captured for all dependencies of a typical
> bitbake build we add do_deploy_archives to the list of recursive
> dependencies of do_build. Without this, archives may be missed for
> recipes such as gcc-source which do not create packages or populate a
> sysroot.
> 
> do_deploy_archives is also added to the recursive dependencies of
> do_populate_sdk so that all sources required for an SDK can be captured.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>
> ---
>  meta/classes/archiver.bbclass | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
> index fef7ad4f62..c11d36d708 100644
> --- a/meta/classes/archiver.bbclass
> +++ b/meta/classes/archiver.bbclass
> @@ -604,7 +604,9 @@ addtask do_ar_configured after do_unpack_and_patch
>  addtask do_ar_mirror after do_fetch
>  addtask do_dumpdata
>  addtask do_ar_recipe
> -addtask do_deploy_archives before do_build
> +addtask do_deploy_archives
> +do_build[recrdeptask] += "do_deploy_archives"
> +do_populate_sdk[recrdeptask] += "do_deploy_archives"

We implemented the --runall option to bitbake to try and avoid having
recrdeptask versions of most tasks. Does that not work here? It should
also work for the SDK I think?

Cheers,

Richard

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

[Paul Barker]

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

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

[Richard Purdie]

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

Re: [PATCH 2/5] archiver.bbclass: Make do_deploy_archives a recursive dependency

[Paul Barker]

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

> On Mon, 2020-03-09 at 14:21 +0000, Paul Barker wrote:
> > To ensure that archives are captured for all dependencies of a typical
> > bitbake build we add do_deploy_archives to the list of recursive
> > dependencies of do_build. Without this, archives may be missed for
> > recipes such as gcc-source which do not create packages or populate a
> > sysroot.
> > 
> > do_deploy_archives is also added to the recursive dependencies of
> > do_populate_sdk so that all sources required for an SDK can be captured.
> > 
> > Signed-off-by: Paul Barker <pbarker@konsulko.com>
> > ---
> >  meta/classes/archiver.bbclass | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
> > index fef7ad4f62..c11d36d708 100644
> > --- a/meta/classes/archiver.bbclass
> > +++ b/meta/classes/archiver.bbclass
> > @@ -604,7 +604,9 @@ addtask do_ar_configured after do_unpack_and_patch
> >  addtask do_ar_mirror after do_fetch
> >  addtask do_dumpdata
> >  addtask do_ar_recipe
> > -addtask do_deploy_archives before do_build
> > +addtask do_deploy_archives
> > +do_build[recrdeptask] += "do_deploy_archives"
> > +do_populate_sdk[recrdeptask] += "do_deploy_archives"  
> 
> We implemented the --runall option to bitbake to try and avoid having
> recrdeptask versions of most tasks. Does that not work here? It should
> also work for the SDK I think?

If the archiver is enabled, its tasks should be in the dependency tree of
whatever you're building so that you don't need to invoke bitbake twice to
produce the required artifacts. For images that's the way the archiver has
always worked, if it's enabled then you just need to do `bitbake image` to
build the image and deploy the source archives. This change just extends that
behaviour to cover other things we can build and ensures that we don't miss
sources for recipes like gcc-source.

-- 
Paul Barker
Konsulko Group

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

[Paul Barker]

On Wed, 11 Mar 2020 11:38:44 +0000
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> 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?

I structured things this way after the discussions we've had previously about
not wanting to add too many new code paths to the fetcher. I'd also like to
keep the logic in a bbclass as much as possible so that it can be more easily
carried as a local backport to earlier Yocto Project releases.

I do see your point though, this is liable to grow warts over time as special
cases are added for different fetchers.

The cause of the warts here is that the gitsm fetcher downloads and creates
mirror tarballs for sources which aren't listed in SRC_URI. The archiver
would be simpler if we could assume that all sources are included in SRC_URI.
Perhaps the solution is not to add a "make a mirror copy" API but instead add
an "expand SRC_URI with any dependencies" API that the archiver can call
before it iterates over the list of sources.

-- 
Paul Barker
Konsulko Group

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

[Richard Purdie]

On Wed, 2020-03-11 at 11:50 +0000, Paul Barker wrote:
> I structured things this way after the discussions we've had previously about
> not wanting to add too many new code paths to the fetcher. I'd also like to
> keep the logic in a bbclass as much as possible so that it can be more easily
> carried as a local backport to earlier Yocto Project releases.
> 
> I do see your point though, this is liable to grow warts over time as special
> cases are added for different fetchers.

I appreciate the previous discussions, but yes, this is my concern.

> The cause of the warts here is that the gitsm fetcher downloads and creates
> mirror tarballs for sources which aren't listed in SRC_URI. The archiver
> would be simpler if we could assume that all sources are included in SRC_URI.
> Perhaps the solution is not to add a "make a mirror copy" API but instead add
> an "expand SRC_URI with any dependencies" API that the archiver can call
> before it iterates over the list of sources.

Yes, that sounds like good insight into the real issue and would make
sense, I think that would improve things and alleviate my concerns.

Cheers,

Richard

Re: [OE-core] [PATCH 2/5] archiver.bbclass: Make do_deploy_archives a recursive dependency

[Mark Hatle]

(I know this is an old thread, but I ran into this as well..)

...

On 3/11/20 6:40 AM, Paul Barker wrote:
> On Tue, 10 Mar 2020 23:18:33 +0000
> Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> 
>> On Mon, 2020-03-09 at 14:21 +0000, Paul Barker wrote:
>>> To ensure that archives are captured for all dependencies of a typical
>>> bitbake build we add do_deploy_archives to the list of recursive
>>> dependencies of do_build. Without this, archives may be missed for
>>> recipes such as gcc-source which do not create packages or populate a
>>> sysroot.
>>>
>>> do_deploy_archives is also added to the recursive dependencies of
>>> do_populate_sdk so that all sources required for an SDK can be captured.
>>>
>>> Signed-off-by: Paul Barker <pbarker@konsulko.com>
>>> ---
>>>  meta/classes/archiver.bbclass | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
>>> index fef7ad4f62..c11d36d708 100644
>>> --- a/meta/classes/archiver.bbclass
>>> +++ b/meta/classes/archiver.bbclass
>>> @@ -604,7 +604,9 @@ addtask do_ar_configured after do_unpack_and_patch
>>>  addtask do_ar_mirror after do_fetch
>>>  addtask do_dumpdata
>>>  addtask do_ar_recipe
>>> -addtask do_deploy_archives before do_build
>>> +addtask do_deploy_archives
>>> +do_build[recrdeptask] += "do_deploy_archives"
>>> +do_populate_sdk[recrdeptask] += "do_deploy_archives"  
>>
>> We implemented the --runall option to bitbake to try and avoid having
>> recrdeptask versions of most tasks. Does that not work here? It should
>> also work for the SDK I think?

You can't use a --runall operation on a task target, such as:

bitbake core-image-minimal -c populate_sdk

I can't find any way to get all of the archiver sources for the populate_sdk
target.  The above should resolve this, and as Paul said below it will ensure
that populate_sdk follows the same behavior as regular image generation as well.

I'm wondering if do_populate_sdk_ext needs it as well.

> If the archiver is enabled, its tasks should be in the dependency tree of
> whatever you're building so that you don't need to invoke bitbake twice to
> produce the required artifacts. For images that's the way the archiver has
> always worked, if it's enabled then you just need to do `bitbake image` to
> build the image and deploy the source archives. This change just extends that
> behaviour to cover other things we can build and ensures that we don't miss
> sources for recipes like gcc-source.
> 

Re: [OE-core] [PATCH 2/5] archiver.bbclass: Make do_deploy_archives a recursive dependency

[Mark Hatle]

On 4/1/20 9:49 AM, Mark Hatle wrote:
> (I know this is an old thread, but I ran into this as well..)
> 
> ...
> 
> On 3/11/20 6:40 AM, Paul Barker wrote:
>> On Tue, 10 Mar 2020 23:18:33 +0000
>> Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
>>
>>> On Mon, 2020-03-09 at 14:21 +0000, Paul Barker wrote:
>>>> To ensure that archives are captured for all dependencies of a typical
>>>> bitbake build we add do_deploy_archives to the list of recursive
>>>> dependencies of do_build. Without this, archives may be missed for
>>>> recipes such as gcc-source which do not create packages or populate a
>>>> sysroot.
>>>>
>>>> do_deploy_archives is also added to the recursive dependencies of
>>>> do_populate_sdk so that all sources required for an SDK can be captured.
>>>>
>>>> Signed-off-by: Paul Barker <pbarker@konsulko.com>
>>>> ---
>>>>  meta/classes/archiver.bbclass | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass
>>>> index fef7ad4f62..c11d36d708 100644
>>>> --- a/meta/classes/archiver.bbclass
>>>> +++ b/meta/classes/archiver.bbclass
>>>> @@ -604,7 +604,9 @@ addtask do_ar_configured after do_unpack_and_patch
>>>>  addtask do_ar_mirror after do_fetch
>>>>  addtask do_dumpdata
>>>>  addtask do_ar_recipe
>>>> -addtask do_deploy_archives before do_build
>>>> +addtask do_deploy_archives
>>>> +do_build[recrdeptask] += "do_deploy_archives"
>>>> +do_populate_sdk[recrdeptask] += "do_deploy_archives"  
>>>
>>> We implemented the --runall option to bitbake to try and avoid having
>>> recrdeptask versions of most tasks. Does that not work here? It should
>>> also work for the SDK I think?
> 
> You can't use a --runall operation on a task target, such as:
> 
> bitbake core-image-minimal -c populate_sdk
> 
> I can't find any way to get all of the archiver sources for the populate_sdk
> target.  The above should resolve this, and as Paul said below it will ensure
> that populate_sdk follows the same behavior as regular image generation as well.
> 
> I'm wondering if do_populate_sdk_ext needs it as well.

I verified, adding do_populate_sdk_ext is not needed.. the do_populate_sdk
appears to capture everything in the eSDK, at least in my configuration.

--Mark

>> If the archiver is enabled, its tasks should be in the dependency tree of
>> whatever you're building so that you don't need to invoke bitbake twice to
>> produce the required artifacts. For images that's the way the archiver has
>> always worked, if it's enabled then you just need to do `bitbake image` to
>> build the image and deploy the source archives. This change just extends that
>> behaviour to cover other things we can build and ensures that we don't miss
>> sources for recipes like gcc-source.
>>
>>
>>