[PATCH v3 0/3] devtool: modify: add support for multiple source

[PATCH v3 0/3] devtool: modify: add support for multiple source

[Julien Stephan]

Hello all,

As we discussed in #15162, I tried to implement a devtool unpack tracer.
I did manage to get a list of files unpacked after do_unpack and
manually copy them into WORKDIR but: 
- unpack tracer just tells us the url and the unpack dir, but does not
  directly give the unpacked files. Of course we can get the list by 
  looking at files in the temporary workdir directory at the beginning
  and the end of unpack, but I guess we can also do that without the
  unpack tracer mechanism
- the issue we are trying to fix here, is to get the secondary sources.
  We can obviously use the unpack tracer for that, but the main source,
  and local files (file://) are already correctly handled, so we would 
  end up handling different parts of SRC_URI in different location..
  IMHO the unpack tracer should handle all the src_uri entries itself
  but it means a more global code refactor.

Moreover, it turns out that the issue we are trying to solve was already known
and externalsrc class can handle secondary sources. The fix was
introduced with:
  da1bcf08089 - devtool: add support for multiple git url inside a cargo based recipe <Frederic Martinsons>
We can modify the problematic recipes by adding "type=git-dependency"
on the src_uri entries of the dependencies (secondary sources).

So, I am sending a patch to automatically add the "type=git-dependency"
on secondary sources when a recipe is under devtool control by modifying
the SRC_URI inside the append recipe (in build/workspace/appends/)

Doing that, I tested my work with the bzip2 recipe and found another
bug on do_populate_lic dependency. I added a patch to this series
because otherwise the selftest will not work, but it can be applied
separately.

Cheers
Julien

Julien Stephan (3):
  externalsrc: fix task dependency for do_populate_lic
  devtool: modify: add support for multiple source in SRC_URI
  oeqa/selftest/devtool: add test for recipes with multiple sources in
    SRC_URI

 meta/classes/externalsrc.bbclass        |  1 +
 meta/lib/oeqa/selftest/cases/devtool.py | 26 +++++++++++++++++++++++++
 scripts/lib/devtool/standard.py         | 20 +++++++++++++++++++
 3 files changed, 47 insertions(+)

-- 
2.43.0

[PATCH v3 1/3] externalsrc: fix task dependency for do_populate_lic

[Julien Stephan]

do_populate_lic dependencies are defined inside license.bbclass such as:

  addtask populate_lic after do_patch before do_build

but externalsrc deletes the do_patch task, so the only dependency left for
do_populate_lic is "before do_build"

On a devtool context, when doing devtool modify, sources are extracted inside
build/workspace/sources/${BPN}/ and local files inside
build/workspace/sources/${BPN}/oe-local-files

When building the recipe after a devtool modify, do_unpack is called again to
unpack (possibly modified) local files from
build/workspace/sources/${BPN}/oe-local-files into ${WORKDIR}.

Since the only left dependency for do_populate_lic is do_build, the
do_populate_lic can be called BEFORE do_unpack. Most of the time this is not a
problem, because license files are generally located inside ${S}, which
corresponds to build/workspace/sources/${BPN} (and is already unpacked),
but this can lead to an issue if recipe sets LIC_FILES_CHKSUM to look for
files in ${WORKDIR} (example from init-ifupdown_1.0.bb):

  LIC_FILES_CHKSUM = "file://${WORKDIR}/copyright;md5=3dd6192d306f582dee7687da3d8748ab"

So devtool modify init-ifupdown && bitbake init-ifupdown gives the following
error:

  WARNING: init-ifupdown-1.0-r0 do_populate_lic: Could not copy license file <...>/build/tmp/work/qemux86_64-poky-linux/init-ifupdown/1.0/copyright to <...>/build/tmp/work/qemux86_64-poky-linux/init-ifupdown/1.0/license-destdir/qemux86_64/init-ifupdown/copyright: [Errno 2] No such file or directory: '<...>/build/tmp/work/qemux86_64-poky-linux/init-ifupdown/1.0/copyright'
  ERROR: init-ifupdown-1.0-r0 do_populate_lic: QA Issue: init-ifupdown: LIC_FILES_CHKSUM points to an invalid file: <...>/build/tmp/work/qemux86_64-poky-linux/init-ifupdown/1.0/copyright [license-checksum]
  ERROR: init-ifupdown-1.0-r0 do_populate_lic: Fatal QA errors were found, failing task.
  ERROR: Logfile of failure stored in: <...>/build/tmp/work/qemux86_64-poky-linux/init-ifupdown/1.0/temp/log.do_populate_lic.838584
  ERROR: Task (<...>/poky/meta/recipes-core/init-ifupdown/init-ifupdown_1.0.bb:do_populate_lic) failed with exit code '1'

Fix this by forcing the do_populate_lic task to run after do_unpack

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/classes/externalsrc.bbclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index a54f316aa0a..70e27a8d351 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -104,6 +104,7 @@ python () {
         # If we deltask do_patch, there's no dependency to ensure do_unpack gets run, so add one
         # Note that we cannot use d.appendVarFlag() here because deps is expected to be a list object, not a string
         d.setVarFlag('do_configure', 'deps', (d.getVarFlag('do_configure', 'deps', False) or []) + ['do_unpack'])
+        d.setVarFlag('do_populate_lic', 'deps', (d.getVarFlag('do_populate_lic', 'deps', False) or []) + ['do_unpack'])
 
         for task in d.getVar("SRCTREECOVEREDTASKS").split():
             if local_srcuri and task in fetch_tasks:
-- 
2.43.0

[PATCH v3 2/3] devtool: modify: add support for multiple source in SRC_URI

[Julien Stephan]

[YOCTO #15162]

when doing devtool modify, sources are extracted into a devtool
temporary workdir. The main source is moved inside
build/workspace/sources/${BPN}/ and local files are moved inside
build/workspace/sources/${BPN}/oe-local-files. Secondary sources are
currently not handled and are lost.

Here is the output of devtool modify/build on bzip2 recipe:

NOTE: bzip2: compiling from external source tree <...>/build/workspace/sources/bzip2
ERROR: bzip2-1.0.8-r0 do_install_ptest_base: ExecutionError('<...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/temp/run.do_install_ptest_base.3368', 1, None, None)
ERROR: Logfile of failure stored in: <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/temp/log.do_install_ptest_base.3368
Log data follows:
| DEBUG: Executing shell function do_install_ptest_base
| NOTE: make -j 16 DESTDIR=<...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest install-ptest
| sed  -n '/^runtest:/,/^install-ptest:/{/^install-ptest:/!p}' \
|            ../../../../../../workspace/sources/bzip2/Makefile.am      > <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/Makefile
| cp ../../../../../../workspace/sources/bzip2/sample1.ref      <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/
| cp ../../../../../../workspace/sources/bzip2/sample2.ref      <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/
| cp ../../../../../../workspace/sources/bzip2/sample3.ref      <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/
| cp ../../../../../../workspace/sources/bzip2/sample1.bz2      <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/
| cp ../../../../../../workspace/sources/bzip2/sample2.bz2      <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/
| cp ../../../../../../workspace/sources/bzip2/sample3.bz2      <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/
| ln -s /usr/bin/bzip2          <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/bzip2
| cp: cannot stat '<...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/git/commons-compress': No such file or directory
| WARNING: <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/temp/run.do_install_ptest_base.3368:189 exit 1 from 'cp -r <...>/build/tmp/work/core2-64-poky-linux/bzip2/
1.0.8/git/commons-compress <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/image/usr/lib/bzip2/ptest/bzip2-tests/commons-compress'
| WARNING: Backtrace (BB generated script):
|       #1: do_install_ptest, <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/temp/run.do_install_ptest_base.3368, line 189
|       #2: do_install_ptest_base, <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/temp/run.do_install_ptest_base.3368, line 158
|       #3: main, <...>/build/tmp/work/core2-64-poky-linux/bzip2/1.0.8/temp/run.do_install_ptest_base.3368, line 226
ERROR: Task (<...>/poky/meta/recipes-extended/bzip2/bzip2_1.0.8.bb:do_install_ptest_base) failed with exit code '1'
NOTE: Tasks Summary: Attempted 776 tasks of which 765 didn't need to be rerun and 1 failed.

Summary: 1 task failed:
  <...>/poky/meta/recipes-extended/bzip2/bzip2_1.0.8.bb:do_install_ptest_base

externalsrc class modify SRC_URI to keep only:
* 'file', 'npmsw' and 'crate' sources
* url with type parameter matching 'kmeta' or 'git-dependency'

So by forcing to add type='git-dependency' on secondary sources, we
ensure that when building the recipe, the secondary sources can be
unpacked into WORKDIR.

This allows recipes containing several sources to be built under a
devtool context, but it has some limitations:
* user would not be able to generate patches for the secondary sources
* type="git-dependency" is added for secondary sources even on non git
  sources, so we may want to rename this parameter

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 scripts/lib/devtool/standard.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 13612d64c4c..8fb4b934d43 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -956,6 +956,26 @@ def modify(args, config, basepath, workspace):
 
         bb.utils.mkdirhier(os.path.dirname(appendfile))
         with open(appendfile, 'w') as f:
+            # if not present, add type=git-dependency to the secondary sources
+            # (non local files) so they can be extracted correctly when building a recipe after
+            #  doing a devtool modify on it
+            src_uri = rd.getVar('SRC_URI').split()
+            src_uri_append = []
+            src_uri_remove = []
+
+            # Assume first entry is main source extracted in ${S} so skip it
+            src_uri = src_uri[1::]
+
+            #Add "type=git-dependency" to all non local sources
+            for url in src_uri:
+                if not url.startswith('file://') and not 'type=' in url:
+                    src_uri_remove.append(url)
+                    src_uri_append.append('%s;type=git-dependency' % url)
+
+            if src_uri_remove:
+                f.write('SRC_URI:remove = "%s"\n' % ' '.join(src_uri_remove))
+                f.write('SRC_URI:append = "%s"\n\n' % ' '.join(src_uri_append))
+
             f.write('FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"\n')
             # Local files can be modified/tracked in separate subdir under srctree
             # Mostly useful for packages with S != WORKDIR
-- 
2.43.0

[PATCH v3 3/3] oeqa/selftest/devtool: add test for recipes with multiple sources in SRC_URI

[Julien Stephan]

add a non regression test for devtool modify/build on recipe having
several sources in SRC_URI

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/lib/oeqa/selftest/cases/devtool.py | 26 +++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index a8777207694..6ed51d7a452 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -1086,6 +1086,32 @@ class DevtoolModifyTests(DevtoolBase):
         check('devtool-override-qemuarm', 'This is a test for qemuarm\n')
         check('devtool-override-qemux86', 'This is a test for qemux86\n')
 
+    def test_devtool_modify_multiple_sources(self):
+        # This test check that recipes fetching several sources can be used with devtool modify/build
+        # Check preconditions
+        testrecipe = 'bzip2'
+        src_uri = get_bb_var('SRC_URI', testrecipe)
+        src1 = 'https://' in src_uri
+        src2 = 'git://' in src_uri
+        self.assertTrue(src1 and src2, 'This test expects the %s recipe to fetch both a git source and a tarball and it seems that it no longer does' % testrecipe)
+        # Clean up anything in the workdir/sysroot/sstate cache
+        bitbake('%s -c cleansstate' % testrecipe)
+        # Try modifying a recipe
+        tempdir = tempfile.mkdtemp(prefix='devtoolqa')
+        self.track_for_cleanup(tempdir)
+        self.track_for_cleanup(self.workspacedir)
+        self.add_command_to_tearDown('bitbake -c clean %s' % testrecipe)
+        self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
+        result = runCmd('devtool modify %s -x %s' % (testrecipe, tempdir))
+        self.assertEqual(result.status, 0, "Could not modify recipe %s. Output: %s" % (testrecipe, result.output))
+        # Test devtool status
+        result = runCmd('devtool status')
+        self.assertIn(testrecipe, result.output)
+        self.assertIn(tempdir, result.output)
+        # Try building
+        result = bitbake(testrecipe)
+        self.assertEqual(result.status, 0, "Bitbake failed, exit code %s, output %s" % (result.status, result.output))
+
 class DevtoolUpdateTests(DevtoolBase):
 
     def test_devtool_update_recipe(self):
-- 
2.43.0