None of my images are bootable when this patch is applied, because
symlinks that busybox should provide arent created in the image. This
patch should be looked into a bit further,  how was it tested ?

On Mon, Apr 19, 2021 at 11:43 AM Devendra Tewari
<devendra.tew...@gmail.com> wrote:
>
>
>
> > On 30 Mar 2021, at 08:55, Devendra Tewari <devendra.tew...@gmail.com> wrote:
> >
> > I understand that parallel tasks can result in such issues, but would 
> > expect some kind of dependency graph that would prevent them from 
> > happening. I wish I had more time to understand the issue fully.
> >
> > Here’s the last version of my patch that still uses os.rename, and on error 
> > uses shutil.move, with the reasoning explained in the commit message. There 
> > are tons of os.rename in code elsewhere, and I’ve encountered similar 
> > issues when doing incremental builds, but I’ll leave fixing them to those 
> > with more time at hand. Cheers.
>
> Here’s a patch that catches error with all os.rename in the code, and retries 
> with shutil.move when that happens.
>
>
> >
> > From aeba1f9728f69cdf2ce4ba285be86761d8024f9d Mon Sep 17 00:00:00 2001
> > From: Devendra Tewari <devendra.tew...@gmail.com>
> > Date: Tue, 30 Mar 2021 08:27:40 -0300
> > Subject: [PATCH] Use shutil.move when os.rename fails
> >
> > Incremental build in Docker fails with
> >
> > OSError: [Errno 18] Invalid cross-device link
> >
> > When source and destination are on different overlay filesystems.
> >
> > This change handles error with os.rename and retries with shutil.move.
> > The reason os.rename is still used is because shutil.move is too slow
> > for speed sensitive sections of code.
> > ---
> > meta/classes/sstate.bbclass | 22 ++++++++++++++++++----
> > 1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> > index f579168162..301dfc27db 100644
> > --- a/meta/classes/sstate.bbclass
> > +++ b/meta/classes/sstate.bbclass
> > @@ -384,6 +384,7 @@ def sstate_installpkg(ss, d):
> > def sstate_installpkgdir(ss, d):
> >     import oe.path
> >     import subprocess
> > +    import shutil
> >
> >     sstateinst = d.getVar("SSTATE_INSTDIR")
> >     d.setVar('SSTATE_FIXMEDIR', ss['fixmedir'])
> > @@ -401,7 +402,10 @@ def sstate_installpkgdir(ss, d):
> >
> >     for state in ss['dirs']:
> >         prepdir(state[1])
> > -        os.rename(sstateinst + state[0], state[1])
> > +        try:
> > +            os.rename(sstateinst + state[0], state[1])
> > +        except OSError:
> > +            shutil.move(sstateinst + state[0], state[1])
> >     sstate_install(ss, d)
> >
> >     for plain in ss['plaindirs']:
> > @@ -413,7 +417,10 @@ def sstate_installpkgdir(ss, d):
> >         dest = plain
> >         bb.utils.mkdirhier(src)
> >         prepdir(dest)
> > -        os.rename(src, dest)
> > +        try:
> > +            os.rename(src, dest)
> > +        except OSError:
> > +            shutil.move(src, dest)
> >
> >     return True
> >
> > @@ -638,6 +645,7 @@ python sstate_hardcode_path () {
> >
> > def sstate_package(ss, d):
> >     import oe.path
> > +    import shutil
> >
> >     tmpdir = d.getVar('TMPDIR')
> >
> > @@ -664,7 +672,10 @@ def sstate_package(ss, d):
> >                     continue
> >                 bb.error("sstate found an absolute path symlink %s pointing 
> > at %s. Please replace this with a relative link." % (srcpath, link))
> >         bb.debug(2, "Preparing tree %s for packaging at %s" % (state[1], 
> > sstatebuild + state[0]))
> > -        os.rename(state[1], sstatebuild + state[0])
> > +        try:
> > +            os.rename(state[1], sstatebuild + state[0])
> > +        except OSError:
> > +            shutil.move(state[1], sstatebuild + state[0])
> >
> >     workdir = d.getVar('WORKDIR')
> >     sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")
> > @@ -674,7 +685,10 @@ def sstate_package(ss, d):
> >             pdir = plain.replace(sharedworkdir, sstatebuild)
> >         bb.utils.mkdirhier(plain)
> >         bb.utils.mkdirhier(pdir)
> > -        os.rename(plain, pdir)
> > +        try:
> > +            os.rename(plain, pdir)
> > +        except OSError:
> > +            shutil.move(plain, pdir)
> >
> >     d.setVar('SSTATE_BUILDDIR', sstatebuild)
> >     d.setVar('SSTATE_INSTDIR', sstatebuild)
> > --
> > 2.29.2
> >
> >
> >> On 30 Mar 2021, at 08:10, Richard Purdie 
> >> <richard.pur...@linuxfoundation.org> wrote:
> >>
> >> On Mon, 2021-03-29 at 16:00 -0700, Andre McCurdy wrote:
> >>> On Mon, Mar 29, 2021 at 3:45 PM Devendra Tewari
> >>> <devendra.tew...@gmail.com> wrote:
> >>>>
> >>>> Thanks! My bad. The example I looked up in Python docs had a break and I 
> >>>> just realized it was a looping example.
> >>>>
> >>>> Here’s the updated patch (or should I submit it again via git 
> >>>> send-email?)
> >>>
> >>> It would be better to use shutil.move unconditionally in all cases,
> >>> rather than have a separate shutil.move code path which only gets
> >>> tested by people doing incremental builds in docker.
> >>
> >> This is a speed sensitive section of code and shutil has traditionally 
> >> proved
> >> to be slow so I disagree with that, rename is very much preferred here 
> >> where
> >> we can.
> >>
> >> Cheers,
> >>
> >> Richard
> >>
> >>
> >
>
>
> 
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#150730): 
https://lists.openembedded.org/g/openembedded-core/message/150730
Mute This Topic: https://lists.openembedded.org/mt/81698791/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to