[PATCH v2 0/9] execute patch commands without an implicit shell

[PATCH v2 0/9] execute patch commands without an implicit shell

[Anders Heimer]

Continue the shell=True cleanup in oe.patch.

Run runcmd() argument lists directly instead of joining them into an
implicit shell command string.

This series converts the non test runcmd() callers that invoked it with
-sh.

The series also replaces the cat-to-patch pipeline with patch -i,
removes the obsolete PATCHFILE assignment, converts GitApplyTree
commands to argv lists, and keeps manual-resolution commands as argv
lists.

oelib tests cover argv handling and error reporting, GitApplyTree
patch-name preservation, fallback commit metadata, and run=False
command generation without repository side effects.

Changes in v2:
- Accept both valid Git `%aI` representations of UTC, `Z` and `+00:00`,
  in the fallback metadata test

Anders Heimer (9):
  oe/patch: drop shell=True from runcmd
  oeqa/oelib: add runcmd tests
  oe/patch: convert simple runcmd shell callers
  oe/patch: avoid shell pipeline in _applypatch
  oe/patch: remove obsolete PATCHFILE assignment
  oeqa/oelib: test GitApplyTree patch names
  oe/patch: pass GitApplyTree commands as argv lists
  oe/patch: return manual-resolution commands as argv lists
  oeqa/oelib: test patch command argv handling

 meta/lib/oe/patch.py                        | 131 +++++----
 meta/lib/oeqa/selftest/cases/oelib/patch.py | 282 ++++++++++++++++++++
 scripts/lib/devtool/upgrade.py              |   2 +-
 3 files changed, 355 insertions(+), 60 deletions(-)
 create mode 100644 meta/lib/oeqa/selftest/cases/oelib/patch.py

[PATCH v2 1/9] oe/patch: drop shell=True from runcmd

[Anders Heimer]

Run runcmd() argument lists directly instead of joining them into a
shell command string.

Callers that still require shell syntax continue to invoke sh -c
explicitly and are left for separate cleanup.

Running Popen with shell=True could return shell status 127/126.
Preserve that behavior by translating the errno codes.

Unrelated bug fix: Stop shifting the return code by 8 bits.
subprocess.Popen.returncode is already the process exit status.

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/patch.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 2cd8de22c7..b152a2d784 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -5,6 +5,7 @@
 #
 
 import os
+import errno
 import shlex
 import subprocess
 import oe.path
@@ -37,16 +38,19 @@ def runcmd(args, dir = None):
         # print("cwd: %s -> %s" % (olddir, dir))
 
     try:
-        args = [ shlex.quote(str(arg)) for arg in args ]
-        cmd = " ".join(args)
-        # print("cmd: %s" % cmd)
-        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+        cmd = [str(arg) for arg in args]
+        print_cmd = shlex.join(cmd)
+        try:
+            proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        except OSError as exc:
+            status = 127 if exc.errno in (errno.ENOENT, errno.ENOTDIR) else 126
+            raise CmdError(print_cmd, status, "stdout: \nstderr: %s" % exc) from exc
         stdout, stderr = proc.communicate()
         stdout = stdout.decode('utf-8')
         stderr = stderr.decode('utf-8')
         exitstatus = proc.returncode
         if exitstatus != 0:
-            raise CmdError(cmd, exitstatus >> 8, "stdout: %s\nstderr: %s" % (stdout, stderr))
+            raise CmdError(print_cmd, exitstatus, "stdout: %s\nstderr: %s" % (stdout, stderr))
         if " fuzz " in stdout and "Hunk " in stdout:
             # Drop patch fuzz info with header and footer to log file so
             # insane.bbclass can handle to throw error/warning

[PATCH v2 2/9] oeqa/oelib: add runcmd tests

[Anders Heimer]

Cover literal argv handling, explicit shell use, subprocess exit
status, and exec failure wrapping.

AI-Generated: Claude Opus 4.6

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oeqa/selftest/cases/oelib/patch.py | 45 +++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 meta/lib/oeqa/selftest/cases/oelib/patch.py

diff --git a/meta/lib/oeqa/selftest/cases/oelib/patch.py b/meta/lib/oeqa/selftest/cases/oelib/patch.py
new file mode 100644
index 0000000000..4cbfef4ce6
--- /dev/null
+++ b/meta/lib/oeqa/selftest/cases/oelib/patch.py
@@ -0,0 +1,45 @@
+#
+# Copyright OpenEmbedded Contributors
+#
+# SPDX-License-Identifier: MIT
+#
+
+import sys
+from unittest.case import TestCase
+
+import oe.patch
+
+
+class TestRunCmd(TestCase):
+    def test_runcmd_preserves_argv_elements(self):
+        output = oe.patch.runcmd([
+            sys.executable,
+            "-c",
+            "import sys; print(sys.argv[1])",
+            "value with spaces;$(false)*",
+        ])
+        self.assertEqual(output, "value with spaces;$(false)*\n")
+
+    def test_runcmd_allows_explicit_shell(self):
+        output = oe.patch.runcmd([
+            "sh", "-c", 'printf "%s" "$1"', "sh", "shell value",
+        ])
+        self.assertEqual(output, "shell value")
+
+    def test_runcmd_reports_exit_status(self):
+        with self.assertRaises(oe.patch.CmdError) as ctx:
+            oe.patch.runcmd([
+                sys.executable,
+                "-c",
+                "raise SystemExit(7)",
+            ])
+
+        self.assertEqual(ctx.exception.status, 7)
+
+    def test_runcmd_wraps_exec_failure(self):
+        with self.assertRaises(oe.patch.CmdError) as ctx:
+            oe.patch.runcmd([
+                "/definitely-not-an-existing-executable",
+            ])
+
+        self.assertEqual(ctx.exception.status, 127)

[PATCH v2 3/9] oe/patch: convert simple runcmd shell callers

[Anders Heimer]

Replace simple sh -c runcmd() users with direct argv calls where the
commands do not need shell syntax.

Also replace the cat-to-file redirection used when appending patch files
with shutil.copyfile().

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/patch.py | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index b152a2d784..290eb990f1 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -195,10 +195,12 @@ class PatchTree(PatchSet):
         bb.utils.mkdirhier(self.patchdir)
 
     def _appendPatchFile(self, patch, strippath):
+        import shutil
+
         with open(self.seriespath, 'a') as f:
             f.write(os.path.basename(patch) + "," + strippath + "\n")
-        shellcmd = ["cat", patch, ">" , self.patchdir + "/" + os.path.basename(patch)]
-        runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+        dest = os.path.join(self.patchdir, os.path.basename(patch))
+        shutil.copyfile(patch, dest)
 
     def _removePatch(self, p):
         patch = {}
@@ -433,8 +435,8 @@ class GitApplyTree(PatchTree):
         outlines, author, date, subject = GitApplyTree.interpretPatchHeader(lines)
         if not author or not subject or not date:
             try:
-                shellcmd = ["git", "log", "--format=email", "--follow", "--diff-filter=A", "--", patchfile]
-                out = runcmd(["sh", "-c", " ".join(shellcmd)], os.path.dirname(patchfile))
+                cmd = ["git", "log", "--format=email", "--follow", "--diff-filter=A", "--", patchfile]
+                out = runcmd(cmd, os.path.dirname(patchfile))
             except CmdError:
                 out = None
             if out:
@@ -525,11 +527,11 @@ class GitApplyTree(PatchTree):
         patches = []
         try:
             for name, rev in startcommits.items():
-                shellcmd = ["git", "format-patch", "--no-signature", "--no-numbered", rev, "-o", tempdir]
+                cmd = ["git", "format-patch", "--no-signature", "--no-numbered", rev, "-o", tempdir]
                 if paths:
-                    shellcmd.append('--')
-                    shellcmd.extend(paths)
-                out = runcmd(["sh", "-c", " ".join(shellcmd)], os.path.join(tree, name))
+                    cmd.append('--')
+                    cmd.extend(paths)
+                out = runcmd(cmd, os.path.join(tree, name))
                 if out:
                     for srcfile in out.split():
                         # This loop, which is used to remove any line that
@@ -585,11 +587,11 @@ class GitApplyTree(PatchTree):
     def _commitpatch(self, patch, patchfilevar):
         output = ""
         # Add all files
-        shellcmd = ["git", "add", "-f", "-A", "."]
-        output += runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+        cmd = ["git", "add", "-f", "-A", "."]
+        output += runcmd(cmd, self.dir)
         # Exclude the patches directory
-        shellcmd = ["git", "reset", "HEAD", self.patchdir]
-        output += runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+        cmd = ["git", "reset", "HEAD", self.patchdir]
+        output += runcmd(cmd, self.dir)
         # Commit the result
         (tmpfile, shellcmd) = self.prepareCommit(patch['file'], self.commituser, self.commitemail)
         try:
@@ -642,21 +644,21 @@ class GitApplyTree(PatchTree):
             except CmdError:
                 # Need to abort the git am, or we'll still be within it at the end
                 try:
-                    shellcmd = ["git", "--work-tree=%s" % reporoot, "am", "--abort"]
-                    runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+                    cmd = ["git", "--work-tree=%s" % reporoot, "am", "--abort"]
+                    runcmd(cmd, self.dir)
                 except CmdError:
                     pass
                 # git am won't always clean up after itself, sadly, so...
-                shellcmd = ["git", "--work-tree=%s" % reporoot, "reset", "--hard", "HEAD"]
-                runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+                cmd = ["git", "--work-tree=%s" % reporoot, "reset", "--hard", "HEAD"]
+                runcmd(cmd, self.dir)
                 # Also need to take care of any stray untracked files
-                shellcmd = ["git", "--work-tree=%s" % reporoot, "clean", "-f"]
-                runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+                cmd = ["git", "--work-tree=%s" % reporoot, "clean", "-f"]
+                runcmd(cmd, self.dir)
 
                 # Fall back to git apply
-                shellcmd = ["git", "--git-dir=%s" % reporoot, "apply", "-p%s" % patch['strippath']]
+                cmd = ["git", "--git-dir=%s" % reporoot, "apply", "-p%s" % patch['strippath']]
                 try:
-                    output = _applypatchhelper(shellcmd, patch, force, reverse, run)
+                    output = _applypatchhelper(cmd, patch, force, reverse, run)
                 except CmdError:
                     # Fall back to patch
                     output = PatchTree._applypatch(self, patch, force, reverse, run)

[PATCH v2 4/9] oe/patch: avoid shell pipeline in _applypatch

[Anders Heimer]

Use patch -i to read the patch file directly instead of running
cat through a shell pipeline.

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/patch.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 290eb990f1..1ff57a9f8a 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -235,24 +235,24 @@ class PatchTree(PatchSet):
         self.patches.insert(i, patch)
 
     def _applypatch(self, patch, force = False, reverse = False, run = True):
-        shellcmd = ["cat", patch['file'], "|", "patch", "--no-backup-if-mismatch", "-p", patch['strippath']]
+        cmd = ["patch", "--no-backup-if-mismatch", "-p", str(patch['strippath']), "-i", patch['file']]
         if reverse:
-            shellcmd.append('-R')
+            cmd.append('-R')
 
         if not run:
-            return "sh" + "-c" + " ".join(shellcmd)
+            return cmd
 
         if not force:
-            shellcmd.append('--dry-run')
+            cmd.append('--dry-run')
 
         try:
-            output = runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+            output = runcmd(cmd, self.dir)
 
             if force:
                 return
 
-            shellcmd.pop(len(shellcmd) - 1)
-            output = runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+            cmd.pop(len(cmd) - 1)
+            output = runcmd(cmd, self.dir)
         except CmdError as err:
             raise bb.BBHandledException("Applying '%s' failed:\n%s" %
                                         (os.path.basename(patch['file']), err.output))

[PATCH v2 5/9] oe/patch: remove obsolete PATCHFILE assignment

[Anders Heimer]

PATCHFILE was used by an older Git hook to preserve the original
patch filename. Current GitApplyTree records that filename in
refs/notes/devtool and extractPatches() reads the note when recreating
patches.

No current OE-Core, BitBake or installed hook consumer remains, so
remove the unused environment assignment. This to simplify the runcmd
argv-list changes.

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/patch.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 1ff57a9f8a..9240637189 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -584,7 +584,7 @@ class GitApplyTree(PatchTree):
                 check_dirtyness = True
         return check_dirtyness
 
-    def _commitpatch(self, patch, patchfilevar):
+    def _commitpatch(self, patch):
         output = ""
         # Add all files
         cmd = ["git", "add", "-f", "-A", "."]
@@ -595,7 +595,6 @@ class GitApplyTree(PatchTree):
         # Commit the result
         (tmpfile, shellcmd) = self.prepareCommit(patch['file'], self.commituser, self.commitemail)
         try:
-            shellcmd.insert(0, patchfilevar)
             output += runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
         finally:
             os.remove(tmpfile)
@@ -621,7 +620,6 @@ class GitApplyTree(PatchTree):
 
         patch_applied = True
         try:
-            patchfilevar = 'PATCHFILE="%s"' % os.path.basename(patch['file'])
             if self._need_dirty_check():
                 # Check dirtyness of the tree
                 try:
@@ -633,10 +631,10 @@ class GitApplyTree(PatchTree):
                         # The tree is dirty, no need to try to apply patches with git anymore
                         # since they fail, fallback directly to patch
                         output = PatchTree._applypatch(self, patch, force, reverse, run)
-                        output += self._commitpatch(patch, patchfilevar)
+                        output += self._commitpatch(patch)
                         return output
             try:
-                shellcmd = [patchfilevar, "git", "--work-tree=%s" % reporoot]
+                shellcmd = ["git", "--work-tree=%s" % reporoot]
                 self.gitCommandUserOptions(shellcmd, self.commituser, self.commitemail)
                 shellcmd += ["am", "--committer-date-is-author-date",
                              "-3", "--keep-cr", "--no-scissors", "-p%s" % patch['strippath']]
@@ -662,7 +660,7 @@ class GitApplyTree(PatchTree):
                 except CmdError:
                     # Fall back to patch
                     output = PatchTree._applypatch(self, patch, force, reverse, run)
-                output += self._commitpatch(patch, patchfilevar)
+                output += self._commitpatch(patch)
                 return output
         except:
             patch_applied = False

[PATCH v2 6/9] oeqa/oelib: test GitApplyTree patch names

[Anders Heimer]

Exercise both the git-am path and the fallback commit path through
GitApplyTree.Import() and Push(). Verify that refs/notes/devtool records
the original patch filename and that extractPatches() recreates patches
with that name.

AI-Generated: Claude Opus 4.6

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oeqa/selftest/cases/oelib/patch.py | 136 ++++++++++++++++++++
 1 file changed, 136 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/oelib/patch.py b/meta/lib/oeqa/selftest/cases/oelib/patch.py
index 4cbfef4ce6..69e06b5ac1 100644
--- a/meta/lib/oeqa/selftest/cases/oelib/patch.py
+++ b/meta/lib/oeqa/selftest/cases/oelib/patch.py
@@ -4,12 +4,28 @@
 # SPDX-License-Identifier: MIT
 #
 
+import os
+import shutil
+import subprocess
 import sys
+import tempfile
 from unittest.case import TestCase
 
 import oe.patch
 
 
+class PatchTestDataStore:
+    def __init__(self, workdir):
+        self.vars = {
+            "PATCH_GIT_USER_NAME": "OE Test",
+            "PATCH_GIT_USER_EMAIL": "oe-test@example.com",
+            "WORKDIR": workdir,
+        }
+
+    def getVar(self, name):
+        return self.vars.get(name, "")
+
+
 class TestRunCmd(TestCase):
     def test_runcmd_preserves_argv_elements(self):
         output = oe.patch.runcmd([
@@ -43,3 +59,123 @@ class TestRunCmd(TestCase):
             ])
 
         self.assertEqual(ctx.exception.status, 127)
+
+
+class RecordingGitApplyTree(oe.patch.GitApplyTree):
+    def __init__(self, *args, **kwargs):
+        self.commitpatch_called = False
+        super().__init__(*args, **kwargs)
+
+    def _commitpatch(self, patch, *args):
+        self.commitpatch_called = True
+        return super()._commitpatch(patch, *args)
+
+
+class TestGitApplyTree(TestCase):
+    def setUp(self):
+        if shutil.which("git") is None:
+            self.skipTest("git not found")
+        if shutil.which("patch") is None:
+            self.skipTest("patch not found")
+
+    def git(self, cwd, *args):
+        subprocess.check_call(
+            ["git"] + list(args),
+            cwd=cwd,
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        )
+
+    def make_repo(self, tmpdir, name, text="base\n"):
+        repo = os.path.join(tmpdir, name)
+        os.mkdir(repo)
+        self.git(repo, "init")
+        self.git(repo, "config", "user.name", "OE Test")
+        self.git(repo, "config", "user.email", "oe-test@example.com")
+        with open(os.path.join(repo, "file.txt"), "w") as f:
+            f.write(text)
+        self.git(repo, "add", "file.txt")
+        self.git(repo, "commit", "-m", "base")
+        return repo
+
+    def make_git_am_patch(self, tmpdir, basename):
+        repo = self.make_repo(tmpdir, "source")
+        with open(os.path.join(repo, "file.txt"), "w") as f:
+            f.write("git am change\n")
+        self.git(repo, "commit", "-am", "git am change")
+        patchdir = os.path.join(tmpdir, "patches")
+        os.mkdir(patchdir)
+        subprocess.check_call(
+            ["git", "format-patch", "-1", "HEAD", "-o", patchdir],
+            cwd=repo,
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        )
+        patch = os.path.join(patchdir, os.listdir(patchdir)[0])
+        renamed = os.path.join(patchdir, basename)
+        os.rename(patch, renamed)
+        return renamed
+
+    def make_plain_diff_patch(self, tmpdir, basename):
+        patch = os.path.join(tmpdir, basename)
+        with open(patch, "w") as f:
+            f.write(
+                "Subject: [PATCH] plain diff change\n"
+                "\n"
+                "--- a/file.txt\n"
+                "+++ b/file.txt\n"
+                "@@ -1 +1 @@\n"
+                "-base\n"
+                "+plain diff change\n"
+            )
+        return patch
+
+    def apply_patch(self, tree, patch):
+        tree._need_dirty_check = lambda: False
+        tree.Import({"file": patch, "strippath": "1"}, False)
+        tree.Push(False)
+
+    def assert_note_and_extract(self, repo, patchname, expected):
+        note = oe.patch.runcmd(
+            ["git", "notes", "--ref", oe.patch.GitApplyTree.notes_ref,
+             "show", "HEAD"],
+            repo,
+        )
+        self.assertIn("%s: %s" % (oe.patch.GitApplyTree.original_patch,
+                                  patchname), note)
+
+        with tempfile.TemporaryDirectory(prefix="oe-patch-extract-") as outdir:
+            patches = oe.patch.GitApplyTree.extractPatches(
+                repo, {"": "HEAD~1"}, outdir
+            )
+            self.assertEqual([os.path.basename(p) for p in patches], [patchname])
+            with open(patches[0]) as f:
+                self.assertIn(expected, f.read())
+
+    def test_git_am_preserves_original_patch_name(self):
+        with tempfile.TemporaryDirectory(prefix="oe-gitapply-am-") as tmpdir:
+            patchname = "0001-distinct-original-name.patch"
+            patch = self.make_git_am_patch(tmpdir, patchname)
+            repo = self.make_repo(tmpdir, "target")
+            tree = RecordingGitApplyTree(repo, PatchTestDataStore(tmpdir))
+
+            self.apply_patch(tree, patch)
+
+            self.assertFalse(tree.commitpatch_called)
+            with open(os.path.join(repo, "file.txt")) as f:
+                self.assertEqual(f.read(), "git am change\n")
+            self.assert_note_and_extract(repo, patchname, "+git am change")
+
+    def test_fallback_preserves_original_patch_name(self):
+        with tempfile.TemporaryDirectory(prefix="oe-gitapply-fallback-") as tmpdir:
+            patchname = "plain-diff-original-name.patch"
+            patch = self.make_plain_diff_patch(tmpdir, patchname)
+            repo = self.make_repo(tmpdir, "target")
+            tree = RecordingGitApplyTree(repo, PatchTestDataStore(tmpdir))
+
+            self.apply_patch(tree, patch)
+
+            self.assertTrue(tree.commitpatch_called)
+            with open(os.path.join(repo, "file.txt")) as f:
+                self.assertEqual(f.read(), "plain diff change\n")
+            self.assert_note_and_extract(repo, patchname, "+plain diff change")

[PATCH v2 7/9] oe/patch: pass GitApplyTree commands as argv lists

[Anders Heimer]

After removing PATCHFILE from the git-am command, the GitApplyTree apply
and commit commands no longer need shell syntax. Build the commands as
argv lists and pass them directly to runcmd().

Remove shell quoting from git -c, --author and --date arguments now that
those values are passed directly to Git instead of through sh -c. Update
the devtool upgrade caller to use shlex.join() when it still embeds
those options in a shell command string.

Keep run=False command generation as argv lists and avoid adding notes or
committing fallback changes when no command was run. Run the dirty-tree
status check from self.dir so it inspects the patch tree consistently.

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/patch.py           | 50 +++++++++++++++++++---------------
 scripts/lib/devtool/upgrade.py |  2 +-
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 9240637189..c76b78fcac 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -419,9 +419,9 @@ class GitApplyTree(PatchTree):
             commituser = d.getVar('PATCH_GIT_USER_NAME')
             commitemail = d.getVar('PATCH_GIT_USER_EMAIL')
         if commituser:
-            cmd += ['-c', 'user.name="%s"' % commituser]
+            cmd += ['-c', 'user.name=%s' % commituser]
         if commitemail:
-            cmd += ['-c', 'user.email="%s"' % commitemail]
+            cmd += ['-c', 'user.email=%s' % commitemail]
 
     @staticmethod
     def prepareCommit(patchfile, commituser=None, commitemail=None):
@@ -464,9 +464,9 @@ class GitApplyTree(PatchTree):
         cmd += ["commit", "-F", tmpfile, "--no-verify", "--no-gpg-sign"]
         # git doesn't like plain email addresses as authors
         if author and '<' in author:
-            cmd.append('--author="%s"' % author)
+            cmd.append('--author=%s' % author)
         if date:
-            cmd.append('--date="%s"' % date)
+            cmd.append('--date=%s' % date)
         return (tmpfile, cmd)
 
     @staticmethod
@@ -593,9 +593,9 @@ class GitApplyTree(PatchTree):
         cmd = ["git", "reset", "HEAD", self.patchdir]
         output += runcmd(cmd, self.dir)
         # Commit the result
-        (tmpfile, shellcmd) = self.prepareCommit(patch['file'], self.commituser, self.commitemail)
+        (tmpfile, cmd) = self.prepareCommit(patch['file'], self.commituser, self.commitemail)
         try:
-            output += runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+            output += runcmd(cmd, self.dir)
         finally:
             os.remove(tmpfile)
         return output
@@ -603,16 +603,24 @@ class GitApplyTree(PatchTree):
     def _applypatch(self, patch, force = False, reverse = False, run = True):
         import shutil
 
-        def _applypatchhelper(shellcmd, patch, force = False, reverse = False, run = True):
+        def _applypatchhelper(cmd, patch, force = False, reverse = False, run = True):
+            cmd = list(cmd)
+
             if reverse:
-                shellcmd.append('-R')
+                cmd.append('-R')
 
-            shellcmd.append(patch['file'])
+            cmd.append(patch['file'])
 
             if not run:
-                return "sh" + "-c" + " ".join(shellcmd)
+                return cmd
 
-            return runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+            return runcmd(cmd, self.dir)
+
+        def _committed_fallback_output(output):
+            if not run:
+                return output
+            output += self._commitpatch(patch)
+            return output
 
         reporoot = (runcmd("git rev-parse --show-toplevel".split(), self.dir) or '').strip()
         if not reporoot:
@@ -623,7 +631,7 @@ class GitApplyTree(PatchTree):
             if self._need_dirty_check():
                 # Check dirtyness of the tree
                 try:
-                    output = runcmd(["git", "--work-tree=%s" % reporoot, "status", "--short"])
+                    output = runcmd(["git", "--work-tree=%s" % reporoot, "status", "--short"], self.dir)
                 except CmdError:
                     pass
                 else:
@@ -631,14 +639,13 @@ class GitApplyTree(PatchTree):
                         # The tree is dirty, no need to try to apply patches with git anymore
                         # since they fail, fallback directly to patch
                         output = PatchTree._applypatch(self, patch, force, reverse, run)
-                        output += self._commitpatch(patch)
-                        return output
+                        return _committed_fallback_output(output)
             try:
-                shellcmd = ["git", "--work-tree=%s" % reporoot]
-                self.gitCommandUserOptions(shellcmd, self.commituser, self.commitemail)
-                shellcmd += ["am", "--committer-date-is-author-date",
-                             "-3", "--keep-cr", "--no-scissors", "-p%s" % patch['strippath']]
-                return _applypatchhelper(shellcmd, patch, force, reverse, run)
+                cmd = ["git", "--work-tree=%s" % reporoot]
+                self.gitCommandUserOptions(cmd, self.commituser, self.commitemail)
+                cmd += ["am", "--committer-date-is-author-date",
+                        "-3", "--keep-cr", "--no-scissors", "-p%s" % patch['strippath']]
+                return _applypatchhelper(cmd, patch, force, reverse, run)
             except CmdError:
                 # Need to abort the git am, or we'll still be within it at the end
                 try:
@@ -660,13 +667,12 @@ class GitApplyTree(PatchTree):
                 except CmdError:
                     # Fall back to patch
                     output = PatchTree._applypatch(self, patch, force, reverse, run)
-                output += self._commitpatch(patch)
-                return output
+                return _committed_fallback_output(output)
         except:
             patch_applied = False
             raise
         finally:
-            if patch_applied:
+            if patch_applied and run:
                 GitApplyTree.addNote(self.dir, "HEAD", GitApplyTree.original_patch, os.path.basename(patch['file']), self.commituser, self.commitemail)
 
 
diff --git a/scripts/lib/devtool/upgrade.py b/scripts/lib/devtool/upgrade.py
index 061a1ce2a0..74adb29402 100644
--- a/scripts/lib/devtool/upgrade.py
+++ b/scripts/lib/devtool/upgrade.py
@@ -283,7 +283,7 @@ def _extract_new_source(newpv, srctree, no_patch, srcrev, srcbranch, branch, kee
 
         useroptions = []
         oe.patch.GitApplyTree.gitCommandUserOptions(useroptions, d=rd)
-        __run('git %s commit -q -m "Commit of upstream changes at version %s" --allow-empty' % (' '.join(useroptions), newpv))
+        __run('git %s commit -q -m "Commit of upstream changes at version %s" --allow-empty' % (shlex.join(useroptions), newpv))
         __run('git tag -f --no-sign devtool-base-%s' % newpv)
 
     revs = {}

[PATCH v2 8/9] oe/patch: return manual-resolution commands as argv lists

[Anders Heimer]

PatchTree and GitApplyTree now generate patch commands as argv lists
when run=False.

Pass run through when pushing one patch and return the command without
advancing the current patch state. Use shlex.join() when writing the
command to the manual resolver's shell startup file.

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/patch.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index c76b78fcac..1d50e83ab7 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -277,7 +277,10 @@ class PatchTree(PatchSet):
                 next = 0
 
             bb.note("applying patch %s" % self.patches[next])
-            ret = self._applypatch(self.patches[next], force)
+            ret = self._applypatch(self.patches[next], force, run=run)
+
+            if not run:
+                return ret
 
             self._current = next
             return ret
@@ -868,7 +871,7 @@ class UserResolver(Resolver):
                 f.write("echo 'Dropping to a shell, so patch rejects can be fixed manually.'\n")
                 f.write("echo 'Run \"quilt refresh\" when patch is corrected, press CTRL+D to exit.'\n")
                 f.write("echo ''\n")
-                f.write(" ".join(patchcmd) + "\n")
+                f.write(shlex.join(patchcmd) + "\n")
             os.chmod(rcfile, 0o775)
 
             self.terminal("bash --rcfile " + rcfile, 'Patch Rejects: Please fix patch rejects manually', self.patchset.d)

[PATCH v2 9/9] oeqa/oelib: test patch command argv handling

[Anders Heimer]

Exercise fallback patch metadata with an author, date and committer name
containing spaces so direct argv execution passes those values to Git
without shell quoting.

Also cover the GitApplyTree and PatchTree run=False paths so
manual-resolution command generation continues to return argv lists
without applying the patch. Use patch filenames with spaces to verify
the paths remain single argv elements.

AI-Generated: Claude Opus 4.6

Reviewed-by: Daniel Turull <daniel.turull@ericsson.com>
Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oeqa/selftest/cases/oelib/patch.py | 101 ++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/oelib/patch.py b/meta/lib/oeqa/selftest/cases/oelib/patch.py
index 69e06b5ac1..8492549764 100644
--- a/meta/lib/oeqa/selftest/cases/oelib/patch.py
+++ b/meta/lib/oeqa/selftest/cases/oelib/patch.py
@@ -61,6 +61,37 @@ class TestRunCmd(TestCase):
         self.assertEqual(ctx.exception.status, 127)
 
 
+class TestPatchTree(TestCase):
+    def test_push_run_false_returns_argv(self):
+        with tempfile.TemporaryDirectory(prefix="oe-patchtree-run-false-") as tmpdir:
+            srcdir = os.path.join(tmpdir, "source")
+            os.mkdir(srcdir)
+            with open(os.path.join(srcdir, "file.txt"), "w") as f:
+                f.write("base\n")
+
+            patch = os.path.join(tmpdir, "patch with spaces.patch")
+            with open(patch, "w") as f:
+                f.write(
+                    "--- a/file.txt\n"
+                    "+++ b/file.txt\n"
+                    "@@ -1 +1 @@\n"
+                    "-base\n"
+                    "+changed\n"
+                )
+
+            tree = oe.patch.PatchTree(srcdir, PatchTestDataStore(tmpdir))
+            tree.Import({"file": patch, "strippath": "1"}, False)
+
+            cmd = tree.Push(False, run=False)
+
+            self.assertEqual(cmd[:5], [
+                "patch", "--no-backup-if-mismatch", "-p", "1", "-i",
+            ])
+            self.assertEqual(cmd[-1], patch)
+            self.assertIsNone(tree.current())
+            with open(os.path.join(srcdir, "file.txt")) as f:
+                self.assertEqual(f.read(), "base\n")
+
 class RecordingGitApplyTree(oe.patch.GitApplyTree):
     def __init__(self, *args, **kwargs):
         self.commitpatch_called = False
@@ -120,6 +151,8 @@ class TestGitApplyTree(TestCase):
         patch = os.path.join(tmpdir, basename)
         with open(patch, "w") as f:
             f.write(
+                "Author: Fallback Author <fallback.author@example.com>\n"
+                "Date: Fri, 01 Jan 2021 12:34:56 +0000\n"
                 "Subject: [PATCH] plain diff change\n"
                 "\n"
                 "--- a/file.txt\n"
@@ -152,6 +185,14 @@ class TestGitApplyTree(TestCase):
             with open(patches[0]) as f:
                 self.assertIn(expected, f.read())
 
+    def assert_no_note(self, repo):
+        with self.assertRaises(oe.patch.CmdError):
+            oe.patch.runcmd(
+                ["git", "notes", "--ref", oe.patch.GitApplyTree.notes_ref,
+                 "show", "HEAD"],
+                repo,
+            )
+
     def test_git_am_preserves_original_patch_name(self):
         with tempfile.TemporaryDirectory(prefix="oe-gitapply-am-") as tmpdir:
             patchname = "0001-distinct-original-name.patch"
@@ -166,6 +207,51 @@ class TestGitApplyTree(TestCase):
                 self.assertEqual(f.read(), "git am change\n")
             self.assert_note_and_extract(repo, patchname, "+git am change")
 
+    def test_push_run_false_returns_argv(self):
+        with tempfile.TemporaryDirectory(prefix="oe-gitapply-run-false-") as tmpdir:
+            patchname = "0001-distinct original name.patch"
+            patch = self.make_git_am_patch(tmpdir, patchname)
+            repo = self.make_repo(tmpdir, "target")
+            tree = RecordingGitApplyTree(repo, PatchTestDataStore(tmpdir))
+            tree._need_dirty_check = lambda: False
+            tree.Import({"file": patch, "strippath": "1"}, False)
+
+            cmd = tree.Push(False, run=False)
+
+            self.assertIsInstance(cmd, list)
+            self.assertEqual(cmd[0], "git")
+            self.assertIn("am", cmd)
+            self.assertEqual(cmd[-1], patch)
+            self.assertFalse(tree.commitpatch_called)
+            self.assertIsNone(tree.current())
+            self.assert_no_note(repo)
+            with open(os.path.join(repo, "file.txt")) as f:
+                self.assertEqual(f.read(), "base\n")
+
+    def test_dirty_push_run_false_returns_argv(self):
+        with tempfile.TemporaryDirectory(prefix="oe-gitapply-run-false-") as tmpdir:
+            patchname = "plain-diff original name.patch"
+            patch = self.make_plain_diff_patch(tmpdir, patchname)
+            repo = self.make_repo(tmpdir, "target")
+            with open(os.path.join(repo, "file.txt"), "a") as f:
+                f.write("dirty\n")
+
+            tree = RecordingGitApplyTree(repo, PatchTestDataStore(tmpdir))
+            tree._need_dirty_check = lambda: True
+            tree.Import({"file": patch, "strippath": "1"}, False)
+
+            cmd = tree.Push(False, run=False)
+
+            self.assertEqual(cmd[:5], [
+                "patch", "--no-backup-if-mismatch", "-p", "1", "-i",
+            ])
+            self.assertEqual(cmd[-1], patch)
+            self.assertFalse(tree.commitpatch_called)
+            self.assertIsNone(tree.current())
+            self.assert_no_note(repo)
+            with open(os.path.join(repo, "file.txt")) as f:
+                self.assertEqual(f.read(), "base\ndirty\n")
+
     def test_fallback_preserves_original_patch_name(self):
         with tempfile.TemporaryDirectory(prefix="oe-gitapply-fallback-") as tmpdir:
             patchname = "plain-diff-original-name.patch"
@@ -178,4 +264,19 @@ class TestGitApplyTree(TestCase):
             self.assertTrue(tree.commitpatch_called)
             with open(os.path.join(repo, "file.txt")) as f:
                 self.assertEqual(f.read(), "plain diff change\n")
+            metadata = oe.patch.runcmd([
+                "git", "show", "-s",
+                "--format=%an%n%ae%n%cn%n%ce%n%aI",
+                "HEAD",
+            ], repo).splitlines()
+            self.assertEqual(metadata[:4], [
+                "Fallback Author",
+                "fallback.author@example.com",
+                "OE Test",
+                "oe-test@example.com",
+            ])
+            self.assertIn(metadata[4], (
+                "2021-01-01T12:34:56+00:00",
+                "2021-01-01T12:34:56Z",
+            ))
             self.assert_note_and_extract(repo, patchname, "+plain diff change")