[PATCH] devtool: standard: Add new patches in correct order when finishing

[PATCH] devtool: standard: Add new patches in correct order when finishing

[Peter Kjellerstedt]

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>
---
 meta/lib/oeqa/selftest/cases/devtool.py | 30 +++++++++++++++++++------
 scripts/lib/devtool/standard.py         |  6 ++++-
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index cf5ac6e9d7..0c5f11b58f 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -2286,6 +2286,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):
@@ -2305,8 +2310,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()
@@ -2315,7 +2327,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')
@@ -2328,15 +2340,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 1fd5947c41..7afb4654cc 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1322,7 +1322,11 @@ def _export_patches(srctree, rd, start_revs, destdir, changed_revs=None):
     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
+        # Sort the filenames to avoid the arbitrary order resulting from using
+        # os.walk(). This matters for added patches, and the assumption is that
+        # they are prefixed by a four digit number resulting from the order in
+        # which they were committed.
+        new_patches = sorted(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,

Re: [OE-core] [PATCH] devtool: standard: Add new patches in correct order when finishing

[Richard Purdie]

Hi Peter,

On Fri, 2026-02-06 at 02:13 +0100, Peter Kjellerstedt via lists.openembedded.org wrote:
> 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>
> ---
>  meta/lib/oeqa/selftest/cases/devtool.py | 30 +++++++++++++++++++------
>  scripts/lib/devtool/standard.py         |  6 ++++-
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> 
> 
[....]

> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> index 1fd5947c41..7afb4654cc 100644
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -1322,7 +1322,11 @@ def _export_patches(srctree, rd, start_revs, destdir, changed_revs=None):
>      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
> +        # Sort the filenames to avoid the arbitrary order resulting from using
> +        # os.walk(). This matters for added patches, and the assumption is that
> +        # they are prefixed by a four digit number resulting from the order in
> +        # which they were committed.
> +        new_patches = sorted(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,

This looks like a good find and something we should fix.

I did wonder if we always number the patches? I ask as the code in
extractPatches() calls:

"git", "format-patch", "--no-signature", "--no-numbered"

which hints that it might not?

Could we pass the patch list from that function in the correct order
rather than relying on a directory listing?

Cheers,

Richard

RE: [OE-core] [PATCH] devtool: standard: Add new patches in correct order when finishing

[Peter Kjellerstedt]

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 12 februari 2026 16:16
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] devtool: standard: Add new patches in correct order when finishing
> 
> Hi Peter,
> 
> On Fri, 2026-02-06 at 02:13 +0100, Peter Kjellerstedt via lists.openembedded.org wrote:
> > 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>
> > ---
> >  meta/lib/oeqa/selftest/cases/devtool.py | 30 +++++++++++++++++++------
> >  scripts/lib/devtool/standard.py         |  6 ++++-
> >  2 files changed, 28 insertions(+), 8 deletions(-)
> >
> >
> >
> [....]
> 
> > diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> > index 1fd5947c41..7afb4654cc 100644
> > --- a/scripts/lib/devtool/standard.py
> > +++ b/scripts/lib/devtool/standard.py
> > @@ -1322,7 +1322,11 @@ def _export_patches(srctree, rd, start_revs, destdir, changed_revs=None):
> >      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
> > +        # Sort the filenames to avoid the arbitrary order resulting from using
> > +        # os.walk(). This matters for added patches, and the assumption is that
> > +        # they are prefixed by a four digit number resulting from the order in
> > +        # which they were committed.
> > +        new_patches = sorted(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,
> 
> This looks like a good find and something we should fix.
> 
> I did wonder if we always number the patches? I ask as the code in
> extractPatches() calls:
> 
> "git", "format-patch", "--no-signature", "--no-numbered"
> 
> which hints that it might not?

The --no-numbered option only affects the Subject line in the generated 
patches and changes it from "[PATCH x/2] ..." to "[PATCH] ..." when 
multiple patches are created. The file names are always prefixed by a 
number.

> 
> Could we pass the patch list from that function in the correct order
> rather than relying on a directory listing?

Yes, we can. I have sent an updated version of the patch that does this.

> 
> Cheers,
> 
> Richard

//Peter