[PATCH v2 0/9] Several fixes around recipetool appendsrcfile(s) and oe.recipeutils.bbappend_recipe

[PATCH v2 0/9] Several fixes around recipetool appendsrcfile(s) and oe.recipeutils.bbappend_recipe

[Julien Stephan]

Hi all,

I was trying to use recipetool appendsrcfile to add a file to a recipe and I noticed several issues:
* -m is not correctly supported
* recipetool appendsrfile(s) are missing a usefull dry-run mode
* appendsrc function relies on oe.recipeutils.bbappend_recipe but
  duplicates some code: it constructs itself the new src_uri entry
  although oe.recipeutils.bbappend_recipe is already doing it
* we are lacking a mode to patch the recipe itself

So this series tries to fix the issues above, fix the selftest
accordingly and add new test for -m and for "patch mode" (update recipe)
and also add a way to specify the name of the file to add
(in oe.recipeutils.bbappend_recipe)

Improvements from v1:
- in bbappend_recipe function remove old entry if an entry with different parameters already exists 
to avoid duplicate entries with different parameters

Link to v1: https://lists.openembedded.org/g/openembedded-core/message/191544


Julien Stephan (9):
  recipetool: appendsrcfile(s): add dry-run mode
  recipeutils: bbappend_recipe: fix undefined variable
  recipeutils: bbappend_recipe: add a way to specify the name of the
    file to add
  recipeutils: bbappend_recipe: remove old srcuri entry if parameters
    are different
  recipetool: appendsrcfile(s): use params instead of extraline
  recipeutils: bbappend_recipe: allow to patch the recipe itself
  recipetool: appendsrcfile(s): add a mode to update the recipe itself
  oeqa/selftest/recipetool: appendsrfile: add test for machine
  oeqa/selftest/recipetool: appendsrc: add test for update mode

 meta/lib/oe/recipeutils.py                 | 62 +++++++++++-----
 meta/lib/oeqa/selftest/cases/recipetool.py | 83 +++++++++++++++++++---
 scripts/lib/devtool/standard.py            |  6 +-
 scripts/lib/recipetool/append.py           | 63 +++++++++++-----
 4 files changed, 168 insertions(+), 46 deletions(-)

-- 
2.42.0

[PATCH v2 1/9] recipetool: appendsrcfile(s): add dry-run mode

[Julien Stephan]

Add dry-run mode for recipetool appendsrcfile and appendsrcfiles, with
if necessary, a diff of changes

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 scripts/lib/recipetool/append.py | 33 +++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/recipetool/append.py b/scripts/lib/recipetool/append.py
index 9dbb1cc4b5a..314ff4e20fd 100644
--- a/scripts/lib/recipetool/append.py
+++ b/scripts/lib/recipetool/append.py
@@ -18,6 +18,7 @@ import shutil
 import scriptutils
 import errno
 from collections import defaultdict
+import difflib
 
 logger = logging.getLogger('recipetool')
 
@@ -355,7 +356,35 @@ def appendsrc(args, files, rd, extralines=None):
             extralines.append('SRC_URI += {0}'.format(source_uri))
         copyfiles[newfile] = srcfile
 
-    oe.recipeutils.bbappend_recipe(rd, args.destlayer, copyfiles, None, wildcardver=args.wildcard_version, machine=args.machine, extralines=extralines)
+    dry_run_output = None
+    dry_run_outdir = None
+    if args.dry_run:
+        import tempfile
+        dry_run_output = tempfile.TemporaryDirectory(prefix='devtool')
+        dry_run_outdir = dry_run_output.name
+
+    appendfile, _ = oe.recipeutils.bbappend_recipe(rd, args.destlayer, copyfiles, None, wildcardver=args.wildcard_version, machine=args.machine, extralines=extralines, redirect_output=dry_run_outdir)
+    if args.dry_run:
+        output = ''
+        appendfilename = os.path.basename(appendfile)
+        newappendfile = appendfile
+        if appendfile and os.path.exists(appendfile):
+            with open(appendfile, 'r') as f:
+                oldlines = f.readlines()
+        else:
+            appendfile = '/dev/null'
+            oldlines = []
+
+        with open(os.path.join(dry_run_outdir, appendfilename), 'r') as f:
+            newlines = f.readlines()
+        diff = difflib.unified_diff(oldlines, newlines, appendfile, newappendfile)
+        difflines = list(diff)
+        if difflines:
+            output += ''.join(difflines)
+        if output:
+            logger.info('Diff of changed files:\n%s' % output)
+        else:
+            logger.info('No changed files')
     tinfoil.modified_files()
 
 def appendsrcfiles(parser, args):
@@ -436,6 +465,7 @@ def register_commands(subparsers):
                                    help='Create/update a bbappend to add or replace source files',
                                    description='Creates a bbappend (or updates an existing one) to add or replace the specified file in the recipe sources, either those in WORKDIR or those in the source tree. This command lets you specify multiple files with a destination directory, so cannot specify the destination filename. See the `appendsrcfile` command for the other behavior.')
     parser.add_argument('-D', '--destdir', help='Destination directory (relative to S or WORKDIR, defaults to ".")', default='', type=destination_path)
+    parser.add_argument('-n', '--dry-run', help='Dry run mode', action='store_true')
     parser.add_argument('files', nargs='+', metavar='FILE', help='File(s) to be added to the recipe sources (WORKDIR or S)', type=existing_path)
     parser.set_defaults(func=lambda a: appendsrcfiles(parser, a), parserecipes=True)
 
@@ -443,6 +473,7 @@ def register_commands(subparsers):
                                    parents=[common_src],
                                    help='Create/update a bbappend to add or replace a source file',
                                    description='Creates a bbappend (or updates an existing one) to add or replace the specified files in the recipe sources, either those in WORKDIR or those in the source tree. This command lets you specify the destination filename, not just destination directory, but only works for one file. See the `appendsrcfiles` command for the other behavior.')
+    parser.add_argument('-n', '--dry-run', help='Dry run mode', action='store_true')
     parser.add_argument('file', metavar='FILE', help='File to be added to the recipe sources (WORKDIR or S)', type=existing_path)
     parser.add_argument('destfile', metavar='DESTFILE', nargs='?', help='Destination path (relative to S or WORKDIR, optional)', type=destination_path)
     parser.set_defaults(func=lambda a: appendsrcfile(parser, a), parserecipes=True)
-- 
2.42.0

[PATCH v2 2/9] recipeutils: bbappend_recipe: fix undefined variable

[Julien Stephan]

In the case get_bbappend_path returns None (could not find the layer
containing the recipe) the error message tries to print the recipefile,
but it is not defined. Fix it.

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/lib/oe/recipeutils.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 3336db8ab06..7465a2d0ab4 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -702,6 +702,7 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
     if not removevalues:
         removevalues = {}
 
+    recipefile = rd.getVar('FILE')
     # Determine how the bbappend should be named
     appendpath, pathok = get_bbappend_path(rd, destlayerdir, wildcardver)
     if not appendpath:
-- 
2.42.0

[PATCH v2 3/9] recipeutils: bbappend_recipe: add a way to specify the name of the file to add

[Julien Stephan]

bbappend_recipe can take a dict of source files to add to SRC_URI where
the key is the full path to the file to be added and the value is
the original file in the recipe space.

Modify the dict value to use a tupple: (originalfile, newname), to allow
specifying the name of the file we would like to add

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/lib/oe/recipeutils.py       | 20 ++++++++++++--------
 scripts/lib/devtool/standard.py  |  6 +++---
 scripts/lib/recipetool/append.py |  4 ++--
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 7465a2d0ab4..9784ae3b031 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -671,12 +671,13 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
         rd: data dictionary for the recipe
         destlayerdir: base directory of the layer to place the bbappend in
             (subdirectory path from there will be determined automatically)
-        srcfiles: dict of source files to add to SRC_URI, where the value
-            is the full path to the file to be added, and the value is the
-            original filename as it would appear in SRC_URI or None if it
-            isn't already present. You may pass None for this parameter if
-            you simply want to specify your own content via the extralines
-            parameter.
+        srcfiles: dict of source files to add to SRC_URI, where the key
+            is the full path to the file to be added, and the value is a tuple
+            containing the original filename as it would appear in SRC_URI
+            or None if it isn't already present and the new name of the file or
+            None to use by default basename(original filename).
+            You may pass None for this parameter if you simply want to specify
+            your own content via the extralines parameter.
         install: dict mapping entries in srcfiles to a tuple of two elements:
             install path (*without* ${D} prefix) and permission value (as a
             string, e.g. '0644').
@@ -764,11 +765,14 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
     copyfiles = {}
     if srcfiles:
         instfunclines = []
-        for i, (newfile, origsrcfile) in enumerate(srcfiles.items()):
+        for i, (newfile, (origsrcfile, newname)) in enumerate(srcfiles.items()):
             srcfile = origsrcfile
             srcurientry = None
             if not srcfile:
-                srcfile = os.path.basename(newfile)
+                if newname:
+                    srcfile = newname
+                else:
+                    srcfile = os.path.basename(newfile)
                 srcurientry = 'file://%s' % srcfile
                 if params and params[i]:
                     srcurientry = '%s;%s' % (srcurientry, ';'.join('%s=%s' % (k,v) for k,v in params[i].items()))
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index d53fb810071..fa473014f90 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1568,7 +1568,7 @@ def _update_recipe_srcrev(recipename, workspace, srctree, rd, appendlayerdir, wi
                 update_srcuri = True
 
         if appendlayerdir:
-            files = dict((os.path.join(local_files_dir, key), val) for
+            files = dict((os.path.join(local_files_dir, key), (val, None)) for
                           key, val in list(upd_f.items()) + list(new_f.items()))
             removevalues = {}
             if update_srcuri:
@@ -1678,9 +1678,9 @@ def _update_recipe_patch(recipename, workspace, srctree, rd, appendlayerdir, wil
         destpath = None
         srcuri = (rd.getVar('SRC_URI', False) or '').split()
         if appendlayerdir:
-            files = OrderedDict((os.path.join(local_files_dir, key), val) for
+            files = OrderedDict((os.path.join(local_files_dir, key), (val, None)) for
                          key, val in list(upd_f.items()) + list(new_f.items()))
-            files.update(OrderedDict((os.path.join(patches_dir, key), val) for
+            files.update(OrderedDict((os.path.join(patches_dir, key), (val, None)) for
                               key, val in list(upd_p.items()) + list(new_p.items())))
             if files or remove_files:
                 removevalues = None
diff --git a/scripts/lib/recipetool/append.py b/scripts/lib/recipetool/append.py
index 314ff4e20fd..09e314481f1 100644
--- a/scripts/lib/recipetool/append.py
+++ b/scripts/lib/recipetool/append.py
@@ -300,7 +300,7 @@ def appendfile(args):
                 if st.st_mode & stat.S_IXUSR:
                     perms = '0755'
             install = {args.newfile: (args.targetpath, perms)}
-        oe.recipeutils.bbappend_recipe(rd, args.destlayer, {args.newfile: sourcepath}, install, wildcardver=args.wildcard_version, machine=args.machine)
+        oe.recipeutils.bbappend_recipe(rd, args.destlayer, {args.newfile: (None, sourcepath)}, install, wildcardver=args.wildcard_version, machine=args.machine)
         tinfoil.modified_files()
         return 0
     else:
@@ -354,7 +354,7 @@ def appendsrc(args, files, rd, extralines=None):
                 logger.warning('{0!r} is already in SRC_URI, not adding'.format(source_uri))
         else:
             extralines.append('SRC_URI += {0}'.format(source_uri))
-        copyfiles[newfile] = srcfile
+        copyfiles[newfile] = (None, srcfile)
 
     dry_run_output = None
     dry_run_outdir = None
-- 
2.42.0

[PATCH v2 4/9] recipeutils: bbappend_recipe: remove old srcuri entry if parameters are different

[Julien Stephan]

Currently we do not add a new src_ury entry if the entry already exists
AND the parameters are the same.

I believe that when an entry already exist with different parameters,
we should remove it and add the new entry otherwise we end up with two
entries with different parameters

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/lib/oe/recipeutils.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 9784ae3b031..8aa82396c86 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -774,14 +774,22 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
                 else:
                     srcfile = os.path.basename(newfile)
                 srcurientry = 'file://%s' % srcfile
+                oldentry = None
+                for uri in rd.getVar('SRC_URI').split():
+                    if srcurientry in uri:
+                        oldentry = uri
                 if params and params[i]:
                     srcurientry = '%s;%s' % (srcurientry, ';'.join('%s=%s' % (k,v) for k,v in params[i].items()))
                 # Double-check it's not there already
                 # FIXME do we care if the entry is added by another bbappend that might go away?
                 if not srcurientry in rd.getVar('SRC_URI').split():
                     if machine:
+                        if oldentry:
+                            appendline('SRC_URI:remove%s' % appendoverride, '=', ' ' + oldentry)
                         appendline('SRC_URI:append%s' % appendoverride, '=', ' ' + srcurientry)
                     else:
+                        if oldentry:
+                            appendline('SRC_URI:remove', '=', oldentry)
                         appendline('SRC_URI', '+=', srcurientry)
             copyfiles[newfile] = srcfile
             if install:
-- 
2.42.0

[PATCH v2 5/9] recipetool: appendsrcfile(s): use params instead of extraline

[Julien Stephan]

appendsrc function relies on oe.recipeutils.bbappend_recipe to
copy files and add the corresponding entries in SRC_URI.

Currently, appendsrc function build itself the new SRC_URI entry to add the
correct subdir param, and gives it using the extralines parameter.
This has 2 drawbacks:
- oe.recipeutils.bbappend_recipe can already do this if we specify the
  correct params, so we have duplicate code
- the duplicated code is not fully functional: for example, it doesn't
  take into account the -m/--machine parameter

So fix this by not using extralines but give correctly formatted params.

Also remove the check for already existing entries as
oe.recipeutils.bbappend_recipe already implement it

The new bbappend file now have the SRC_URI entry after the
FILESEXTRAPATHS so fix the selftest.

Update test_recipetool_appendsrcfile_existing_in_src_uri_diff_params
test because recipetool appendsrcfiles used to not add new src_uri entry
if the entry already exist even with different parameters while
oe.recipeutils.bbappend_recipe adds it if parameters are different (and
remove the old entry)

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/lib/oeqa/selftest/cases/recipetool.py | 21 +++++++++++--------
 scripts/lib/recipetool/append.py           | 24 ++++++++--------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/meta/lib/oeqa/selftest/cases/recipetool.py b/meta/lib/oeqa/selftest/cases/recipetool.py
index 55cbba9ca74..21cb350e8a6 100644
--- a/meta/lib/oeqa/selftest/cases/recipetool.py
+++ b/meta/lib/oeqa/selftest/cases/recipetool.py
@@ -1054,9 +1054,9 @@ class RecipetoolAppendsrcBase(RecipetoolBase):
         for uri in src_uri:
             p = urllib.parse.urlparse(uri)
             if p.scheme == 'file':
-                return p.netloc + p.path
+                return p.netloc + p.path, uri
 
-    def _test_appendsrcfile(self, testrecipe, filename=None, destdir=None, has_src_uri=True, srcdir=None, newfile=None, options=''):
+    def _test_appendsrcfile(self, testrecipe, filename=None, destdir=None, has_src_uri=True, srcdir=None, newfile=None, remove=None, options=''):
         if newfile is None:
             newfile = self.testfile
 
@@ -1083,12 +1083,18 @@ class RecipetoolAppendsrcBase(RecipetoolBase):
 
         expectedlines = ['FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"\n',
                          '\n']
+
+        if remove:
+            for entry in remove:
+                expectedlines.extend(['SRC_URI:remove = "%s"\n' % entry,
+                                       '\n'])
+
         if has_src_uri:
             uri = 'file://%s' % filename
             if expected_subdir:
                 uri += ';subdir=%s' % expected_subdir
-            expectedlines[0:0] = ['SRC_URI += "%s"\n' % uri,
-                                  '\n']
+            expectedlines.extend(['SRC_URI += "%s"\n' % uri,
+                                  '\n'])
 
         return self._try_recipetool_appendsrcfile(testrecipe, newfile, destpath, options, expectedlines, [filename])
 
@@ -1143,18 +1149,17 @@ class RecipetoolAppendsrcTests(RecipetoolAppendsrcBase):
 
     def test_recipetool_appendsrcfile_existing_in_src_uri(self):
         testrecipe = 'base-files'
-        filepath = self._get_first_file_uri(testrecipe)
+        filepath,_  = self._get_first_file_uri(testrecipe)
         self.assertTrue(filepath, 'Unable to test, no file:// uri found in SRC_URI for %s' % testrecipe)
         self._test_appendsrcfile(testrecipe, filepath, has_src_uri=False)
 
     def test_recipetool_appendsrcfile_existing_in_src_uri_diff_params(self):
         testrecipe = 'base-files'
         subdir = 'tmp'
-        filepath = self._get_first_file_uri(testrecipe)
+        filepath, srcuri_entry = self._get_first_file_uri(testrecipe)
         self.assertTrue(filepath, 'Unable to test, no file:// uri found in SRC_URI for %s' % testrecipe)
 
-        output = self._test_appendsrcfile(testrecipe, filepath, subdir, has_src_uri=False)
-        self.assertTrue(any('with different parameters' in l for l in output))
+        self._test_appendsrcfile(testrecipe, filepath, subdir, remove=[srcuri_entry])
 
     def test_recipetool_appendsrcfile_replace_file_srcdir(self):
         testrecipe = 'bash'
diff --git a/scripts/lib/recipetool/append.py b/scripts/lib/recipetool/append.py
index 09e314481f1..7e56290a0ab 100644
--- a/scripts/lib/recipetool/append.py
+++ b/scripts/lib/recipetool/append.py
@@ -300,6 +300,8 @@ def appendfile(args):
                 if st.st_mode & stat.S_IXUSR:
                     perms = '0755'
             install = {args.newfile: (args.targetpath, perms)}
+        if sourcepath:
+            sourcepath = os.path.basename(sourcepath)
         oe.recipeutils.bbappend_recipe(rd, args.destlayer, {args.newfile: (None, sourcepath)}, install, wildcardver=args.wildcard_version, machine=args.machine)
         tinfoil.modified_files()
         return 0
@@ -329,6 +331,7 @@ def appendsrc(args, files, rd, extralines=None):
 
     copyfiles = {}
     extralines = extralines or []
+    params = []
     for newfile, srcfile in files.items():
         src_destdir = os.path.dirname(srcfile)
         if not args.use_workdir:
@@ -339,22 +342,12 @@ def appendsrc(args, files, rd, extralines=None):
             src_destdir = os.path.join(os.path.relpath(srcdir, workdir), src_destdir)
         src_destdir = os.path.normpath(src_destdir)
 
-        source_uri = 'file://{0}'.format(os.path.basename(srcfile))
         if src_destdir and src_destdir != '.':
-            source_uri += ';subdir={0}'.format(src_destdir)
-
-        simple = bb.fetch.URI(source_uri)
-        simple.params = {}
-        simple_str = str(simple)
-        if simple_str in simplified:
-            existing = simplified[simple_str]
-            if source_uri != existing:
-                logger.warning('{0!r} is already in SRC_URI, with different parameters: {1!r}, not adding'.format(source_uri, existing))
-            else:
-                logger.warning('{0!r} is already in SRC_URI, not adding'.format(source_uri))
+            params.append({'subdir': src_destdir})
         else:
-            extralines.append('SRC_URI += {0}'.format(source_uri))
-        copyfiles[newfile] = (None, srcfile)
+            params.append({})
+
+        copyfiles[newfile] = (None, os.path.basename(srcfile))
 
     dry_run_output = None
     dry_run_outdir = None
@@ -363,7 +356,8 @@ def appendsrc(args, files, rd, extralines=None):
         dry_run_output = tempfile.TemporaryDirectory(prefix='devtool')
         dry_run_outdir = dry_run_output.name
 
-    appendfile, _ = oe.recipeutils.bbappend_recipe(rd, args.destlayer, copyfiles, None, wildcardver=args.wildcard_version, machine=args.machine, extralines=extralines, redirect_output=dry_run_outdir)
+    appendfile, _ = oe.recipeutils.bbappend_recipe(rd, args.destlayer, copyfiles, None, wildcardver=args.wildcard_version, machine=args.machine, extralines=extralines, params=params,
+                                                   redirect_output=dry_run_outdir)
     if args.dry_run:
         output = ''
         appendfilename = os.path.basename(appendfile)
-- 
2.42.0

[PATCH v2 6/9] recipeutils: bbappend_recipe: allow to patch the recipe itself

[Julien Stephan]

Add a new parameter update_original_recipe to allow to patch a recipe
instead of creating/updating a bbappend

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 meta/lib/oe/recipeutils.py | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 8aa82396c86..013b7412742 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -664,7 +664,7 @@ def get_bbappend_path(d, destlayerdir, wildcardver=False):
     return (appendpath, pathok)
 
 
-def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False, machine=None, extralines=None, removevalues=None, redirect_output=None, params=None):
+def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False, machine=None, extralines=None, removevalues=None, redirect_output=None, params=None, update_original_recipe=False):
     """
     Writes a bbappend file for a recipe
     Parameters:
@@ -698,19 +698,29 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
         params:
             Parameters to use when adding entries to SRC_URI. If specified,
             should be a list of dicts with the same length as srcfiles.
+        update_original_recipe:
+            Force to update the original recipe instead of creating/updating
+            a bbapend. destlayerdir must contain the original recipe
     """
 
     if not removevalues:
         removevalues = {}
 
     recipefile = rd.getVar('FILE')
-    # Determine how the bbappend should be named
-    appendpath, pathok = get_bbappend_path(rd, destlayerdir, wildcardver)
-    if not appendpath:
-        bb.error('Unable to determine layer directory containing %s' % recipefile)
-        return (None, None)
-    if not pathok:
-        bb.warn('Unable to determine correct subdirectory path for bbappend file - check that what %s adds to BBFILES also matches .bbappend files. Using %s for now, but until you fix this the bbappend will not be applied.' % (os.path.join(destlayerdir, 'conf', 'layer.conf'), os.path.dirname(appendpath)))
+    if update_original_recipe:
+        if destlayerdir not in recipefile:
+            bb.error("destlayerdir %s doesn't contain the original recipe (%s), cannot update it" % (destlayerdir, recipefile))
+            return (None, None)
+
+        appendpath = recipefile
+    else:
+        # Determine how the bbappend should be named
+        appendpath, pathok = get_bbappend_path(rd, destlayerdir, wildcardver)
+        if not appendpath:
+            bb.error('Unable to determine layer directory containing %s' % recipefile)
+            return (None, None)
+        if not pathok:
+            bb.warn('Unable to determine correct subdirectory path for bbappend file - check that what %s adds to BBFILES also matches .bbappend files. Using %s for now, but until you fix this the bbappend will not be applied.' % (os.path.join(destlayerdir, 'conf', 'layer.conf'), os.path.dirname(appendpath)))
 
     appenddir = os.path.dirname(appendpath)
     if not redirect_output:
@@ -755,7 +765,7 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
             bbappendlines.append((varname, op, value))
 
     destsubdir = rd.getVar('PN')
-    if srcfiles:
+    if not update_original_recipe and srcfiles:
         bbappendlines.append(('FILESEXTRAPATHS:prepend', ':=', '${THISDIR}/${PN}:'))
 
     appendoverride = ''
@@ -789,7 +799,10 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
                         appendline('SRC_URI:append%s' % appendoverride, '=', ' ' + srcurientry)
                     else:
                         if oldentry:
-                            appendline('SRC_URI:remove', '=', oldentry)
+                            if update_original_recipe:
+                                removevalues['SRC_URI'] = oldentry
+                            else:
+                                appendline('SRC_URI:remove', '=', oldentry)
                         appendline('SRC_URI', '+=', srcurientry)
             copyfiles[newfile] = srcfile
             if install:
@@ -811,6 +824,8 @@ def bbappend_recipe(rd, destlayerdir, srcfiles, install=None, wildcardver=False,
         # multiple times per operation when we're handling overrides)
         if os.path.exists(appendpath) and not os.path.exists(outfile):
             shutil.copy2(appendpath, outfile)
+    elif update_original_recipe:
+        outfile = recipefile
     else:
         bb.note('Writing append file %s' % appendpath)
         outfile = appendpath
-- 
2.42.0

[PATCH v2 7/9] recipetool: appendsrcfile(s): add a mode to update the recipe itself

[Julien Stephan]

Add a new option -u/--update-recipe to update the recipe itself instead
of creating/updating a bbappend file

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 scripts/lib/recipetool/append.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/recipetool/append.py b/scripts/lib/recipetool/append.py
index 7e56290a0ab..66ed160a0a6 100644
--- a/scripts/lib/recipetool/append.py
+++ b/scripts/lib/recipetool/append.py
@@ -357,7 +357,9 @@ def appendsrc(args, files, rd, extralines=None):
         dry_run_outdir = dry_run_output.name
 
     appendfile, _ = oe.recipeutils.bbappend_recipe(rd, args.destlayer, copyfiles, None, wildcardver=args.wildcard_version, machine=args.machine, extralines=extralines, params=params,
-                                                   redirect_output=dry_run_outdir)
+                                                   redirect_output=dry_run_outdir, update_original_recipe=args.update_recipe)
+    if not appendfile:
+        return
     if args.dry_run:
         output = ''
         appendfilename = os.path.basename(appendfile)
@@ -459,6 +461,7 @@ def register_commands(subparsers):
                                    help='Create/update a bbappend to add or replace source files',
                                    description='Creates a bbappend (or updates an existing one) to add or replace the specified file in the recipe sources, either those in WORKDIR or those in the source tree. This command lets you specify multiple files with a destination directory, so cannot specify the destination filename. See the `appendsrcfile` command for the other behavior.')
     parser.add_argument('-D', '--destdir', help='Destination directory (relative to S or WORKDIR, defaults to ".")', default='', type=destination_path)
+    parser.add_argument('-u', '--update-recipe', help='Update recipe instead of creating (or updating) a bbapend file. DESTLAYER must contains the recipe to update', action='store_true')
     parser.add_argument('-n', '--dry-run', help='Dry run mode', action='store_true')
     parser.add_argument('files', nargs='+', metavar='FILE', help='File(s) to be added to the recipe sources (WORKDIR or S)', type=existing_path)
     parser.set_defaults(func=lambda a: appendsrcfiles(parser, a), parserecipes=True)
@@ -467,6 +470,7 @@ def register_commands(subparsers):
                                    parents=[common_src],
                                    help='Create/update a bbappend to add or replace a source file',
                                    description='Creates a bbappend (or updates an existing one) to add or replace the specified files in the recipe sources, either those in WORKDIR or those in the source tree. This command lets you specify the destination filename, not just destination directory, but only works for one file. See the `appendsrcfiles` command for the other behavior.')
+    parser.add_argument('-u', '--update-recipe', help='Update recipe instead of creating (or updating) a bbapend file. DESTLAYER must contains the recipe to update', action='store_true')
     parser.add_argument('-n', '--dry-run', help='Dry run mode', action='store_true')
     parser.add_argument('file', metavar='FILE', help='File to be added to the recipe sources (WORKDIR or S)', type=existing_path)
     parser.add_argument('destfile', metavar='DESTFILE', nargs='?', help='Destination path (relative to S or WORKDIR, optional)', type=destination_path)
-- 
2.42.0

[PATCH v2 8/9] oeqa/selftest/recipetool: appendsrfile: add test for machine

[Julien Stephan]

Add a new test for machine specific bbappend override

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

diff --git a/meta/lib/oeqa/selftest/cases/recipetool.py b/meta/lib/oeqa/selftest/cases/recipetool.py
index 21cb350e8a6..c55025e7df6 100644
--- a/meta/lib/oeqa/selftest/cases/recipetool.py
+++ b/meta/lib/oeqa/selftest/cases/recipetool.py
@@ -1056,7 +1056,7 @@ class RecipetoolAppendsrcBase(RecipetoolBase):
             if p.scheme == 'file':
                 return p.netloc + p.path, uri
 
-    def _test_appendsrcfile(self, testrecipe, filename=None, destdir=None, has_src_uri=True, srcdir=None, newfile=None, remove=None, options=''):
+    def _test_appendsrcfile(self, testrecipe, filename=None, destdir=None, has_src_uri=True, srcdir=None, newfile=None, remove=None, machine=None , options=''):
         if newfile is None:
             newfile = self.testfile
 
@@ -1084,17 +1084,39 @@ class RecipetoolAppendsrcBase(RecipetoolBase):
         expectedlines = ['FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"\n',
                          '\n']
 
+        override = ""
+        if machine:
+            options += ' -m %s' % machine
+            override = ':append:%s' % machine
+            expectedlines.extend(['PACKAGE_ARCH = "${MACHINE_ARCH}"\n',
+                                  '\n'])
+
         if remove:
             for entry in remove:
-                expectedlines.extend(['SRC_URI:remove = "%s"\n' % entry,
+                if machine:
+                    entry_remove_line = 'SRC_URI:remove:%s = " %s"\n' % (machine, entry)
+                else:
+                    entry_remove_line = 'SRC_URI:remove = "%s"\n' % entry
+
+                expectedlines.extend([entry_remove_line,
                                        '\n'])
 
         if has_src_uri:
             uri = 'file://%s' % filename
             if expected_subdir:
                 uri += ';subdir=%s' % expected_subdir
-            expectedlines.extend(['SRC_URI += "%s"\n' % uri,
-                                  '\n'])
+            if machine:
+                src_uri_line = 'SRC_URI%s = " %s"\n' % (override, uri)
+            else:
+                src_uri_line = 'SRC_URI += "%s"\n' % uri
+
+            expectedlines.extend([src_uri_line, '\n'])
+
+        with open("/tmp/tmp.txt", "w") as file:
+            print(expectedlines, file=file)
+
+        if machine:
+            filename = '%s/%s' % (machine, filename)
 
         return self._try_recipetool_appendsrcfile(testrecipe, newfile, destpath, options, expectedlines, [filename])
 
@@ -1153,13 +1175,23 @@ class RecipetoolAppendsrcTests(RecipetoolAppendsrcBase):
         self.assertTrue(filepath, 'Unable to test, no file:// uri found in SRC_URI for %s' % testrecipe)
         self._test_appendsrcfile(testrecipe, filepath, has_src_uri=False)
 
-    def test_recipetool_appendsrcfile_existing_in_src_uri_diff_params(self):
+    def test_recipetool_appendsrcfile_existing_in_src_uri_diff_params(self, machine=None):
         testrecipe = 'base-files'
         subdir = 'tmp'
         filepath, srcuri_entry = self._get_first_file_uri(testrecipe)
         self.assertTrue(filepath, 'Unable to test, no file:// uri found in SRC_URI for %s' % testrecipe)
 
-        self._test_appendsrcfile(testrecipe, filepath, subdir, remove=[srcuri_entry])
+        self._test_appendsrcfile(testrecipe, filepath, subdir, machine=machine, remove=[srcuri_entry])
+
+    def test_recipetool_appendsrcfile_machine(self):
+        # A very basic test
+        self._test_appendsrcfile('base-files', 'a-file', machine='mymachine')
+
+        # Force cleaning the output of previous test
+        self.tearDownLocal()
+
+        # A more complex test: existing entry in src_uri with different param
+        self.test_recipetool_appendsrcfile_existing_in_src_uri_diff_params(machine='mymachine')
 
     def test_recipetool_appendsrcfile_replace_file_srcdir(self):
         testrecipe = 'bash'
-- 
2.42.0

[PATCH v2 9/9] oeqa/selftest/recipetool: appendsrc: add test for update mode

[Julien Stephan]

add a basic test for testing the update mode of recipetool
appendsrcfile(s)

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

diff --git a/meta/lib/oeqa/selftest/cases/recipetool.py b/meta/lib/oeqa/selftest/cases/recipetool.py
index c55025e7df6..0a82b081a2d 100644
--- a/meta/lib/oeqa/selftest/cases/recipetool.py
+++ b/meta/lib/oeqa/selftest/cases/recipetool.py
@@ -1193,6 +1193,34 @@ class RecipetoolAppendsrcTests(RecipetoolAppendsrcBase):
         # A more complex test: existing entry in src_uri with different param
         self.test_recipetool_appendsrcfile_existing_in_src_uri_diff_params(machine='mymachine')
 
+    def test_recipetool_appendsrcfile_update_recipe_basic(self):
+        testrecipe = "base-files"
+        recipefile = get_bb_var('FILE', testrecipe)
+        result = runCmd('bitbake-layers show-layers')
+        layerrecipe = None
+        for line in result.output.splitlines()[3:]:
+            with open("/tmp/juju.txt", "a") as file:
+                layer = line.split()[1]
+                print(layer, file=file)
+                if layer in recipefile:
+                    layerrecipe = layer
+                    break
+        self.assertTrue(layerrecipe, 'Unable to find the layer containing %s' % testrecipe)
+        cmd = 'recipetool appendsrcfile -u %s %s %s' % (layerrecipe, testrecipe, self.testfile)
+        result = runCmd(cmd)
+        self.assertNotIn('Traceback', result.output)
+        self.add_command_to_tearDown('cd %s; rm -f %s/%s; git checkout .' % (os.path.dirname(recipefile), testrecipe, os.path.basename(self.testfile)))
+
+        expected_status = [(' M', '.*/%s$' % os.path.basename(recipefile)),
+                           ('??', '.*/%s/%s/%s$' % (testrecipe, testrecipe, os.path.basename(self.testfile)))]
+        self._check_repo_status(os.path.dirname(recipefile), expected_status)
+        result = runCmd('git diff %s' % os.path.basename(recipefile), cwd=os.path.dirname(recipefile))
+        removelines = []
+        addlines = [
+            'file://%s \\\\' % os.path.basename(self.testfile),
+        ]
+        self._check_diff(result.output, addlines, removelines)
+
     def test_recipetool_appendsrcfile_replace_file_srcdir(self):
         testrecipe = 'bash'
         filepath = 'Makefile.in'
-- 
2.42.0

Re: [OE-core] [PATCH v2 0/9] Several fixes around recipetool appendsrcfile(s) and oe.recipeutils.bbappend_recipe

[Alexandre Belloni]

Hello,

The series doesn't apply on master, can you rebase?

On 04/12/2023 16:25:54+0100, Julien Stephan wrote:
> Hi all,
> 
> I was trying to use recipetool appendsrcfile to add a file to a recipe and I noticed several issues:
> * -m is not correctly supported
> * recipetool appendsrfile(s) are missing a usefull dry-run mode
> * appendsrc function relies on oe.recipeutils.bbappend_recipe but
>   duplicates some code: it constructs itself the new src_uri entry
>   although oe.recipeutils.bbappend_recipe is already doing it
> * we are lacking a mode to patch the recipe itself
> 
> So this series tries to fix the issues above, fix the selftest
> accordingly and add new test for -m and for "patch mode" (update recipe)
> and also add a way to specify the name of the file to add
> (in oe.recipeutils.bbappend_recipe)
> 
> Improvements from v1:
> - in bbappend_recipe function remove old entry if an entry with different parameters already exists 
> to avoid duplicate entries with different parameters
> 
> Link to v1: https://lists.openembedded.org/g/openembedded-core/message/191544
> 
> 
> Julien Stephan (9):
>   recipetool: appendsrcfile(s): add dry-run mode
>   recipeutils: bbappend_recipe: fix undefined variable
>   recipeutils: bbappend_recipe: add a way to specify the name of the
>     file to add
>   recipeutils: bbappend_recipe: remove old srcuri entry if parameters
>     are different
>   recipetool: appendsrcfile(s): use params instead of extraline
>   recipeutils: bbappend_recipe: allow to patch the recipe itself
>   recipetool: appendsrcfile(s): add a mode to update the recipe itself
>   oeqa/selftest/recipetool: appendsrfile: add test for machine
>   oeqa/selftest/recipetool: appendsrc: add test for update mode
> 
>  meta/lib/oe/recipeutils.py                 | 62 +++++++++++-----
>  meta/lib/oeqa/selftest/cases/recipetool.py | 83 +++++++++++++++++++---
>  scripts/lib/devtool/standard.py            |  6 +-
>  scripts/lib/recipetool/append.py           | 63 +++++++++++-----
>  4 files changed, 168 insertions(+), 46 deletions(-)
> 
> -- 
> 2.42.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#191744): https://lists.openembedded.org/g/openembedded-core/message/191744
> Mute This Topic: https://lists.openembedded.org/mt/102972288/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com