> 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
>>
>>
>
0001-use-shutil.move-when-os.rename-fails.patch
Description: Binary data
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#150682): https://lists.openembedded.org/g/openembedded-core/message/150682 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] -=-=-=-=-=-=-=-=-=-=-=-
