| Message ID | 20260415085824.1730569-1-michael@rndt.dev |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | sstate: Add fallback when hardlink fails | expand |
On Wed, 2026-04-15 at 10:58 +0200, Michael Arndt via lists.openembedded.org wrote: > Previously the sstate didn't work on file systems that don't support hardlinks. > For example when using WebDAV to share the sstate. This change avoids the > problem by adding a fallback in case the hardlink fails. > > Signed-off-by: Michael Arndt <michael@rndt.dev> > --- > meta/classes-global/sstate.bbclass | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) The change makes it unconditionally overwrite the file in the case of any failure. This totally breaks the point of the existing code :(. The challenge is that the sstate directory is shared, so two builds can write to the same file at the same time. Our policy is "first wins" and anything later is just skipped. The code also needs to make sure that if the sstate file is written, the paired signature matches. I appreciate the issue you're trying to solve but this isn't the right fix. Generic try/except cases are almost always a red flag in general. Cheers, Richard
Hello Richard, > The challenge is that the sstate directory is shared, so two builds can > write to the same file at the same time. Our policy is "first wins" and > anything later is just skipped. The code also needs to make sure that > if the sstate file is written, the paired signature matches. Thanks for taking a look at my change. I guess then there is no simple fix for the problem, because checking if the file exists and replacing it must be a single atomic operation. Without hardlinks that is a problem. > Generic try/except cases are almost always a red flag in general. Agreed, that was also the reason why I was surprised that my sstate didn't work. The existing code already swallows the IOException from os.link so you don't see the error about the missing hardlink support. I guess I will have to use a different file system, but thanks anyway. Greetings, Michael
On Wed, 2026-04-15 at 13:52 +0200, Michael Arndt wrote: > Hello Richard, > > > The challenge is that the sstate directory is shared, so two builds can > > write to the same file at the same time. Our policy is "first wins" and > > anything later is just skipped. The code also needs to make sure that > > if the sstate file is written, the paired signature matches. > > Thanks for taking a look at my change. I guess then there is no simple > fix for the problem, because checking if the file exists and replacing > it must be a single atomic operation. Without hardlinks that is a problem. > > > Generic try/except cases are almost always a red flag in general. > > Agreed, that was also the reason why I was surprised that my sstate > didn't work. The existing code already swallows the IOException from > os.link so you don't see the error about the missing hardlink support. > > I guess I will have to use a different file system, but thanks anyway. If you can catch a specific exception about it being not supported, we could at least show that to the user and fail. I noticed there are other generic try/except blocks in there which could probably be improved. This is pretty core/key code which we've had challenges with in the past though and it does need to work simply/reliably! Cheers, Richard
> If you can catch a specific exception about it being not supported, we > could at least show that to the user and fail. I noticed there are > other generic try/except blocks in there which could probably be > improved. Sure I know which errno the exception contains (ENOSYS), but wouldn't it make more sense to only ignore the one error that we want to ignore and fail for everything else? As I understand it we only want to ignore is the FileExistsError, correct? What would be preferred here? Greetings, Michael
diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass index 88449d19c7..2b18cafb60 100644 --- a/meta/classes-global/sstate.bbclass +++ b/meta/classes-global/sstate.bbclass @@ -795,6 +795,14 @@ python sstate_create_and_sign_package () { except: pass + # Create hardlink with fallback to rename. Useful for file systems that + # don't support hardlinks. + def hardlink(src, dst): + try: + os.link(src, dst) + except: + src.rename(dst) + def update_file(src, dst, force=False): if dst.is_symlink() and not dst.exists(): force=True @@ -804,7 +812,7 @@ python sstate_create_and_sign_package () { if force: src.rename(dst) else: - os.link(src, dst) + hardlink(src, dst) return True except: pass @@ -862,7 +870,7 @@ python sstate_create_and_sign_package () { with NamedTemporaryFile(prefix=sstate_pkg.name, dir=sstate_pkg.parent) as tmp_pkg_fd: tmp_pkg = tmp_pkg_fd.name sstate_archive_package(tmp_pkg, d) - update_file(tmp_pkg, sstate_pkg) + update_file(Path(tmp_pkg), sstate_pkg) # update_file() may have renamed tmp_pkg, which must exist when the # NamedTemporaryFile() context handler ends. touch(Path(tmp_pkg))
Previously the sstate didn't work on file systems that don't support hardlinks. For example when using WebDAV to share the sstate. This change avoids the problem by adding a fallback in case the hardlink fails. Signed-off-by: Michael Arndt <michael@rndt.dev> --- meta/classes-global/sstate.bbclass | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)