* [PATCH 0/2] package: replace copydebugsources shell pipelines with Popen @ 2026-06-16 8:25 Anders Heimer 2026-06-16 8:25 ` [PATCH 1/2] " Anders Heimer 2026-06-16 8:25 ` [PATCH 2/2] oeqa/oelib: add copydebugsources tests Anders Heimer 0 siblings, 2 replies; 8+ messages in thread From: Anders Heimer @ 2026-06-16 8:25 UTC (permalink / raw) To: openembedded-core; +Cc: 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen 2026-06-16 8:25 [PATCH 0/2] package: replace copydebugsources shell pipelines with Popen Anders Heimer @ 2026-06-16 8:25 ` Anders Heimer 2026-06-16 12:12 ` [OE-core] " Paul Barker 2026-06-16 8:25 ` [PATCH 2/2] oeqa/oelib: add copydebugsources tests Anders Heimer 1 sibling, 1 reply; 8+ messages in thread From: Anders Heimer @ 2026-06-16 8:25 UTC (permalink / raw) To: openembedded-core; +Cc: 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]: ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen 2026-06-16 8:25 ` [PATCH 1/2] " Anders Heimer @ 2026-06-16 12:12 ` Paul Barker 2026-06-16 13:35 ` Anders Heimer 0 siblings, 1 reply; 8+ messages in thread From: Paul Barker @ 2026-06-16 12:12 UTC (permalink / raw) To: Anders Heimer, openembedded-core [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen 2026-06-16 12:12 ` [OE-core] " Paul Barker @ 2026-06-16 13:35 ` Anders Heimer 2026-06-16 13:42 ` Paul Barker 2026-06-16 13:44 ` Richard Purdie 0 siblings, 2 replies; 8+ messages in thread From: Anders Heimer @ 2026-06-16 13:35 UTC (permalink / raw) To: Paul Barker, Anders Heimer, openembedded-core 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen 2026-06-16 13:35 ` Anders Heimer @ 2026-06-16 13:42 ` Paul Barker 2026-06-16 13:44 ` Richard Purdie 1 sibling, 0 replies; 8+ messages in thread From: Paul Barker @ 2026-06-16 13:42 UTC (permalink / raw) To: anders.heimer, Anders Heimer, openembedded-core; +Cc: Richard Purdie [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen 2026-06-16 13:35 ` Anders Heimer 2026-06-16 13:42 ` Paul Barker @ 2026-06-16 13:44 ` Richard Purdie 2026-06-16 14:13 ` Anders Heimer 1 sibling, 1 reply; 8+ messages in thread From: Richard Purdie @ 2026-06-16 13:44 UTC (permalink / raw) To: anders.heimer, Paul Barker, Anders Heimer, openembedded-core 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [OE-core] [PATCH 1/2] package: replace copydebugsources shell pipelines with Popen 2026-06-16 13:44 ` Richard Purdie @ 2026-06-16 14:13 ` Anders Heimer 0 siblings, 0 replies; 8+ messages in thread From: Anders Heimer @ 2026-06-16 14:13 UTC (permalink / raw) To: Richard Purdie, Paul Barker, openembedded-core [-- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] oeqa/oelib: add copydebugsources tests 2026-06-16 8:25 [PATCH 0/2] package: replace copydebugsources shell pipelines with Popen Anders Heimer 2026-06-16 8:25 ` [PATCH 1/2] " Anders Heimer @ 2026-06-16 8:25 ` Anders Heimer 1 sibling, 0 replies; 8+ messages in thread From: Anders Heimer @ 2026-06-16 8:25 UTC (permalink / raw) To: openembedded-core; +Cc: 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)) ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-06-16 14:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-16 8:25 [PATCH 0/2] package: replace copydebugsources shell pipelines with Popen Anders Heimer 2026-06-16 8:25 ` [PATCH 1/2] " Anders Heimer 2026-06-16 12:12 ` [OE-core] " Paul Barker 2026-06-16 13:35 ` Anders Heimer 2026-06-16 13:42 ` Paul Barker 2026-06-16 13:44 ` Richard Purdie 2026-06-16 14:13 ` Anders Heimer 2026-06-16 8:25 ` [PATCH 2/2] oeqa/oelib: add copydebugsources tests Anders Heimer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.