| Message ID | 20250826134134.2164051-1-olani@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | sstate.bbclass: Open file with context manager | expand |
Hi, On 8/26/25 3:41 PM, Ola x Nilsson via lists.openembedded.org wrote: > From: Ola x Nilsson <olani@axis.com> > > In sstat_install and sstate_clean_cache. > > Signed-off-by: Ola x Nilsson <olani@axis.com> > --- > meta/classes-global/sstate.bbclass | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass > index e3d6373b3f..2fd29d7323 100644 > --- a/meta/classes-global/sstate.bbclass > +++ b/meta/classes-global/sstate.bbclass > @@ -306,18 +306,17 @@ def sstate_install(ss, d): > sharedfiles.append(ss['fixmedir'] + "/fixmepath") > > # Write out the manifest > - f = open(manifest, "w") > - for file in sharedfiles: > - f.write(file + "\n") > + with open(manifest, "w") as f: > + for file in sharedfiles: > + f.write(file + "\n") > Random pass-by comment. I'm wondering if we shouldn't simply create a big string and write it at once instead of having a write for each entry in the sharefiles. E.g. something like: ``` dirs = sorted(shareddirs, key=len) with open(manifest, "w") as f: if sharedfiles: f.write("\n".join(sharedfiles) + "\n") if dirs: f.write("\n".join(reversed(dirs)) + "\n") ``` It should limit the number of writes to the file but not sure how big sharefiles or dirs can be, which means a VERY big temporary string potentially. Since it's in the hot path, would need to run some benchmarks (what? how?) to know if it actually improves things or not. Cheers, Quentin
diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass index e3d6373b3f..2fd29d7323 100644 --- a/meta/classes-global/sstate.bbclass +++ b/meta/classes-global/sstate.bbclass @@ -306,18 +306,17 @@ def sstate_install(ss, d): sharedfiles.append(ss['fixmedir'] + "/fixmepath") # Write out the manifest - f = open(manifest, "w") - for file in sharedfiles: - f.write(file + "\n") + with open(manifest, "w") as f: + for file in sharedfiles: + f.write(file + "\n") - # We want to ensure that directories appear at the end of the manifest - # so that when we test to see if they should be deleted any contents - # added by the task will have been removed first. - dirs = sorted(shareddirs, key=len) - # Must remove children first, which will have a longer path than the parent - for di in reversed(dirs): - f.write(di + "\n") - f.close() + # We want to ensure that directories appear at the end of the manifest + # so that when we test to see if they should be deleted any contents + # added by the task will have been removed first. + dirs = sorted(shareddirs, key=len) + # Must remove children first, which will have a longer path than the parent + for di in reversed(dirs): + f.write(di + "\n") # Append to the list of manifests for this PACKAGE_ARCH @@ -481,9 +480,8 @@ def sstate_clean_cachefiles(d): def sstate_clean_manifest(manifest, d, canrace=False, prefix=None): import oe.path - mfile = open(manifest) - entries = mfile.readlines() - mfile.close() + with open(manifest) as mfile: + entries = mfile.readlines() for entry in entries: entry = entry.strip()