Can you document the cases that os.rename() is failing ? And also why
would we expect the shutil.move() to work in those cases ?
If a change like this cases issues in the future, we need that extra
information in the commit head for proper triage.
Bruce
On Mon, Mar 29, 2021 at 11:16 AM Devendra Tewari
<devendra.tewari@...> wrote:

---
meta/classes/sstate.bbclass | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index f579168162..f94aa96d70 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,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(sstateinst + state[0], state[1])
sstate_install(ss, d)

for plain in ss['plaindirs']:
@@ -413,7 +418,11 @@ def sstate_installpkgdir(ss, d):
dest = plain
bb.utils.mkdirhier(src)
prepdir(dest)
- os.rename(src, dest)
+ try:
+ os.rename(src, dest)
+ break
+ except OSError:
+ shutil.move(src, dest)

return True

@@ -638,6 +647,7 @@ python sstate_hardcode_path () {

def sstate_package(ss, d):
import oe.path
+ import shutil

tmpdir = d.getVar('TMPDIR')

@@ -664,7 +674,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(state[1], sstatebuild + state[0])

workdir = d.getVar('WORKDIR')
sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")
@@ -674,7 +688,11 @@ 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)
+ break
+ except OSError:
+ shutil.move(plain, pdir)

d.setVar('SSTATE_BUILDDIR', sstatebuild)
d.setVar('SSTATE_INSTDIR', sstatebuild)
--
2.29.2

On 29 Mar 2021, at 12:23, Konrad Weihmann <kweihmann@...> wrote:

Yes please quote a bit from the python manpage [1] - I certainly see the difference (and the edge cases where os.rename might fail), but that should be documented as part of the commit message

[1] https://docs.python.org/3/library/shutil.html#shutil.move

On 29.03.21 17:21, Bruce Ashfield wrote:

Can you document the cases that os.rename() is failing ? And also why
would we expect the shutil.move() to work in those cases ?
If a change like this cases issues in the future, we need that extra
information in the commit head for proper triage.
Bruce
On Mon, Mar 29, 2021 at 11:16 AM Devendra Tewari
<devendra.tewari@...> wrote:

---
meta/classes/sstate.bbclass | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index f579168162..f94aa96d70 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,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(sstateinst + state[0], state[1])
sstate_install(ss, d)

for plain in ss['plaindirs']:
@@ -413,7 +418,11 @@ def sstate_installpkgdir(ss, d):
dest = plain
bb.utils.mkdirhier(src)
prepdir(dest)
- os.rename(src, dest)
+ try:
+ os.rename(src, dest)
+ break
+ except OSError:
+ shutil.move(src, dest)

return True

@@ -638,6 +647,7 @@ python sstate_hardcode_path () {

def sstate_package(ss, d):
import oe.path
+ import shutil

tmpdir = d.getVar('TMPDIR')

@@ -664,7 +674,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(state[1], sstatebuild + state[0])

workdir = d.getVar('WORKDIR')
sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")
@@ -674,7 +688,11 @@ 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)
+ break
+ except OSError:
+ shutil.move(plain, pdir)

d.setVar('SSTATE_BUILDDIR', sstatebuild)
d.setVar('SSTATE_INSTDIR', sstatebuild)
--
2.29.2

On 29 Mar 2021, at 12:35, Devendra Tewari <devendra.tewari@...> wrote:

Sure.

Would the following commit message be sufficient?

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 the error with os.rename and retries with shutil.move.

Thanks,
Devendra

On 29 Mar 2021, at 12:23, Konrad Weihmann <kweihmann@...> wrote:

Yes please quote a bit from the python manpage [1] - I certainly see the difference (and the edge cases where os.rename might fail), but that should be documented as part of the commit message

[1] https://docs.python.org/3/library/shutil.html#shutil.move

On 29.03.21 17:21, Bruce Ashfield wrote:

Can you document the cases that os.rename() is failing ? And also why
would we expect the shutil.move() to work in those cases ?
If a change like this cases issues in the future, we need that extra
information in the commit head for proper triage.
Bruce
On Mon, Mar 29, 2021 at 11:16 AM Devendra Tewari
<devendra.tewari@...> wrote:

---
meta/classes/sstate.bbclass | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index f579168162..f94aa96d70 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,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(sstateinst + state[0], state[1])
sstate_install(ss, d)

for plain in ss['plaindirs']:
@@ -413,7 +418,11 @@ def sstate_installpkgdir(ss, d):
dest = plain
bb.utils.mkdirhier(src)
prepdir(dest)
- os.rename(src, dest)
+ try:
+ os.rename(src, dest)
+ break
+ except OSError:
+ shutil.move(src, dest)

return True

@@ -638,6 +647,7 @@ python sstate_hardcode_path () {

def sstate_package(ss, d):
import oe.path
+ import shutil

tmpdir = d.getVar('TMPDIR')

@@ -664,7 +674,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(state[1], sstatebuild + state[0])

workdir = d.getVar('WORKDIR')
sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")
@@ -674,7 +688,11 @@ 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)
+ break
+ except OSError:
+ shutil.move(plain, pdir)

d.setVar('SSTATE_BUILDDIR', sstatebuild)
d.setVar('SSTATE_INSTDIR', sstatebuild)
--
2.29.2

On 29 Mar 2021, at 17:38, Richard Purdie <richard.purdie@...> wrote:

On Mon, 2021-03-29 at 12:14 -0300, Devendra Tewari wrote:

---
 meta/classes/sstate.bbclass | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index f579168162..f94aa96d70 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,11 @@ 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])
+            break

That break should definitely not be there, that changes behaviour.

+        except OSError:
+            shutil.move(sstateinst + state[0], state[1])
     sstate_install(ss, d)
 

     for plain in ss['plaindirs']:
@@ -413,7 +418,11 @@ def sstate_installpkgdir(ss, d):
         dest = plain
         bb.utils.mkdirhier(src)
         prepdir(dest)
-        os.rename(src, dest)
+        try:
+            os.rename(src, dest)
+            break

Same here...

+        except OSError:
+            shutil.move(src, dest)
 

     return True
 

@@ -638,6 +647,7 @@ python sstate_hardcode_path () {
 

 def sstate_package(ss, d):
     import oe.path
+    import shutil
 

     tmpdir = d.getVar('TMPDIR')
 

@@ -664,7 +674,11 @@ 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])
+            break

and again...

+        except OSError:
+            shutil.move(state[1], sstatebuild + state[0])
 

     workdir = d.getVar('WORKDIR')
     sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")
@@ -674,7 +688,11 @@ 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)
+            break

and again...

+        except OSError:
+            shutil.move(plain, pdir)
 

     d.setVar('SSTATE_BUILDDIR', sstatebuild)
     d.setVar('SSTATE_INSTDIR', sstatebuild)

On 29 Mar 2021, at 20:00, Andre McCurdy <armccurdy@...> wrote:

On Mon, Mar 29, 2021 at 3:45 PM Devendra Tewari
<devendra.tewari@...> 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.

On 29 Mar 2021, at 20:10, Andre McCurdy <armccurdy@...> wrote:

On Mon, Mar 29, 2021 at 4:07 PM Devendra Tewari
<devendra.tewari@...> wrote:

I did try that but got an error that does not happen when we try os.rename first. I'll try to reproduce it again.

I suspect there may be subtle differences in os.rename vs shutil.move with respect to what happens when origin and/or destination do not exist or are invalid.

If there are subtle issues which you don't yet understand then that's
all the more reason for not hiding the new shutil.move code behind a
test which will pass for 99.9% of users. Please figure that out before
sending another version of the patch.

On 29 Mar 2021, at 20:00, Andre McCurdy <armccurdy@...> wrote:

On Mon, Mar 29, 2021 at 3:45 PM Devendra Tewari
<devendra.tewari@...> 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.

On 29 Mar 2021, at 20:13, Devendra Tewari <devendra.tewari@...> wrote:

Will do. Thanks.

On 29 Mar 2021, at 20:10, Andre McCurdy <armccurdy@...> wrote:

On Mon, Mar 29, 2021 at 4:07 PM Devendra Tewari
<devendra.tewari@...> wrote:

I did try that but got an error that does not happen when we try os.rename first. I'll try to reproduce it again.

I suspect there may be subtle differences in os.rename vs shutil.move with respect to what happens when origin and/or destination do not exist or are invalid.

If there are subtle issues which you don't yet understand then that's
all the more reason for not hiding the new shutil.move code behind a
test which will pass for 99.9% of users. Please figure that out before
sending another version of the patch.

On 29 Mar 2021, at 20:00, Andre McCurdy <armccurdy@...> wrote:

On Mon, Mar 29, 2021 at 3:45 PM Devendra Tewari
<devendra.tewari@...> 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.

On 30 Mar 2021, at 08:10, Richard Purdie <richard.purdie@...> 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@...> 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

Also, this is due to https://bugzilla.yoctoproject.org/show_bug.cgi?id=1430.

On 29 Mar 2021, at 12:35, Devendra Tewari <devendra.tewari@...> wrote:

Sure.

Would the following commit message be sufficient?

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 the error with os.rename and retries with shutil.move.

Thanks,
Devendra

On 29 Mar 2021, at 12:23, Konrad Weihmann <kweihmann@...> wrote:

Yes please quote a bit from the python manpage [1] - I certainly see the difference (and the edge cases where os.rename might fail), but that should be documented as part of the commit message

[1] https://docs.python.org/3/library/shutil.html#shutil.move

On 29.03.21 17:21, Bruce Ashfield wrote:

Can you document the cases that os.rename() is failing ? And also why
would we expect the shutil.move() to work in those cases ?
If a change like this cases issues in the future, we need that extra
information in the commit head for proper triage.
Bruce
On Mon, Mar 29, 2021 at 11:16 AM Devendra Tewari
<devendra.tewari@...> wrote:

---
meta/classes/sstate.bbclass | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index f579168162..f94aa96d70 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,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(sstateinst + state[0], state[1])
sstate_install(ss, d)

for plain in ss['plaindirs']:
@@ -413,7 +418,11 @@ def sstate_installpkgdir(ss, d):
dest = plain
bb.utils.mkdirhier(src)
prepdir(dest)
- os.rename(src, dest)
+ try:
+ os.rename(src, dest)
+ break
+ except OSError:
+ shutil.move(src, dest)

return True

@@ -638,6 +647,7 @@ python sstate_hardcode_path () {

def sstate_package(ss, d):
import oe.path
+ import shutil

tmpdir = d.getVar('TMPDIR')

@@ -664,7 +674,11 @@ 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])
+ break
+ except OSError:
+ shutil.move(state[1], sstatebuild + state[0])

workdir = d.getVar('WORKDIR')
sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")
@@ -674,7 +688,11 @@ 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)
+ break
+ except OSError:
+ shutil.move(plain, pdir)

d.setVar('SSTATE_BUILDDIR', sstatebuild)
d.setVar('SSTATE_INSTDIR', sstatebuild)
--
2.29.2

Em 30 de mar. de 2021, à(s) 18:59, Denys Dmytriyenko <denis@...> escreveu:

The link is for a 10-year old bug, probably not what you wanted.
Also, if a patch fixes existing bug in bugzilla, it needs to reference it in
commit message as well - [YOCTO#1234567]


On Mon, Mar 29, 2021 at 12:37:45PM -0300, Devendra Tewari wrote:

Also, this is due to https://bugzilla.yoctoproject.org/show_bug.cgi?id=1430.

On 29 Mar 2021, at 12:35, Devendra Tewari <devendra.tewari@...> wrote:

Sure.

Would the following commit message be sufficient?

  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 the error with os.rename and retries with shutil.move.

Thanks,

Devendra

On 29 Mar 2021, at 12:23, Konrad Weihmann <kweihmann@...> wrote:

Yes please quote a bit from the python manpage [1] - I certainly see the difference (and the edge cases where os.rename might fail), but that should be documented as part of the commit message

[1] https://docs.python.org/3/library/shutil.html#shutil.move

On 29.03.21 17:21, Bruce Ashfield wrote:

Can you document the cases that os.rename() is failing ? And also why

would we expect the shutil.move() to work in those cases ?

If a change like this cases issues in the future, we need that extra

information in the commit head for proper triage.

Bruce

On Mon, Mar 29, 2021 at 11:16 AM Devendra Tewari

<devendra.tewari@...> wrote:

---

meta/classes/sstate.bbclass | 26 ++++++++++++++++++++++----

1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass

index f579168162..f94aa96d70 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,11 @@ 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])

+            break

+        except OSError:

+            shutil.move(sstateinst + state[0], state[1])

   sstate_install(ss, d)

   for plain in ss['plaindirs']:

@@ -413,7 +418,11 @@ def sstate_installpkgdir(ss, d):

       dest = plain

       bb.utils.mkdirhier(src)

       prepdir(dest)

-        os.rename(src, dest)

+        try:

+            os.rename(src, dest)

+            break

+        except OSError:

+            shutil.move(src, dest)

   return True

@@ -638,6 +647,7 @@ python sstate_hardcode_path () {

def sstate_package(ss, d):

   import oe.path

+    import shutil

   tmpdir = d.getVar('TMPDIR')

@@ -664,7 +674,11 @@ 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])

+            break

+        except OSError:

+            shutil.move(state[1], sstatebuild + state[0])

   workdir = d.getVar('WORKDIR')

   sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared")

@@ -674,7 +688,11 @@ 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)

+            break

+        except OSError:

+            shutil.move(plain, pdir)

   d.setVar('SSTATE_BUILDDIR', sstatebuild)

   d.setVar('SSTATE_INSTDIR', sstatebuild)

--

2.29.2

Em 21 de abr. de 2021, à(s) 02:22, Khem Raj <raj.khem@...> 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@...> wrote:

On 30 Mar 2021, at 08:55, Devendra Tewari <devendra.tewari@...> 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@...>
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@...> 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@...> 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