[PATCH 0/2] package: replace copydebugsources shell pipelines with Popen

[PATCH 0/2] package: replace copydebugsources shell pipelines with Popen

[Anders Heimer]

Continue the OE-Core shell=True cleanup in oe.package.

Replace the copydebugsources() shell pipelines with explicit Popen
chains using argv lists, env= for LC_ALL and cwd= for cpio. Also replace
the externalsrc mv shell glob with glob.glob(glob.escape(...)) so
metacharacters in the directory path are handled literally.

The first copy pipeline keeps the previous failure-tolerant behavior,
while the symlink fixup pipeline now checks each stage directly. Add
oeqa tests for normal copying, symlink dereferencing, multiple
-ffile-prefix-map entries, ignored source paths, and externalsrc
relocation.

Anders Heimer (2):
  package: replace copydebugsources shell pipelines with Popen
  oeqa/oelib: add copydebugsources tests

 meta/lib/oe/package.py                        |  63 +++--
 meta/lib/oeqa/selftest/cases/oelib/package.py | 220 ++++++++++++++++++
 2 files changed, 265 insertions(+), 18 deletions(-)
 create mode 100644 meta/lib/oeqa/selftest/cases/oelib/package.py

[PATCH 1/2] package: replace copydebugsources shell pipelines with Popen

[Anders Heimer]

Convert the copydebugsources command pipelines to explicit Popen calls
using argument lists. Use env= for LC_ALL, cwd= for the cpio working
directory and glob.glob() for the externalsrc move.

The first pipeline keeps ignoring command failures as before since some
inputs are expected to fail. The symlink fixup pipeline checks each stage
so failures are reported directly.

Skip the externalsrc mv when the glob has no matches and let the
following empty-directory cleanup handle the empty tree.

Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oe/package.py | 63 ++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index c375acc124..ad4b7a2769 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -1017,26 +1017,50 @@ def copydebugsources(debugsrcdir, sources, d):
         bb.utils.mkdirhier(basepath)
         cpath.updatecache(basepath)
 
-        for pmap in prefixmap:
+        env = os.environ.copy()
+        env["LC_ALL"] = "C"
+
+        for pmap, prefix in prefixmap.items():
+            dstroot = dvar + prefix
             # Ignore files from the recipe sysroots (target and native)
-            cmd =  "LC_ALL=C ; sort -z -u '%s' | egrep -v -z '((<internal>|<built-in>)$|/.*recipe-sysroot.*/)' | " % sourcefile
+            sort_p = subprocess.Popen(["sort", "-z", "-u", "--", sourcefile], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
+            egrep_p = subprocess.Popen(["egrep", "-v", "-z", "-e", r"((<internal>|<built-in>)$|/.*recipe-sysroot.*/)"], stdin=sort_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
+            sort_p.stdout.close()
+
             # We need to ignore files that are not actually ours
             # we do this by only paying attention to items from this package
-            cmd += "fgrep -zw '%s' | " % prefixmap[pmap]
+            fgrep_p = subprocess.Popen(["fgrep", "-zw", "-e", prefix], stdin=egrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
+            egrep_p.stdout.close()
+
             # Remove prefix in the source paths
-            cmd += "sed 's#%s/##g' | " % (prefixmap[pmap])
-            cmd += "(cd '%s' ; cpio -pd0mlLu --no-preserve-owner '%s%s' 2>/dev/null)" % (pmap, dvar, prefixmap[pmap])
+            sed_p = subprocess.Popen(["sed", "s#%s/##g" % prefix], stdin=fgrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
+            fgrep_p.stdout.close()
+
+            cpio_p = subprocess.Popen(["cpio", "-pd0mlLu", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, cwd=pmap, stderr=subprocess.DEVNULL, env=env)
+            sed_p.stdout.close()
+
+            for proc in (cpio_p, sed_p, fgrep_p, egrep_p, sort_p):
+                proc.wait()
 
-            try:
-                subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
-            except subprocess.CalledProcessError:
-                # Can "fail" if internal headers/transient sources are attempted
-                pass
             # cpio seems to have a bug with -lL together and symbolic links are just copied, not dereferenced.
             # Work around this by manually finding and copying any symbolic links that made it through.
-            cmd = "find %s%s -type l -print0 -delete | sed s#%s%s/##g | (cd '%s' ; cpio -pd0mL --no-preserve-owner '%s%s')" % \
-                    (dvar, prefixmap[pmap], dvar, prefixmap[pmap], pmap, dvar, prefixmap[pmap])
-            subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
+            # The source copy pipeline above can fail without aborting, so there may be no copied tree to scan for symlinks.
+            if not os.path.exists(dstroot):
+                continue
+
+            find_p = subprocess.Popen(["find", dstroot, "-type", "l", "-print0", "-delete"], stdout=subprocess.PIPE)
+            sed_p = subprocess.Popen(["sed", "s#%s/##g" % dstroot], stdin=find_p.stdout, stdout=subprocess.PIPE)
+            find_p.stdout.close()
+
+            cpio_p = subprocess.Popen(["cpio", "-pd0mL", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, stderr=subprocess.DEVNULL, cwd=pmap)
+            sed_p.stdout.close()
+
+            procs = (cpio_p, sed_p, find_p)
+            for proc in procs:
+                proc.wait()
+            for proc in procs:
+                if proc.returncode:
+                    raise subprocess.CalledProcessError(proc.returncode, proc.args)
 
         # debugsources.list may be polluted from the host if we used externalsrc,
         # cpio uses copy-pass and may have just created a directory structure
@@ -1046,13 +1070,16 @@ def copydebugsources(debugsrcdir, sources, d):
 
         # Same check as above for externalsrc
         if workdir not in sdir:
-            if os.path.exists(dvar + debugsrcdir + sdir):
-                cmd = "mv %s%s%s/* %s%s" % (dvar, debugsrcdir, sdir, dvar,debugsrcdir)
-                subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
+            srcdir = dvar + debugsrcdir + sdir
+            dstdir = dvar + debugsrcdir
+            if os.path.exists(srcdir):
+                entries = glob.glob(os.path.join(glob.escape(srcdir), "*"))
+                if entries:
+                    subprocess.check_output(["mv", "--"] + entries + [dstdir], stderr=subprocess.STDOUT)
 
         # The copy by cpio may have resulted in some empty directories!  Remove these
-        cmd = "find %s%s -empty -type d -delete" % (dvar, debugsrcdir)
-        subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
+        cmd = ["find", dvar + debugsrcdir, "-empty", "-type", "d", "-delete"]
+        subprocess.check_output(cmd, stderr=subprocess.STDOUT)
 
         # Also remove debugsrcdir if its empty
         for p in nosuchdir[::-1]:

[PATCH 2/2] oeqa/oelib: add copydebugsources tests

[Anders Heimer]

Cover debug source copying, ignored source paths and externalsrc
relocation.

AI-Generated: Claude Opus 4.6

Signed-off-by: Anders Heimer <anders.heimer@est.tech>
---
 meta/lib/oeqa/selftest/cases/oelib/package.py | 239 ++++++++++++++++++
 1 file changed, 239 insertions(+)
 create mode 100644 meta/lib/oeqa/selftest/cases/oelib/package.py

diff --git a/meta/lib/oeqa/selftest/cases/oelib/package.py b/meta/lib/oeqa/selftest/cases/oelib/package.py
new file mode 100644
index 0000000000..39c7d0661d
--- /dev/null
+++ b/meta/lib/oeqa/selftest/cases/oelib/package.py
@@ -0,0 +1,239 @@
+#
+# Copyright OpenEmbedded Contributors
+#
+# SPDX-License-Identifier: MIT
+#
+
+import os
+import shutil
+import tempfile
+from unittest.case import TestCase
+
+class FakeDataStore:
+    def __init__(self, values):
+        self.values = values
+
+    def getVar(self, name):
+        return self.values.get(name)
+
+    def expand(self, value):
+        for name, replacement in self.values.items():
+            value = value.replace("${%s}" % name, replacement)
+        return value
+
+class TestCopyDebugSources(TestCase):
+    def setUp(self):
+        for tool in ("sort", "egrep", "fgrep", "sed", "cpio", "find", "mv"):
+            if shutil.which(tool) is None:
+                self.skipTest("Required tool %s not found" % tool)
+
+    def copydebugsources(self, debugsrcdir, sources, d):
+        try:
+            import oe.package
+        except ImportError:
+            self.skipTest("Cannot import oe.package")
+
+        oe.package.copydebugsources(debugsrcdir, sources, d)
+
+    def test_copydebugsources_copies_files_and_dereferences_links(self):
+        with tempfile.TemporaryDirectory(prefix="oe-test-package-") as tmpdir:
+            source_root = os.path.join(tmpdir, "source")
+            second_source_root = os.path.join(tmpdir, "second-source")
+            workdir = os.path.join(tmpdir, "work")
+            pkgd = os.path.join(tmpdir, "pkgd")
+            debugsrcdir = "/usr/src/debug/testpkg/1.0"
+            second_debugsrcdir = "/usr/src/debug/secondpkg/1.0"
+
+            os.makedirs(os.path.join(source_root, "nested"))
+            os.makedirs(second_source_root)
+            os.makedirs(workdir)
+            os.makedirs(pkgd)
+
+            normal = os.path.join(source_root, "nested", "normal.c")
+            special = os.path.join(source_root, "nested", "name with ; spaces.c")
+            leading_dash = os.path.join(source_root, "nested", "-leading-dash.c")
+            target = os.path.join(source_root, "nested", "target.c")
+            link = os.path.join(source_root, "nested", "link.c")
+            ignored_src = os.path.join(source_root, "recipe-sysroot", "ignored.c")
+            second = os.path.join(second_source_root, "second.c")
+
+            with open(normal, "w") as f:
+                f.write("normal\n")
+            with open(special, "w") as f:
+                f.write("special\n")
+            with open(leading_dash, "w") as f:
+                f.write("leading dash\n")
+            with open(target, "w") as f:
+                f.write("target\n")
+            os.symlink("target.c", link)
+            os.makedirs(os.path.dirname(ignored_src))
+            with open(ignored_src, "w") as f:
+                f.write("ignored\n")
+            with open(second, "w") as f:
+                f.write("second\n")
+
+            sources = [
+                os.path.join(debugsrcdir, "nested", "normal.c"),
+                os.path.join(debugsrcdir, "nested", "name with ; spaces.c"),
+                os.path.join(debugsrcdir, "nested", "-leading-dash.c"),
+                os.path.join(debugsrcdir, "nested", "link.c"),
+                os.path.join(debugsrcdir, "recipe-sysroot", "ignored.c"),
+                os.path.join(second_debugsrcdir, "second.c"),
+                "<internal>",
+            ]
+            d = FakeDataStore({
+                "WORKDIR": workdir,
+                "PKGD": pkgd,
+                "STRIP": "strip",
+                "OBJCOPY": "objcopy",
+                "S": os.path.join(workdir, "source"),
+                "CFLAGS": (
+                    "-ffile-prefix-map=%s=%s "
+                    "-ffile-prefix-map=%s=%s"
+                ) % (
+                    source_root,
+                    debugsrcdir,
+                    second_source_root,
+                    second_debugsrcdir,
+                ),
+            })
+
+            self.copydebugsources(debugsrcdir, sources, d)
+
+            copied_normal = os.path.join(pkgd, debugsrcdir.lstrip("/"),
+                                         "nested", "normal.c")
+            copied_special = os.path.join(pkgd, debugsrcdir.lstrip("/"),
+                                          "nested", "name with ; spaces.c")
+            copied_leading_dash = os.path.join(pkgd, debugsrcdir.lstrip("/"),
+                                               "nested", "-leading-dash.c")
+            copied_link = os.path.join(pkgd, debugsrcdir.lstrip("/"),
+                                       "nested", "link.c")
+            copied_second = os.path.join(pkgd, second_debugsrcdir.lstrip("/"),
+                                         "second.c")
+            ignored = os.path.join(pkgd, debugsrcdir.lstrip("/"),
+                                   "recipe-sysroot", "ignored.c")
+
+            with open(copied_normal) as f:
+                self.assertEqual(f.read(), "normal\n")
+            with open(copied_special) as f:
+                self.assertEqual(f.read(), "special\n")
+            with open(copied_leading_dash) as f:
+                self.assertEqual(f.read(), "leading dash\n")
+            with open(copied_link) as f:
+                self.assertEqual(f.read(), "target\n")
+            with open(copied_second) as f:
+                self.assertEqual(f.read(), "second\n")
+            self.assertFalse(os.path.islink(copied_link))
+            self.assertFalse(os.path.exists(ignored))
+
+    def test_copydebugsources_ignores_missing_destroot(self):
+        with tempfile.TemporaryDirectory(prefix="oe-test-package-") as tmpdir:
+            source_root = os.path.join(tmpdir, "source")
+            workdir = os.path.join(tmpdir, "work")
+            pkgd = os.path.join(tmpdir, "pkgd")
+            debugsrcdir = "/usr/src/debug/testpkg/1.0"
+            mapped_debugsrcdir = os.path.join(debugsrcdir, "mapped")
+
+            os.makedirs(source_root)
+            os.makedirs(workdir)
+            os.makedirs(pkgd)
+
+            sources = [
+                os.path.join(mapped_debugsrcdir, "recipe-sysroot", "ignored.c"),
+                "<internal>",
+                "<built-in>",
+            ]
+            d = FakeDataStore({
+                "WORKDIR": workdir,
+                "PKGD": pkgd,
+                "STRIP": "strip",
+                "OBJCOPY": "objcopy",
+                "S": os.path.join(workdir, "source"),
+                "CFLAGS": "-ffile-prefix-map=%s=%s" % (
+                    source_root,
+                    mapped_debugsrcdir,
+                ),
+            })
+
+            self.copydebugsources(debugsrcdir, sources, d)
+
+            self.assertFalse(os.path.exists(pkgd + mapped_debugsrcdir))
+            self.assertFalse(os.path.exists(pkgd + debugsrcdir))
+
+    def test_copydebugsources_moves_externalsrc_relocation(self):
+        with tempfile.TemporaryDirectory(prefix="oe-test-package-") as tmpdir:
+            source_root = os.path.join(tmpdir, "external-[source]")
+            workdir = os.path.join(tmpdir, "work")
+            pkgd = os.path.join(tmpdir, "pkgd")
+            debugsrcdir = "/usr/src/debug/testpkg/1.0"
+
+            os.makedirs(source_root)
+            os.makedirs(workdir)
+            os.makedirs(pkgd)
+
+            source = os.path.join(source_root, "real.c")
+            with open(source, "w") as f:
+                f.write("real\n")
+
+            relocation = pkgd + debugsrcdir + source_root
+            relocated_name = "-relocated with ; spaces.c"
+            os.makedirs(relocation)
+            with open(os.path.join(relocation, relocated_name), "w") as f:
+                f.write("relocated\n")
+
+            sources = [os.path.join(debugsrcdir, "real.c")]
+            d = FakeDataStore({
+                "WORKDIR": workdir,
+                "PKGD": pkgd,
+                "STRIP": "strip",
+                "OBJCOPY": "objcopy",
+                "S": source_root,
+                "CFLAGS": "-ffile-prefix-map=%s=%s" % (source_root, debugsrcdir),
+            })
+
+            self.copydebugsources(debugsrcdir, sources, d)
+
+            copied_source = os.path.join(pkgd, debugsrcdir.lstrip("/"), "real.c")
+            moved_source = os.path.join(pkgd, debugsrcdir.lstrip("/"), relocated_name)
+
+            with open(copied_source) as f:
+                self.assertEqual(f.read(), "real\n")
+            with open(moved_source) as f:
+                self.assertEqual(f.read(), "relocated\n")
+            self.assertFalse(os.path.exists(relocation))
+
+    def test_copydebugsources_ignores_empty_externalsrc_relocation(self):
+        with tempfile.TemporaryDirectory(prefix="oe-test-package-") as tmpdir:
+            source_root = os.path.join(tmpdir, "external-source")
+            workdir = os.path.join(tmpdir, "work")
+            pkgd = os.path.join(tmpdir, "pkgd")
+            debugsrcdir = "/usr/src/debug/testpkg/1.0"
+
+            os.makedirs(source_root)
+            os.makedirs(workdir)
+            os.makedirs(pkgd)
+
+            source = os.path.join(source_root, "real.c")
+            with open(source, "w") as f:
+                f.write("real\n")
+
+            relocation = pkgd + debugsrcdir + source_root
+            os.makedirs(relocation)
+
+            sources = [os.path.join(debugsrcdir, "real.c")]
+            d = FakeDataStore({
+                "WORKDIR": workdir,
+                "PKGD": pkgd,
+                "STRIP": "strip",
+                "OBJCOPY": "objcopy",
+                "S": source_root,
+                "CFLAGS": "-ffile-prefix-map=%s=%s" % (source_root, debugsrcdir),
+            })
+
+            self.copydebugsources(debugsrcdir, sources, d)
+
+            copied_source = os.path.join(pkgd, debugsrcdir.lstrip("/"), "real.c")
+
+            with open(copied_source) as f:
+                self.assertEqual(f.read(), "real\n")
+            self.assertFalse(os.path.exists(relocation))

Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen

[Paul Barker]

[-- Attachment #1: Type: text/plain, Size: 6715 bytes --]

On Tue, 2026-06-16 at 10:25 +0200, Anders Heimer wrote:
> Convert the copydebugsources command pipelines to explicit Popen calls
> using argument lists. Use env= for LC_ALL, cwd= for the cpio working
> directory and glob.glob() for the externalsrc move.
> 
> The first pipeline keeps ignoring command failures as before since some
> inputs are expected to fail. The symlink fixup pipeline checks each stage
> so failures are reported directly.
> 
> Skip the externalsrc mv when the glob has no matches and let the
> following empty-directory cleanup handle the empty tree.
> 
> Signed-off-by: Anders Heimer <anders.heimer@est.tech>
> ---
>  meta/lib/oe/package.py | 63 ++++++++++++++++++++++++++++++------------
>  1 file changed, 45 insertions(+), 18 deletions(-)
> 
> diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
> index c375acc124..ad4b7a2769 100644
> --- a/meta/lib/oe/package.py
> +++ b/meta/lib/oe/package.py
> @@ -1017,26 +1017,50 @@ def copydebugsources(debugsrcdir, sources, d):
>          bb.utils.mkdirhier(basepath)
>          cpath.updatecache(basepath)
>  
> -        for pmap in prefixmap:
> +        env = os.environ.copy()
> +        env["LC_ALL"] = "C"
> +
> +        for pmap, prefix in prefixmap.items():
> +            dstroot = dvar + prefix
>              # Ignore files from the recipe sysroots (target and native)
> -            cmd =  "LC_ALL=C ; sort -z -u '%s' | egrep -v -z '((<internal>|<built-in>)$|/.*recipe-sysroot.*/)' | " % sourcefile
> +            sort_p = subprocess.Popen(["sort", "-z", "-u", "--", sourcefile], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> +            egrep_p = subprocess.Popen(["egrep", "-v", "-z", "-e", r"((<internal>|<built-in>)$|/.*recipe-sysroot.*/)"], stdin=sort_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> +            sort_p.stdout.close()
> +
>              # We need to ignore files that are not actually ours
>              # we do this by only paying attention to items from this package
> -            cmd += "fgrep -zw '%s' | " % prefixmap[pmap]
> +            fgrep_p = subprocess.Popen(["fgrep", "-zw", "-e", prefix], stdin=egrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> +            egrep_p.stdout.close()
> +
>              # Remove prefix in the source paths
> -            cmd += "sed 's#%s/##g' | " % (prefixmap[pmap])
> -            cmd += "(cd '%s' ; cpio -pd0mlLu --no-preserve-owner '%s%s' 2>/dev/null)" % (pmap, dvar, prefixmap[pmap])
> +            sed_p = subprocess.Popen(["sed", "s#%s/##g" % prefix], stdin=fgrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> +            fgrep_p.stdout.close()
> +
> +            cpio_p = subprocess.Popen(["cpio", "-pd0mlLu", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, cwd=pmap, stderr=subprocess.DEVNULL, env=env)
> +            sed_p.stdout.close()
> +
> +            for proc in (cpio_p, sed_p, fgrep_p, egrep_p, sort_p):
> +                proc.wait()

Hi Anders, thanks for the patches!

If we're reworking this code, I think we should replace the complex
sed/grep/sort pipeline with Python code. We can read into a Python list
and sort/filter using the Python standard library, then pass the results
to cpio.

>  
> -            try:
> -                subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
> -            except subprocess.CalledProcessError:
> -                # Can "fail" if internal headers/transient sources are attempted
> -                pass
>              # cpio seems to have a bug with -lL together and symbolic links are just copied, not dereferenced.
>              # Work around this by manually finding and copying any symbolic links that made it through.
> -            cmd = "find %s%s -type l -print0 -delete | sed s#%s%s/##g | (cd '%s' ; cpio -pd0mL --no-preserve-owner '%s%s')" % \
> -                    (dvar, prefixmap[pmap], dvar, prefixmap[pmap], pmap, dvar, prefixmap[pmap])
> -            subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
> +            # The source copy pipeline above can fail without aborting, so there may be no copied tree to scan for symlinks.
> +            if not os.path.exists(dstroot):
> +                continue
> +
> +            find_p = subprocess.Popen(["find", dstroot, "-type", "l", "-print0", "-delete"], stdout=subprocess.PIPE)
> +            sed_p = subprocess.Popen(["sed", "s#%s/##g" % dstroot], stdin=find_p.stdout, stdout=subprocess.PIPE)
> +            find_p.stdout.close()
> +
> +            cpio_p = subprocess.Popen(["cpio", "-pd0mL", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, stderr=subprocess.DEVNULL, cwd=pmap)
> +            sed_p.stdout.close()
> +
> +            procs = (cpio_p, sed_p, find_p)
> +            for proc in procs:
> +                proc.wait()
> +            for proc in procs:
> +                if proc.returncode:
> +                    raise subprocess.CalledProcessError(proc.returncode, proc.args)

This is a simpler pipeline but it may still be nicer to bring it into
Python code.

>  
>          # debugsources.list may be polluted from the host if we used externalsrc,
>          # cpio uses copy-pass and may have just created a directory structure
> @@ -1046,13 +1070,16 @@ def copydebugsources(debugsrcdir, sources, d):
>  
>          # Same check as above for externalsrc
>          if workdir not in sdir:
> -            if os.path.exists(dvar + debugsrcdir + sdir):
> -                cmd = "mv %s%s%s/* %s%s" % (dvar, debugsrcdir, sdir, dvar,debugsrcdir)
> -                subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
> +            srcdir = dvar + debugsrcdir + sdir
> +            dstdir = dvar + debugsrcdir
> +            if os.path.exists(srcdir):
> +                entries = glob.glob(os.path.join(glob.escape(srcdir), "*"))
> +                if entries:
> +                    subprocess.check_output(["mv", "--"] + entries + [dstdir], stderr=subprocess.STDOUT)

We could simply use shutil.move().

>  
>          # The copy by cpio may have resulted in some empty directories!  Remove these
> -        cmd = "find %s%s -empty -type d -delete" % (dvar, debugsrcdir)
> -        subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
> +        cmd = ["find", dvar + debugsrcdir, "-empty", "-type", "d", "-delete"]
> +        subprocess.check_output(cmd, stderr=subprocess.STDOUT)

I think this is would be more complex in Python code so it is probably
best left as a find command.

Best regards,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen

[Anders Heimer]

Hi Paul,

On 6/16/26 14:12, Paul Barker wrote:
> On Tue, 2026-06-16 at 10:25 +0200, Anders Heimer wrote:
>> -        for pmap in prefixmap:
>> +        env = os.environ.copy()
>> +        env["LC_ALL"] = "C"
>> +
>> +        for pmap, prefix in prefixmap.items():
>> +            dstroot = dvar + prefix
>>               # Ignore files from the recipe sysroots (target and native)
>> -            cmd =  "LC_ALL=C ; sort -z -u '%s' | egrep -v -z '((<internal>|<built-in>)$|/.*recipe-sysroot.*/)' | " % sourcefile
>> +            sort_p = subprocess.Popen(["sort", "-z", "-u", "--", sourcefile], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>> +            egrep_p = subprocess.Popen(["egrep", "-v", "-z", "-e", r"((<internal>|<built-in>)$|/.*recipe-sysroot.*/)"], stdin=sort_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>> +            sort_p.stdout.close()
>> +
>>               # We need to ignore files that are not actually ours
>>               # we do this by only paying attention to items from this package
>> -            cmd += "fgrep -zw '%s' | " % prefixmap[pmap]
>> +            fgrep_p = subprocess.Popen(["fgrep", "-zw", "-e", prefix], stdin=egrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>> +            egrep_p.stdout.close()
>> +
>>               # Remove prefix in the source paths
>> -            cmd += "sed 's#%s/##g' | " % (prefixmap[pmap])
>> -            cmd += "(cd '%s' ; cpio -pd0mlLu --no-preserve-owner '%s%s' 2>/dev/null)" % (pmap, dvar, prefixmap[pmap])
>> +            sed_p = subprocess.Popen(["sed", "s#%s/##g" % prefix], stdin=fgrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>> +            fgrep_p.stdout.close()
>> +
>> +            cpio_p = subprocess.Popen(["cpio", "-pd0mlLu", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, cwd=pmap, stderr=subprocess.DEVNULL, env=env)
>> +            sed_p.stdout.close()
>> +
>> +            for proc in (cpio_p, sed_p, fgrep_p, egrep_p, sort_p):
>> +                proc.wait()
> Hi Anders, thanks for the patches!
>
> If we're reworking this code, I think we should replace the complex
> sed/grep/sort pipeline with Python code. We can read into a Python list
> and sort/filter using the Python standard library, then pass the results
> to cpio.

Thank you,  I am very happy to implement this approach instead. I 
strongly agree with all your comments.

/Anders

Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen

[Paul Barker]

[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]

On Tue, 2026-06-16 at 15:35 +0200, Anders Heimer via
lists.openembedded.org wrote:
> Hi Paul,
> 
> On 6/16/26 14:12, Paul Barker wrote:
> > On Tue, 2026-06-16 at 10:25 +0200, Anders Heimer wrote:
> > > -        for pmap in prefixmap:
> > > +        env = os.environ.copy()
> > > +        env["LC_ALL"] = "C"
> > > +
> > > +        for pmap, prefix in prefixmap.items():
> > > +            dstroot = dvar + prefix
> > >               # Ignore files from the recipe sysroots (target and native)
> > > -            cmd =  "LC_ALL=C ; sort -z -u '%s' | egrep -v -z '((<internal>|<built-in>)$|/.*recipe-sysroot.*/)' | " % sourcefile
> > > +            sort_p = subprocess.Popen(["sort", "-z", "-u", "--", sourcefile], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            egrep_p = subprocess.Popen(["egrep", "-v", "-z", "-e", r"((<internal>|<built-in>)$|/.*recipe-sysroot.*/)"], stdin=sort_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            sort_p.stdout.close()
> > > +
> > >               # We need to ignore files that are not actually ours
> > >               # we do this by only paying attention to items from this package
> > > -            cmd += "fgrep -zw '%s' | " % prefixmap[pmap]
> > > +            fgrep_p = subprocess.Popen(["fgrep", "-zw", "-e", prefix], stdin=egrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            egrep_p.stdout.close()
> > > +
> > >               # Remove prefix in the source paths
> > > -            cmd += "sed 's#%s/##g' | " % (prefixmap[pmap])
> > > -            cmd += "(cd '%s' ; cpio -pd0mlLu --no-preserve-owner '%s%s' 2>/dev/null)" % (pmap, dvar, prefixmap[pmap])
> > > +            sed_p = subprocess.Popen(["sed", "s#%s/##g" % prefix], stdin=fgrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            fgrep_p.stdout.close()
> > > +
> > > +            cpio_p = subprocess.Popen(["cpio", "-pd0mlLu", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, cwd=pmap, stderr=subprocess.DEVNULL, env=env)
> > > +            sed_p.stdout.close()
> > > +
> > > +            for proc in (cpio_p, sed_p, fgrep_p, egrep_p, sort_p):
> > > +                proc.wait()
> > Hi Anders, thanks for the patches!
> > 
> > If we're reworking this code, I think we should replace the complex
> > sed/grep/sort pipeline with Python code. We can read into a Python list
> > and sort/filter using the Python standard library, then pass the results
> > to cpio.
> 
> Thank you,  I am very happy to implement this approach instead. I 
> strongly agree with all your comments.

Hi Anders,

I was discussing this with Richard just now, I may be a bit over eager!

This code is performance sensitive as there's sometimes a lot of debug
sources to copy. We've seen previously that shutil.copy() is much slower
than shelling out to `mv`, so we probably want to keep that one as a
subprocess. The rest of the cleanup I suggested is worth doing.

Thanks,

-- 
Paul Barker


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen

[Richard Purdie]

On Tue, 2026-06-16 at 15:35 +0200, Anders Heimer via lists.openembedded.org wrote:
> On 6/16/26 14:12, Paul Barker wrote:
> > On Tue, 2026-06-16 at 10:25 +0200, Anders Heimer wrote:
> > > -        for pmap in prefixmap:
> > > +        env = os.environ.copy()
> > > +        env["LC_ALL"] = "C"
> > > +
> > > +        for pmap, prefix in prefixmap.items():
> > > +            dstroot = dvar + prefix
> > >               # Ignore files from the recipe sysroots (target and native)
> > > -            cmd =  "LC_ALL=C ; sort -z -u '%s' | egrep -v -z '((<internal>|<built-in>)$|/.*recipe-sysroot.*/)' | " % sourcefile
> > > +            sort_p = subprocess.Popen(["sort", "-z", "-u", "--", sourcefile], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            egrep_p = subprocess.Popen(["egrep", "-v", "-z", "-e", r"((<internal>|<built-in>)$|/.*recipe-sysroot.*/)"], stdin=sort_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            sort_p.stdout.close()
> > > +
> > >               # We need to ignore files that are not actually ours
> > >               # we do this by only paying attention to items from this package
> > > -            cmd += "fgrep -zw '%s' | " % prefixmap[pmap]
> > > +            fgrep_p = subprocess.Popen(["fgrep", "-zw", "-e", prefix], stdin=egrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            egrep_p.stdout.close()
> > > +
> > >               # Remove prefix in the source paths
> > > -            cmd += "sed 's#%s/##g' | " % (prefixmap[pmap])
> > > -            cmd += "(cd '%s' ; cpio -pd0mlLu --no-preserve-owner '%s%s' 2>/dev/null)" % (pmap, dvar, prefixmap[pmap])
> > > +            sed_p = subprocess.Popen(["sed", "s#%s/##g" % prefix], stdin=fgrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
> > > +            fgrep_p.stdout.close()
> > > +
> > > +            cpio_p = subprocess.Popen(["cpio", "-pd0mlLu", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, cwd=pmap, stderr=subprocess.DEVNULL, env=env)
> > > +            sed_p.stdout.close()
> > > +
> > > +            for proc in (cpio_p, sed_p, fgrep_p, egrep_p, sort_p):
> > > +                proc.wait()
> > Hi Anders, thanks for the patches!
> > 
> > If we're reworking this code, I think we should replace the complex
> > sed/grep/sort pipeline with Python code. We can read into a Python list
> > and sort/filter using the Python standard library, then pass the results
> > to cpio.
> 
> Thank you,  I am very happy to implement this approach instead. I 
> strongly agree with all your comments.

There are some other things to consider here. This is a fairly
sensitive area of code from a performance perspective. You could code
much of this in python using shutil for example however shutil has
traditionally been up to an order of magnitude slower. As such, this
code was optimized to be fast, hence the use of cpio.

In many cases I worry less about performance but this is one area it
does really matter and makes a big difference to build speed overall.
python can be fast if carefully written but if this used shutil for
example, it likely won't be.

Cheers,

Richard

Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen

[Anders Heimer]

[-- Attachment #1: Type: text/plain, Size: 3842 bytes --]

Hi Richard,

On 6/16/26 15:44, Richard Purdie wrote:
> On Tue, 2026-06-16 at 15:35 +0200, Anders Heimer via lists.openembedded.org wrote:
>> On 6/16/26 14:12, Paul Barker wrote:
>>> On Tue, 2026-06-16 at 10:25 +0200, Anders Heimer wrote:
>>>> -        for pmap in prefixmap:
>>>> +        env = os.environ.copy()
>>>> +        env["LC_ALL"] = "C"
>>>> +
>>>> +        for pmap, prefix in prefixmap.items():
>>>> +            dstroot = dvar + prefix
>>>>                # Ignore files from the recipe sysroots (target and native)
>>>> -            cmd =  "LC_ALL=C ; sort -z -u '%s' | egrep -v -z '((<internal>|<built-in>)$|/.*recipe-sysroot.*/)' | " % sourcefile
>>>> +            sort_p = subprocess.Popen(["sort", "-z", "-u", "--", sourcefile], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>>>> +            egrep_p = subprocess.Popen(["egrep", "-v", "-z", "-e", r"((<internal>|<built-in>)$|/.*recipe-sysroot.*/)"], stdin=sort_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>>>> +            sort_p.stdout.close()
>>>> +
>>>>                # We need to ignore files that are not actually ours
>>>>                # we do this by only paying attention to items from this package
>>>> -            cmd += "fgrep -zw '%s' | " % prefixmap[pmap]
>>>> +            fgrep_p = subprocess.Popen(["fgrep", "-zw", "-e", prefix], stdin=egrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>>>> +            egrep_p.stdout.close()
>>>> +
>>>>                # Remove prefix in the source paths
>>>> -            cmd += "sed 's#%s/##g' | " % (prefixmap[pmap])
>>>> -            cmd += "(cd '%s' ; cpio -pd0mlLu --no-preserve-owner '%s%s' 2>/dev/null)" % (pmap, dvar, prefixmap[pmap])
>>>> +            sed_p = subprocess.Popen(["sed", "s#%s/##g" % prefix], stdin=fgrep_p.stdout, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, env=env)
>>>> +            fgrep_p.stdout.close()
>>>> +
>>>> +            cpio_p = subprocess.Popen(["cpio", "-pd0mlLu", "--no-preserve-owner", dstroot], stdin=sed_p.stdout, cwd=pmap, stderr=subprocess.DEVNULL, env=env)
>>>> +            sed_p.stdout.close()
>>>> +
>>>> +            for proc in (cpio_p, sed_p, fgrep_p, egrep_p, sort_p):
>>>> +                proc.wait()
>>> Hi Anders, thanks for the patches!
>>>
>>> If we're reworking this code, I think we should replace the complex
>>> sed/grep/sort pipeline with Python code. We can read into a Python list
>>> and sort/filter using the Python standard library, then pass the results
>>> to cpio.
>> Thank you,  I am very happy to implement this approach instead. I
>> strongly agree with all your comments.
> There are some other things to consider here. This is a fairly
> sensitive area of code from a performance perspective. You could code
> much of this in python using shutil for example however shutil has
> traditionally been up to an order of magnitude slower. As such, this
> code was optimized to be fast, hence the use of cpio.
>
> In many cases I worry less about performance but this is one area it
> does really matter and makes a big difference to build speed overall.
> python can be fast if carefully written but if this used shutil for
> example, it likely won't be.
>
> Cheers,
>
> Richard
Good point. I will avoid replacing the copy step with shutil and keep 
cpio in the path. I can look at whether only the sort/filter part can 
move to Python while still feeding cpio.

I had not considered the performance sensitivity here, so I will run 
some benchmarking before proposing any larger change.

Best regards,
Anders


[-- Attachment #2: Type: text/html, Size: 6420 bytes --]