diff mbox series

[PATCHv2,2/2] devtool: standard: Add new patches in correct order when finishing

Message ID 20260224232236.2048910-2-pkj@axis.com
State New
Headers show
Series [PATCHv2,1/2] lib/oe/patch: Make GitApplyTree.extractPatches() return the patches | expand

Commit Message

Peter Kjellerstedt Feb. 24, 2026, 11:22 p.m. UTC
Make sure that new patches that are added as a result of using devtool
finish are added to the SRC_URI in the same order they were committed.

Previously, the order was a result of the arbitrary order the patch
files were returned by os.walk(), which typically resulted in them being
added to the SRC_URI in the reverse order they were committed.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---

PATCHv2: Rely on GitApplyTree.extractPatches() to return the paths to
         the patches rather than using os.walk() to find them.

 meta/lib/oeqa/selftest/cases/devtool.py |  30 +++++--
 scripts/lib/devtool/standard.py         | 110 ++++++++++++------------
 2 files changed, 78 insertions(+), 62 deletions(-)
diff mbox series

Patch

diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index d1209dd94e..870317d4ff 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -2314,6 +2314,11 @@  class DevtoolUpgradeTests(DevtoolBase):
         result = runCmd('git status --porcelain', cwd=tempdir)
         self.assertIn('M maps.c', result.output)
         result = runCmd('git commit maps.c -m "Add a comment to the code"', cwd=tempdir)
+        # Make another change to the source
+        result = runCmd('sed -i \'/^#include "mdadm.h"/a \\/* Here is another comment *\\/\' maps.c', cwd=tempdir)
+        result = runCmd('git status --porcelain', cwd=tempdir)
+        self.assertIn('M maps.c', result.output)
+        result = runCmd('git commit maps.c -m "Add another comment to the code"', cwd=tempdir)
         for entry in os.listdir(recipedir):
             filesdir = os.path.join(recipedir, entry)
             if os.path.isdir(filesdir):
@@ -2333,8 +2338,15 @@  class DevtoolUpgradeTests(DevtoolBase):
         self.assertNotIn(recipe, result.output, 'Recipe should have been reset by finish but wasn\'t')
         self.assertNotExists(os.path.join(self.workspacedir, 'recipes', recipe), 'Recipe directory should not exist after finish')
         expected_status = [(' M', '.*/%s$' % os.path.basename(oldrecipefile)),
-                           ('??', '.*/.*-Add-a-comment-to-the-code.patch$')]
+                           ('??', '.*/.*-Add-a-comment-to-the-code.patch$'),
+                           ('??', '.*/.*-Add-another-comment-to-the-code.patch$')]
         self._check_repo_status(recipedir, expected_status)
+        result = runCmd('git diff %s' % os.path.basename(oldrecipefile), cwd=os.path.dirname(oldrecipefile))
+        # Check that the recipe got updated as expected
+        # Can't use self._check_diff() as the order of the added files matter.
+        result = result.output.splitlines()
+        self.assertEqual('+           file://0001-Add-a-comment-to-the-code.patch \\', result[8])
+        self.assertEqual('+           file://0002-Add-another-comment-to-the-code.patch \\', result[9])
 
     def test_devtool_finish_modify_otherlayer(self):
         recipe, oldrecipefile, recipedir, filesdir = self._setup_test_devtool_finish_modify()
@@ -2343,7 +2355,7 @@  class DevtoolUpgradeTests(DevtoolBase):
         relpth = os.path.relpath(recipedir, os.path.join(get_bb_var('COREBASE'), 'meta'))
         appenddir = os.path.join(get_test_layer(), relpth)
         self.track_for_cleanup(appenddir)
-        # Try finish to the original layer
+        # Try finish to another layer than the original layer
         self.add_command_to_tearDown('rm -rf %s ; cd %s ; git checkout %s' % (recipedir, os.path.dirname(recipedir), recipedir))
         result = runCmd('devtool finish %s meta-selftest' % recipe)
         result = runCmd('devtool status')
@@ -2356,15 +2368,19 @@  class DevtoolUpgradeTests(DevtoolBase):
         recipefn = recipefn.split('_')[0] + '_%'
         appendfile = os.path.join(appenddir, recipefn + '.bbappend')
         self.assertExists(appendfile, 'bbappend %s should have been created but wasn\'t' % appendfile)
+        # Check that the bbappend got created as expected
+        with open(appendfile, 'r') as f:
+            newlines = f.readlines()
+        self.assertEqual('SRC_URI += "file://0001-Add-a-comment-to-the-code.patch file://0002-Add-another-comment-to-the-code.patch"\n', newlines[2])
         newdir = os.path.join(appenddir, recipe)
         files = os.listdir(newdir)
-        foundpatch = None
-        for fn in files:
-            if fnmatch.fnmatch(fn, '*-Add-a-comment-to-the-code.patch'):
-                foundpatch = fn
+        foundpatch = False
+        for fn in files[:]:
+            if fnmatch.fnmatch(fn, '*-Add-a*-comment-to-the-code.patch'):
+                files.remove(fn)
+                foundpatch = True
         if not foundpatch:
             self.fail('No patch file created next to bbappend')
-        files.remove(foundpatch)
         if files:
             self.fail('Unexpected file(s) copied next to bbappend: %s' % ', '.join(files))
 
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index f4d5d7cd3f..b706d9c7a1 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1325,60 +1325,60 @@  def _export_patches(srctree, rd, start_revs, destdir, changed_revs=None):
 
     # Generate patches from Git, exclude local files directory
     patch_pathspec = _git_exclude_path(srctree, 'oe-local-files')
-    GitApplyTree.extractPatches(srctree, start_revs, destdir, patch_pathspec)
-    for dirpath, dirnames, filenames in os.walk(destdir):
-        new_patches = filenames
-        reldirpath = os.path.relpath(dirpath, destdir)
-        for new_patch in new_patches:
-            # Strip numbering from patch names. If it's a git sequence named patch,
-            # the numbers might not match up since we are starting from a different
-            # revision This does assume that people are using unique shortlog
-            # values, but they ought to be anyway...
-            new_basename = seqpatch_re.match(new_patch).group(2)
-            match_name = None
-            old_patch = None
-            for old_patch in existing_patches:
-                old_basename = seqpatch_re.match(old_patch).group(2)
-                old_basename_splitext = os.path.splitext(old_basename)
-                if old_basename.endswith(('.gz', '.bz2', '.Z')) and old_basename_splitext[0] == new_basename:
-                    old_patch_noext = os.path.splitext(old_patch)[0]
-                    match_name = old_patch_noext
-                    break
-                elif new_basename == old_basename:
-                    match_name = old_patch
-                    break
-            if match_name:
-                # Rename patch files
-                if new_patch != match_name:
-                    bb.utils.rename(os.path.join(destdir, new_patch),
-                              os.path.join(destdir, match_name))
-                # Need to pop it off the list now before checking changed_revs
-                oldpath = existing_patches.pop(old_patch)
-                if changed_revs is not None and dirpath in changed_revs:
-                    # Avoid updating patches that have not actually changed
-                    with open(os.path.join(dirpath, match_name), 'r') as f:
-                        firstlineitems = f.readline().split()
-                        # Looking for "From <hash>" line
-                        if len(firstlineitems) > 1 and len(firstlineitems[1]) == 40:
-                            if not firstlineitems[1] in changed_revs[dirpath]:
-                                continue
-                # Recompress if necessary
-                if oldpath.endswith(('.gz', '.Z')):
-                    bb.process.run(['gzip', match_name], cwd=destdir)
-                    if oldpath.endswith('.gz'):
-                        match_name += '.gz'
-                    else:
-                        match_name += '.Z'
-                elif oldpath.endswith('.bz2'):
-                    bb.process.run(['bzip2', match_name], cwd=destdir)
-                    match_name += '.bz2'
-                updated[match_name] = {'path' : oldpath}
-                if reldirpath != ".":
-                    updated[match_name]['patchdir'] = reldirpath
-            else:
-                added[new_patch] = {}
-                if reldirpath != ".":
-                    added[new_patch]['patchdir'] = reldirpath
+    new_patches = GitApplyTree.extractPatches(srctree, start_revs, destdir, patch_pathspec)
+    for patch_path in new_patches:
+        new_patch = os.path.basename(patch_path)
+        dirpath = os.path.dirname(patch_path)
+        patchdir = os.path.basename(dirpath)
+
+        # Strip numbering from patch names. If it's a git sequence named patch,
+        # the numbers might not match up since we are starting from a different
+        # revision This does assume that people are using unique shortlog
+        # values, but they ought to be anyway...
+        new_basename = seqpatch_re.match(new_patch).group(2)
+        match_name = None
+        old_patch = None
+        for old_patch in existing_patches:
+            old_basename = seqpatch_re.match(old_patch).group(2)
+            old_basename_splitext = os.path.splitext(old_basename)
+            if old_basename.endswith(('.gz', '.bz2', '.Z')) and old_basename_splitext[0] == new_basename:
+                old_patch_noext = os.path.splitext(old_patch)[0]
+                match_name = old_patch_noext
+                break
+            elif new_basename == old_basename:
+                match_name = old_patch
+                break
+        if match_name:
+            # Rename patch files
+            if new_patch != match_name:
+                bb.utils.rename(patch_path, os.path.join(dirpath, match_name))
+            # Need to pop it off the list now before checking changed_revs
+            oldpath = existing_patches.pop(old_patch)
+            if changed_revs is not None and dirpath in changed_revs:
+                # Avoid updating patches that have not actually changed
+                with open(os.path.join(dirpath, match_name), 'r') as f:
+                    firstlineitems = f.readline().split()
+                    # Looking for "From <hash>" line
+                    if len(firstlineitems) > 1 and len(firstlineitems[1]) == 40:
+                        if not firstlineitems[1] in changed_revs[dirpath]:
+                            continue
+            # Recompress if necessary
+            if oldpath.endswith(('.gz', '.Z')):
+                bb.process.run(['gzip', match_name], cwd=destdir)
+                if oldpath.endswith('.gz'):
+                    match_name += '.gz'
+                else:
+                    match_name += '.Z'
+            elif oldpath.endswith('.bz2'):
+                bb.process.run(['bzip2', match_name], cwd=destdir)
+                match_name += '.bz2'
+            updated[match_name] = {'path' : oldpath}
+            if patchdir != '.':
+                updated[match_name]['patchdir'] = patchdir
+        else:
+            added[new_patch] = {}
+            if patchdir != '.':
+                added[new_patch]['patchdir'] = patchdir
 
     return (updated, added, existing_patches)
 
@@ -1740,7 +1740,7 @@  def _update_recipe_patch(recipename, workspace, srctree, rd, appendlayerdir, wil
                         patchdir_param['patchdir'] = patchdir
                 patchfn = os.path.join(patches_dir, patchdir, basepath)
                 if os.path.dirname(path) + '/' == dl_dir:
-                    # This is a a downloaded patch file - we now need to
+                    # This is a downloaded patch file - we now need to
                     # replace the entry in SRC_URI with our local version
                     logger.info('Replacing remote patch %s with updated local version' % basepath)
                     path = os.path.join(files_dir, basepath)