Re: [OE-core] [PATCH] Use shutil.move when os.rename fails

["Devendra Tewari" <devendra.tewari@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5844 bytes --]

I created an image for Raspberry Pi Zero W from code at https://github.com/tewarid/docker-meta-raspberrypi/tree/sstate <https://github.com/tewarid/docker-meta-raspberrypi/tree/sstate>, using Docker for macOS, and on Ubuntu 20.04-LTS, and wrote it out to an SDCard using bmaptool. Booted and checked that busybox symlinks in rootfs look all right.

> Em 21 de abr. de 2021, à(s) 02:22, Khem Raj <raj.khem@gmail.com> escreveu:
> 
> 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.tewari@gmail.com> wrote:
>> 
>> 
>> 
>>>> On 30 Mar 2021, at 08:55, Devendra Tewari <devendra.tewari@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.tewari@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.purdie@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.tewari@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
>>>> 
>>>> 
>>> 
>> 
>> 
>> 
>> 

[-- Attachment #2: Type: text/html, Size: 9287 bytes --]