[prev in list] [next in list] [prev in thread] [next in thread]
List: openembedded-core
Subject: Re: [OE-core] [PATCH] Use shutil.move when os.rename fails
From: "Devendra Tewari" <devendra.tewari () gmail ! com>
Date: 2021-03-30 22:38:59
Message-ID: 3FA0EC13-0FC5-4E14-AC33-731A23F515F9 () gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Here's the correct link https://bugzilla.yoctoproject.org/show_bug.cgi?id=14301. I'll \
resubmit the patch referencing the bug in the commit message. Thanks.
> Em 30 de mar. de 2021, Ã (s) 18:59, Denys Dmytriyenko <denis@denix.org> 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@gmail.com> 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@outlook.com> 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@gmail.com> 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
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > >
> >
[Attachment #5 (text/html)]
<html><head><meta http-equiv="content-type" content="text/html; \
charset=utf-8"></head><body dir="auto"><div dir="ltr">Here's the correct link <a \
href="https://bugzilla.yoctoproject.org/show_bug.cgi?id=14301">https://bugzilla.yoctoproject.org/show_bug.cgi?id=14301</a>. \
I'll resubmit the patch referencing the bug in the commit message. Thanks.</div><div \
dir="ltr"><br><blockquote type="cite">Em 30 de mar. de 2021, Ã (s) 18:59, Denys \
Dmytriyenko <denis@denix.org> escreveu:<br><br></blockquote></div><blockquote \
type="cite"><div dir="ltr"><span>The link is for a 10-year old bug, probably not \
what you wanted.</span><br><span>Also, if a patch fixes existing bug in bugzilla, it \
needs to reference it in </span><br><span>commit message as well - \
[YOCTO#1234567]</span><br><span></span><br><span></span><br><span>On Mon, Mar 29, \
2021 at 12:37:45PM -0300, Devendra Tewari wrote:</span><br><blockquote \
type="cite"><span>Also, this is due to \
https://bugzilla.yoctoproject.org/show_bug.cgi?id=1430.</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><blockquote \
type="cite"><span>On 29 Mar 2021, at 12:35, Devendra Tewari \
<devendra.tewari@gmail.com> \
wrote:</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span>Sure.</span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span>Would the following commit message be \
sufficient?</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span> Use shutil.move when os.rename \
fails</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span> Incremental build in Docker \
fails with</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span> OSError: [Errno 18] Invalid \
cross-device link</span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span> When source and destination \
are on different overlay filesystems.</span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span> This change handles the error \
with os.rename and retries with \
shutil.move.</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span>Thanks,</span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span>Devendra</span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>On 29 Mar 2021, at \
12:23, Konrad Weihmann <kweihmann@outlook.com> \
wrote:</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>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</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>[1] \
https://docs.python.org/3/library/shutil.html#shutil.move</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>On 29.03.21 17:21, \
Bruce Ashfield wrote:</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>Can you document the cases that os.rename() is failing ? And also \
why</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>would we expect the shutil.move() to work in those cases \
?</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>If a change like this cases issues in the future, we need that \
extra</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>information in the commit head for proper \
triage.</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>Bruce</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>On Mon, Mar 29, 2021 at 11:16 AM Devendra \
Tewari</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span><devendra.tewari@gmail.com> \
wrote:</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span>---</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>meta/classes/sstate.bbclass | 26 \
++++++++++++++++++++++----</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>1 file changed, 22 insertions(+), 4 \
deletions(-)</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>diff --git a/meta/classes/sstate.bbclass \
b/meta/classes/sstate.bbclass</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>index f579168162..f94aa96d70 \
100644</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>--- \
a/meta/classes/sstate.bbclass</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+++ \
b/meta/classes/sstate.bbclass</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>@@ -384,6 +384,7 @@ def \
sstate_installpkg(ss, \
d):</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>def sstate_installpkgdir(ss, \
d):</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> import \
oe.path</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> import \
subprocess</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+ import \
shutil</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> sstateinst = \
d.getVar("SSTATE_INSTDIR")</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> \
d.setVar('SSTATE_FIXMEDIR', \
ss['fixmedir'])</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>@@ -401,7 +402,11 @@ def \
sstate_installpkgdir(ss, \
d):</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> for state in \
ss['dirs']:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> \
prepdir(state[1])</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>- \
os.rename(sstateinst + state[0], \
state[1])</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+ \
try:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+ \
os.rename(sstateinst \
+ state[0], state[1])</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+ \
break</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+ \
except \
OSError:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>+ \
shutil.move(sstateinst \
+ state[0], state[1])</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> sstate_install(ss, \
d)</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span></span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> for plain in \
ss['plaindirs']:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>@@ -413,7 +418,11 @@ def \
sstate_installpkgdir(ss, \
d):</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> \
dest = \
plain</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> \
bb.utils.mkdirhier(src)</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span> \
prepdir(dest)</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>- \
os.rename(src, \
dest)</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#150092): https://lists.openembedded.org/g/openembedded-core/message/150092
Mute This Topic: https://lists.openembedded.org/mt/81698791/4454766
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [openembedded-core@marc.info]
-=-=-=-=-=-=-=-=-=-=-=-
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic