* [PATCH] Use shutil.move when os.rename fails @ 2021-03-29 15:14 devendra.tewari 2021-03-29 15:21 ` [OE-core] " Bruce Ashfield 2021-03-29 20:38 ` Richard Purdie 0 siblings, 2 replies; 34+ messages in thread From: devendra.tewari @ 2021-03-29 15:14 UTC (permalink / raw) To: openembedded-core; +Cc: Devendra Tewari --- 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 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 15:14 [PATCH] Use shutil.move when os.rename fails devendra.tewari @ 2021-03-29 15:21 ` Bruce Ashfield 2021-03-29 15:23 ` Konrad Weihmann 2021-03-29 20:38 ` Richard Purdie 1 sibling, 1 reply; 34+ messages in thread From: Bruce Ashfield @ 2021-03-29 15:21 UTC (permalink / raw) To: Devendra Tewari; +Cc: Patches and discussions about the oe-core layer 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 > > > > -- - Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end - "Use the force Harry" - Gandalf, Star Trek II ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 15:21 ` [OE-core] " Bruce Ashfield @ 2021-03-29 15:23 ` Konrad Weihmann 2021-03-29 15:35 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Konrad Weihmann @ 2021-03-29 15:23 UTC (permalink / raw) To: openembedded-core, devendra.tewari 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 >> >> >> >> > > > > > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 15:23 ` Konrad Weihmann @ 2021-03-29 15:35 ` Devendra Tewari 2021-03-29 15:37 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-29 15:35 UTC (permalink / raw) To: Konrad Weihmann; +Cc: openembedded-core 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 >>> >>> >>> >>> >> ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 15:35 ` Devendra Tewari @ 2021-03-29 15:37 ` Devendra Tewari 2021-03-30 21:59 ` Denys Dmytriyenko 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-29 15:37 UTC (permalink / raw) To: Konrad Weihmann; +Cc: openembedded-core 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 >>>> >>>> >>>> >>>> >>> > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 15:37 ` Devendra Tewari @ 2021-03-30 21:59 ` Denys Dmytriyenko 2021-03-30 22:38 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Denys Dmytriyenko @ 2021-03-30 21:59 UTC (permalink / raw) To: Devendra Tewari; +Cc: Konrad Weihmann, openembedded-core 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 > >>>> > >>>> > >>>> > >>>> > >>> > > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-30 21:59 ` Denys Dmytriyenko @ 2021-03-30 22:38 ` Devendra Tewari 2021-04-01 19:14 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-30 22:38 UTC (permalink / raw) To: Denys Dmytriyenko; +Cc: Konrad Weihmann, openembedded-core [-- Attachment #1: Type: text/plain, Size: 5068 bytes --] 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 #2: Type: text/html, Size: 27335 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-30 22:38 ` Devendra Tewari @ 2021-04-01 19:14 ` Devendra Tewari 0 siblings, 0 replies; 34+ messages in thread From: Devendra Tewari @ 2021-04-01 19:14 UTC (permalink / raw) To: Denys Dmytriyenko; +Cc: Konrad Weihmann, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 7991 bytes --] From 57b633a93bd91b7b1aa21cce5ac7997b958ca917 Mon Sep 17 00:00:00 2001 From: Devendra Tewari <devendra.tewari@gmail.com> Date: Thu, 1 Apr 2021 16:07:25 -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. [Yocto #14301] --- 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 8e8efd18d5..60f7a94285 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 Tue, Mar 30, 2021 at 7:39 PM Devendra Tewari <devendra.tewari@gmail.com> wrote: > 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 #2: Type: text/html, Size: 31057 bytes --] ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 15:14 [PATCH] Use shutil.move when os.rename fails devendra.tewari 2021-03-29 15:21 ` [OE-core] " Bruce Ashfield @ 2021-03-29 20:38 ` Richard Purdie 2021-03-29 22:45 ` Devendra Tewari 1 sibling, 1 reply; 34+ messages in thread From: Richard Purdie @ 2021-03-29 20:38 UTC (permalink / raw) To: Devendra Tewari, openembedded-core 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) > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 20:38 ` Richard Purdie @ 2021-03-29 22:45 ` Devendra Tewari 2021-03-29 23:00 ` Andre McCurdy 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-29 22:45 UTC (permalink / raw) To: Richard Purdie; +Cc: openembedded-core [-- Attachment #1: Type: text/plain, Size: 6793 bytes --] 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?) From 9deb390dcdcaef66cec2fae39454c7fb3c81c4e4 Mon Sep 17 00:00:00 2001 From: Devendra Tewari <devendra.tewari@gmail.com> Date: Mon, 29 Mar 2021 19:41:02 -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 the error with os.rename and retries with shutil.move. --- meta/classes/sstate.bbclass | 22 ++++++++++++++++++---- vscode-bitbake-build/executeBitBakeCmd.sh | 3 +++ 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100755 vscode-bitbake-build/executeBitBakeCmd.sh 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) diff --git a/vscode-bitbake-build/executeBitBakeCmd.sh b/vscode-bitbake-build/executeBitBakeCmd.sh new file mode 100755 index 0000000000..d7a4c5a5aa --- /dev/null +++ b/vscode-bitbake-build/executeBitBakeCmd.sh @@ -0,0 +1,3 @@ +#!/bin/bash +. ./oe-init-build-env vscode-bitbake-build > /dev/null +bitbake-layers show-layers \ No newline at end of file -- 2.29.2 > On 29 Mar 2021, at 17:38, Richard Purdie <richard.purdie@linuxfoundation.org> 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) >> [-- Attachment #2: Type: text/html, Size: 41671 bytes --] ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 22:45 ` Devendra Tewari @ 2021-03-29 23:00 ` Andre McCurdy 2021-03-29 23:07 ` Devendra Tewari 2021-03-30 11:10 ` Richard Purdie 0 siblings, 2 replies; 34+ messages in thread From: Andre McCurdy @ 2021-03-29 23:00 UTC (permalink / raw) To: Devendra Tewari; +Cc: Richard Purdie, OE Core mailing list 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 23:00 ` Andre McCurdy @ 2021-03-29 23:07 ` Devendra Tewari 2021-03-29 23:10 ` Andre McCurdy 2021-03-30 11:10 ` Richard Purdie 1 sibling, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-29 23:07 UTC (permalink / raw) To: Andre McCurdy; +Cc: Richard Purdie, OE Core mailing list 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. > On 29 Mar 2021, at 20:00, Andre McCurdy <armccurdy@gmail.com> 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. ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 23:07 ` Devendra Tewari @ 2021-03-29 23:10 ` Andre McCurdy 2021-03-29 23:13 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Andre McCurdy @ 2021-03-29 23:10 UTC (permalink / raw) To: Devendra Tewari; +Cc: Richard Purdie, OE Core mailing list On Mon, Mar 29, 2021 at 4:07 PM Devendra Tewari <devendra.tewari@gmail.com> 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@gmail.com> 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. > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 23:10 ` Andre McCurdy @ 2021-03-29 23:13 ` Devendra Tewari 2021-03-30 10:16 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-29 23:13 UTC (permalink / raw) To: Andre McCurdy; +Cc: Richard Purdie, OE Core mailing list Will do. Thanks. > On 29 Mar 2021, at 20:10, Andre McCurdy <armccurdy@gmail.com> wrote: > > On Mon, Mar 29, 2021 at 4:07 PM Devendra Tewari > <devendra.tewari@gmail.com> 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@gmail.com> 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. >> ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 23:13 ` Devendra Tewari @ 2021-03-30 10:16 ` Devendra Tewari 0 siblings, 0 replies; 34+ messages in thread From: Devendra Tewari @ 2021-03-30 10:16 UTC (permalink / raw) To: Andre McCurdy; +Cc: Richard Purdie, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 7666 bytes --] With the following patch From f29ec67239094a256fcfc119fb75be90923d3448 Mon Sep 17 00:00:00 2001 From: Devendra Tewari <devendra.tewari@gmail.com> Date: Mon, 29 Mar 2021 21:11:56 -0300 Subject: [PATCH] Use shutil.move to rename sstate Incremental build in Docker fails with OSError: [Errno 18] Invalid cross-device link When source and destination are on different overlay filesystems. shutil.move uses os.rename when destination is on the current filesystem, otherwise source is copied to destination and then removed. --- meta/classes/sstate.bbclass | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass index f579168162..2e87697e3e 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,7 @@ def sstate_installpkgdir(ss, d): for state in ss['dirs']: prepdir(state[1]) - os.rename(sstateinst + state[0], state[1]) + shutil.move(sstateinst + state[0], state[1]) sstate_install(ss, d) for plain in ss['plaindirs']: @@ -413,7 +414,7 @@ def sstate_installpkgdir(ss, d): dest = plain bb.utils.mkdirhier(src) prepdir(dest) - os.rename(src, dest) + shutil.move(src, dest) return True @@ -638,6 +639,7 @@ python sstate_hardcode_path () { def sstate_package(ss, d): import oe.path + import shutil tmpdir = d.getVar('TMPDIR') @@ -664,7 +666,7 @@ 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]) + shutil.move(state[1], sstatebuild + state[0]) workdir = d.getVar('WORKDIR') sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared") @@ -674,7 +676,7 @@ def sstate_package(ss, d): pdir = plain.replace(sharedworkdir, sstatebuild) bb.utils.mkdirhier(plain) bb.utils.mkdirhier(pdir) - os.rename(plain, pdir) + shutil.move(plain, pdir) d.setVar('SSTATE_BUILDDIR', sstatebuild) d.setVar('SSTATE_INSTDIR', sstatebuild) -- 2.29.2 The build fails with #12 2734.2 2021-03-30 01:14:29 - INFO - NOTE: recipe linux-libc-headers-5.10-r0: task do_package_write_rpm: Started #12 2734.4 2021-03-30 01:14:29 - INFO - NOTE: recipe lzo-native-2.10-r0: task do_prepare_recipe_sysroot: Started #12 2734.8 2021-03-30 01:14:30 - INFO - NOTE: recipe lzo-native-2.10-r0: task do_prepare_recipe_sysroot: Succeeded #12 2734.9 2021-03-30 01:14:30 - INFO - NOTE: Running task 2441 of 4982 (virtual:native:/home/pi/docker-meta-raspberrypi/layers/poky/meta/recipes-support/lzo/lzo_2.10.bb:do_configure) #12 2735.0 2021-03-30 01:14:30 - ERROR - ERROR: linux-libc-headers-5.10-r0 do_package_write_rpm: Error executing a python function in exec_python_func() autogenerated: #12 2735.0 2021-03-30 01:14:30 - ERROR - #12 2735.0 2021-03-30 01:14:30 - ERROR - The stack trace of python calls that resulted in this exception/failure was: #12 2735.0 2021-03-30 01:14:30 - ERROR - File: 'exec_python_func() autogenerated', lineno: 2, function: <module> #12 2735.0 2021-03-30 01:14:30 - ERROR - 0001: #12 2735.0 2021-03-30 01:14:30 - ERROR - *** 0002:write_specfile(d) #12 2735.0 2021-03-30 01:14:30 - ERROR - 0003: #12 2735.0 2021-03-30 01:14:30 - ERROR - File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/classes/package_rpm.bbclass', lineno: 342, function: write_specfile #12 2735.0 2021-03-30 01:14:30 - ERROR - 0338: localdata.setVar('PKG', pkgname) #12 2735.0 2021-03-30 01:14:30 - ERROR - 0339: #12 2735.0 2021-03-30 01:14:30 - ERROR - 0340: localdata.setVar('OVERRIDES', d.getVar("OVERRIDES", False) + ":" + pkg) #12 2735.0 2021-03-30 01:14:30 - ERROR - 0341: #12 2735.0 2021-03-30 01:14:30 - ERROR - *** 0342: conffiles = get_conffiles(pkg, d) #12 2735.0 2021-03-30 01:14:30 - ERROR - 0343: dirfiles = localdata.getVar('DIRFILES') #12 2735.0 2021-03-30 01:14:30 - ERROR - 0344: if dirfiles is not None: #12 2735.0 2021-03-30 01:14:30 - ERROR - 0345: dirfiles = dirfiles.split() #12 2735.0 2021-03-30 01:14:30 - ERROR - 0346: #12 2735.0 2021-03-30 01:14:30 - ERROR - File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/classes/package.bbclass', lineno: 304, function: get_conffiles #12 2735.0 2021-03-30 01:14:30 - ERROR - 0300:def get_conffiles(pkg, d): #12 2735.0 2021-03-30 01:14:30 - ERROR - 0301: pkgdest = d.getVar('PKGDEST') #12 2735.0 2021-03-30 01:14:30 - ERROR - 0302: root = os.path.join(pkgdest, pkg) #12 2735.0 2021-03-30 01:14:30 - ERROR - 0303: cwd = os.getcwd() #12 2735.0 2021-03-30 01:14:30 - ERROR - *** 0304: os.chdir(root) #12 2735.0 2021-03-30 01:14:30 - ERROR - 0305: #12 2735.0 2021-03-30 01:14:30 - ERROR - 0306: conffiles = d.getVar('CONFFILES_%s' % pkg); #12 2735.0 2021-03-30 01:14:30 - ERROR - 0307: if conffiles == None: #12 2735.0 2021-03-30 01:14:30 - ERROR - 0308: conffiles = d.getVar('CONFFILES') #12 2735.0 2021-03-30 01:14:30 - ERROR - Exception: FileNotFoundError: [Errno 2] No such file or directory: '/home/pi/docker-meta-raspberrypi/build/tmp/work/arm1176jzfshf-vfp-poky-linux-gnueabi/linux-libc-headers/5.10-r0/packages-split/linux-libc-headers-src' A fully reproducible build can be found at https://github.com/tewarid/docker-meta-raspberrypi/tree/ee083820c565da646b745bc25e439c13f3205dbe. Please see build instructions in the readme. I can’t seem to find a root cause and don’t have the time to look into this any further. > On 29 Mar 2021, at 20:13, Devendra Tewari <devendra.tewari@gmail.com> wrote: > > Will do. Thanks. > >> On 29 Mar 2021, at 20:10, Andre McCurdy <armccurdy@gmail.com> wrote: >> >> On Mon, Mar 29, 2021 at 4:07 PM Devendra Tewari >> <devendra.tewari@gmail.com> 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@gmail.com> 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. >>> > [-- Attachment #2: Type: text/html, Size: 28215 bytes --] ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-29 23:00 ` Andre McCurdy 2021-03-29 23:07 ` Devendra Tewari @ 2021-03-30 11:10 ` Richard Purdie 2021-03-30 11:55 ` Devendra Tewari 1 sibling, 1 reply; 34+ messages in thread From: Richard Purdie @ 2021-03-30 11:10 UTC (permalink / raw) To: Andre McCurdy, Devendra Tewari; +Cc: OE Core mailing list 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 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-30 11:10 ` Richard Purdie @ 2021-03-30 11:55 ` Devendra Tewari 2021-04-19 18:43 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-03-30 11:55 UTC (permalink / raw) To: Richard Purdie; +Cc: Andre McCurdy, OE Core mailing list 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. 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 > > ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-03-30 11:55 ` Devendra Tewari @ 2021-04-19 18:43 ` Devendra Tewari 2021-04-21 5:21 ` Khem Raj 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-19 18:43 UTC (permalink / raw) To: Richard Purdie; +Cc: Andre McCurdy, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 4870 bytes --] > 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: 0001-use-shutil.move-when-os.rename-fails.patch --] [-- Type: application/octet-stream, Size: 25567 bytes --] From d3472288e80f036dd2f8a8fc72bdeaecd9f386d8 Mon Sep 17 00:00:00 2001 From: Devendra Tewari <devendra.tewari@gmail.com> Date: Mon, 19 Apr 2021 11:23:58 -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. [Yocto #14301] --- bitbake/lib/bb/siggen.py | 6 +++++- bitbake/lib/bb/tests/fetch.py | 13 ++++++++++-- bitbake/lib/bb/utils.py | 16 ++++++++++++--- meta/classes/buildhistory.bbclass | 6 +++++- meta/classes/package.bbclass | 12 +++++++++-- meta/classes/populate_sdk_ext.bbclass | 10 ++++++++-- meta/classes/reproducible_build.bbclass | 6 +++++- meta/classes/sstate.bbclass | 22 +++++++++++++++++---- meta/classes/update-alternatives.bbclass | 11 +++++++++-- meta/lib/oe/package_manager/deb/__init__.py | 18 ++++++++++++++--- meta/lib/oe/package_manager/ipk/__init__.py | 5 ++++- meta/lib/oe/rootfs.py | 15 +++++++++++--- meta/lib/oeqa/selftest/cases/wic.py | 16 ++++++++++++--- scripts/combo-layer | 5 ++++- scripts/lib/devtool/standard.py | 21 ++++++++++++++++---- scripts/lib/devtool/upgrade.py | 7 ++++++- scripts/lib/wic/plugins/imager/direct.py | 5 ++++- 17 files changed, 159 insertions(+), 35 deletions(-) diff --git a/bitbake/lib/bb/siggen.py b/bitbake/lib/bb/siggen.py index 0d88c6ec68..cf89e21439 100644 --- a/bitbake/lib/bb/siggen.py +++ b/bitbake/lib/bb/siggen.py @@ -341,6 +341,7 @@ class SignatureGeneratorBasic(SignatureGenerator): self.unihash_cache.save(self.unitaskhashes) def dump_sigtask(self, fn, task, stampbase, runtime): + import shutil tid = fn + ":" + task referencestamp = stampbase @@ -402,7 +403,10 @@ class SignatureGeneratorBasic(SignatureGenerator): p = pickle.dump(data, stream, -1) stream.flush() os.chmod(tmpfile, 0o664) - os.rename(tmpfile, sigfile) + try: + os.rename(tmpfile, sigfile) + except OSError: + shutil.move(tmpfile, sigfile) except (OSError, IOError) as err: try: os.unlink(tmpfile) diff --git a/bitbake/lib/bb/tests/fetch.py b/bitbake/lib/bb/tests/fetch.py index ddf6e97439..8cf2eb0ac3 100644 --- a/bitbake/lib/bb/tests/fetch.py +++ b/bitbake/lib/bb/tests/fetch.py @@ -1770,6 +1770,7 @@ class GitShallowTest(FetcherTest): self.assertRevCount(1, cwd=os.path.join(self.gitdir, 'gitsubmodule')) def test_shallow_submodule_mirrors(self): + import shutil self.add_empty_file('a') self.add_empty_file('b') @@ -1794,7 +1795,10 @@ class GitShallowTest(FetcherTest): # Set up the mirror mirrordir = os.path.join(self.tempdir, 'mirror') - os.rename(self.dldir, mirrordir) + try: + os.rename(self.dldir, mirrordir) + except OSError: + shutil.move(self.dldir, mirrordir) self.d.setVar('PREMIRRORS', 'gitsm://.*/.* file://%s/\n' % mirrordir) # Fetch from the mirror @@ -1899,6 +1903,7 @@ class GitShallowTest(FetcherTest): assert not os.path.exists(os.path.join(self.gitdir, '.git', 'shallow')) def test_shallow_mirrors(self): + import shutil self.add_empty_file('a') self.add_empty_file('b') @@ -1912,7 +1917,11 @@ class GitShallowTest(FetcherTest): bb.utils.mkdirhier(mirrordir) self.d.setVar('PREMIRRORS', 'git://.*/.* file://%s/\n' % mirrordir) - os.rename(os.path.join(self.dldir, mirrortarball), + try: + os.rename(os.path.join(self.dldir, mirrortarball), + os.path.join(mirrordir, mirrortarball)) + except OSError: + shutil.move(os.path.join(self.dldir, mirrortarball), os.path.join(mirrordir, mirrortarball)) # Fetch from the mirror diff --git a/bitbake/lib/bb/utils.py b/bitbake/lib/bb/utils.py index b282d09abf..f289f5664e 100644 --- a/bitbake/lib/bb/utils.py +++ b/bitbake/lib/bb/utils.py @@ -736,6 +736,7 @@ def movefile(src, dest, newmtime = None, sstat = None): filesystems. Returns true on success and false on failure. Move is atomic. """ + import shutil #print "movefile(" + src + "," + dest + "," + str(newmtime) + "," + str(sstat) + ")" try: @@ -782,7 +783,10 @@ def movefile(src, dest, newmtime = None, sstat = None): if sstat[stat.ST_DEV] == dstat[stat.ST_DEV]: try: - os.rename(src, destpath) + try: + os.rename(src, destpath) + except OSError: + shutil.move(src, destpath) renamefailed = 0 except Exception as e: if e.errno != errno.EXDEV: @@ -796,7 +800,10 @@ def movefile(src, dest, newmtime = None, sstat = None): if stat.S_ISREG(sstat[stat.ST_MODE]): try: # For safety copy then move it over. shutil.copyfile(src, destpath + "#new") - os.rename(destpath + "#new", destpath) + try: + os.rename(destpath + "#new", destpath) + except OSError: + shutil.move(destpath + "#new", destpath) didcopy = 1 except Exception as e: print('movefile: copy', src, '->', dest, 'failed.', e) @@ -874,7 +881,10 @@ def copyfile(src, dest, newmtime = None, sstat = None): # For safety copy then move it over. shutil.copyfile(src, dest + "#new") - os.rename(dest + "#new", dest) + try: + os.rename(dest + "#new", dest) + except OSError: + shutil.move(dest + "#new", dest) except Exception as e: logger.warning("copyfile: copy %s to %s failed (%s)" % (src, dest, e)) return False diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass index 117a44eaf3..50279891a3 100644 --- a/meta/classes/buildhistory.bbclass +++ b/meta/classes/buildhistory.bbclass @@ -872,7 +872,11 @@ python buildhistory_eventhandler() { entries = [ x for x in os.listdir(rootdir) if not x.startswith('.') ] bb.utils.mkdirhier(olddir) for entry in entries: - os.rename(os.path.join(rootdir, entry), + try: + os.rename(os.path.join(rootdir, entry), + os.path.join(olddir, entry)) + except OSError: + shutil.move(os.path.join(rootdir, entry), os.path.join(olddir, entry)) elif isinstance(e, bb.event.BuildCompleted): if reset: diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass index e3f0a7060b..6707c2e9db 100644 --- a/meta/classes/package.bbclass +++ b/meta/classes/package.bbclass @@ -835,6 +835,7 @@ perform_packagecopy[dirs] = "${PKGD}" # the fs-perms.txt files python fixup_perms () { import pwd, grp + import shutil # init using a string with the same format as a line as documented in # the fs-perms.txt file @@ -1049,7 +1050,10 @@ python fixup_perms () { # Create path to move directory to, move it, and then setup the symlink bb.utils.mkdirhier(os.path.dirname(target)) #bb.note("Fixup Perms: Rename %s -> %s" % (dir, ptarget)) - os.rename(origin, target) + try: + os.rename(origin, target) + except OSError: + shutil.move(origin, target) #bb.note("Fixup Perms: Link %s -> %s" % (dir, link)) os.symlink(link, origin) @@ -1776,6 +1780,7 @@ python package_do_shlibs() { import itertools import re, pipes import subprocess + import shutil exclude_shlibs = d.getVar('EXCLUDE_FROM_SHLIBS', False) if exclude_shlibs: @@ -1967,7 +1972,10 @@ python package_do_shlibs() { for (old, new) in renames: bb.note("Renaming %s to %s" % (old, new)) - os.rename(old, new) + try: + os.rename(old, new) + except OSError: + shutil.move(old, new) pkgfiles[pkg].remove(old) shlibs_file = os.path.join(shlibswork_dir, pkg + ".list") diff --git a/meta/classes/populate_sdk_ext.bbclass b/meta/classes/populate_sdk_ext.bbclass index 9112ab6c5e..d2a81e7bf3 100644 --- a/meta/classes/populate_sdk_ext.bbclass +++ b/meta/classes/populate_sdk_ext.bbclass @@ -165,7 +165,10 @@ def create_filtered_tasklist(d, sdkbasepath, tasklistfile, conf_initpath): shutil.rmtree(temp_sdkbasepath) except FileNotFoundError: pass - os.rename(sdkbasepath, temp_sdkbasepath) + try: + os.rename(sdkbasepath, temp_sdkbasepath) + except OSError: + shutil.move(sdkbasepath, temp_sdkbasepath) cmdprefix = '. %s .; ' % conf_initpath logfile = d.getVar('WORKDIR') + '/tasklist_bb_log.txt' try: @@ -175,7 +178,10 @@ def create_filtered_tasklist(d, sdkbasepath, tasklistfile, conf_initpath): if 'attempted to execute unexpectedly and should have been setscened' in e.stdout: msg += '\n----------\n\nNOTE: "attempted to execute unexpectedly and should have been setscened" errors indicate this may be caused by missing sstate artifacts that were likely produced in earlier builds, but have been subsequently deleted for some reason.\n' bb.fatal(msg) - os.rename(temp_sdkbasepath, sdkbasepath) + try: + os.rename(temp_sdkbasepath, sdkbasepath) + except OSError: + shutil.move(temp_sdkbasepath, sdkbasepath) # Clean out residue of running bitbake, which check_sstate_task_list() # will effectively do clean_esdk_builddir(d, sdkbasepath) diff --git a/meta/classes/reproducible_build.bbclass b/meta/classes/reproducible_build.bbclass index f06e00d70d..480d58f86d 100644 --- a/meta/classes/reproducible_build.bbclass +++ b/meta/classes/reproducible_build.bbclass @@ -57,13 +57,17 @@ do_deploy_source_date_epoch () { } python do_deploy_source_date_epoch_setscene () { + import shutil sstate_setscene(d) bb.utils.mkdirhier(d.getVar('SDE_DIR')) sde_file = os.path.join(d.getVar('SDE_DEPLOYDIR'), '__source_date_epoch.txt') if os.path.exists(sde_file): target = d.getVar('SDE_FILE') bb.debug(1, "Moving setscene SDE file %s -> %s" % (sde_file, target)) - os.rename(sde_file, target) + try: + os.rename(sde_file, target) + except OSError: + shutil.move(sde_file, target) else: bb.debug(1, "%s not found!" % sde_file) } diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass index 8e8efd18d5..60f7a94285 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) diff --git a/meta/classes/update-alternatives.bbclass b/meta/classes/update-alternatives.bbclass index 8c2b66e7f1..7e213472e3 100644 --- a/meta/classes/update-alternatives.bbclass +++ b/meta/classes/update-alternatives.bbclass @@ -139,6 +139,7 @@ python apply_update_alternative_renames () { return import re + import shutil def update_files(alt_target, alt_target_rename, pkg, d): f = d.getVar('FILES_' + pkg) @@ -184,7 +185,10 @@ python apply_update_alternative_renames () { link_rename.append((alt_target, alt_target_rename)) else: bb.note('%s: Rename %s -> %s' % (pn, alt_target, alt_target_rename)) - os.rename(src, dest) + try: + os.rename(src, dest) + except OSError: + shutil.move(src, dest) update_files(alt_target, alt_target_rename, pkg, d) else: bb.warn("%s: alternative target (%s or %s) does not exist, skipping..." % (pn, alt_target, alt_target_rename)) @@ -201,7 +205,10 @@ python apply_update_alternative_renames () { if os.path.lexists(link_target): # Ok, the link_target exists, we can rename bb.note('%s: Rename (link) %s -> %s' % (pn, alt_target, alt_target_rename)) - os.rename(src, dest) + try: + os.rename(src, dest) + except OSError: + shutil.move(src, dest) else: # Try to resolve the broken link to link.${BPN} link_maybe = '%s.%s' % (os.readlink(src), pn) diff --git a/meta/lib/oe/package_manager/deb/__init__.py b/meta/lib/oe/package_manager/deb/__init__.py index 2ee68fefb1..8edc653dff 100644 --- a/meta/lib/oe/package_manager/deb/__init__.py +++ b/meta/lib/oe/package_manager/deb/__init__.py @@ -5,6 +5,7 @@ import re import subprocess from oe.package_manager import * +import shutil class DpkgIndexer(Indexer): def _create_configs(self): @@ -214,7 +215,10 @@ class DpkgPM(OpkgDpkgPM): tmp_sf.write(status) - os.rename(status_file + ".tmp", status_file) + try: + os.rename(status_file + ".tmp", status_file) + except OSError: + shutil.move(status_file + ".tmp", status_file) def run_pre_post_installs(self, package_name=None): """ @@ -299,13 +303,21 @@ class DpkgPM(OpkgDpkgPM): for dir in dirs: new_dir = re.sub(r"\.dpkg-new", "", dir) if dir != new_dir: - os.rename(os.path.join(root, dir), + try: + os.rename(os.path.join(root, dir), + os.path.join(root, new_dir)) + except OSError: + shutil.move(os.path.join(root, dir), os.path.join(root, new_dir)) for file in files: new_file = re.sub(r"\.dpkg-new", "", file) if file != new_file: - os.rename(os.path.join(root, file), + try: + os.rename(os.path.join(root, file), + os.path.join(root, new_file)) + except OSError: + shutil.move(os.path.join(root, file), os.path.join(root, new_file)) diff --git a/meta/lib/oe/package_manager/ipk/__init__.py b/meta/lib/oe/package_manager/ipk/__init__.py index da488c1c7f..2aa21949f3 100644 --- a/meta/lib/oe/package_manager/ipk/__init__.py +++ b/meta/lib/oe/package_manager/ipk/__init__.py @@ -213,7 +213,10 @@ class OpkgPM(OpkgDpkgPM): tmp_sf.write(status) - os.rename(status_file + ".tmp", status_file) + try: + os.rename(status_file + ".tmp", status_file) + except OSError: + shutil.move(status_file + ".tmp", status_file) def _create_custom_config(self): bb.note("Building from feeds activated!") diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py index 249c685dcf..8070fbbf9f 100644 --- a/meta/lib/oe/rootfs.py +++ b/meta/lib/oe/rootfs.py @@ -114,7 +114,10 @@ class Rootfs(object, metaclass=ABCMeta): shutil.rmtree(self.image_rootfs + '-orig') except: pass - os.rename(self.image_rootfs, self.image_rootfs + '-orig') + try: + os.rename(self.image_rootfs, self.image_rootfs + '-orig') + except OSError: + shutil.move(self.image_rootfs, self.image_rootfs + '-orig') bb.note(" Creating debug rootfs...") bb.utils.mkdirhier(self.image_rootfs) @@ -165,10 +168,16 @@ class Rootfs(object, metaclass=ABCMeta): shutil.rmtree(self.image_rootfs + '-dbg') except: pass - os.rename(self.image_rootfs, self.image_rootfs + '-dbg') + try: + os.rename(self.image_rootfs, self.image_rootfs + '-dbg') + except OSError: + shutil.move(self.image_rootfs, self.image_rootfs + '-dbg') bb.note(" Restoreing original rootfs...") - os.rename(self.image_rootfs + '-orig', self.image_rootfs) + try: + os.rename(self.image_rootfs + '-orig', self.image_rootfs) + except OSError: + shutil.move(self.image_rootfs + '-orig', self.image_rootfs) def _exec_shell_cmd(self, cmd): fakerootcmd = self.d.getVar('FAKEROOT') diff --git a/meta/lib/oeqa/selftest/cases/wic.py b/meta/lib/oeqa/selftest/cases/wic.py index fa81584a8c..fa93973087 100644 --- a/meta/lib/oeqa/selftest/cases/wic.py +++ b/meta/lib/oeqa/selftest/cases/wic.py @@ -1269,6 +1269,7 @@ class Wic2(WicTestCase): def test_expand_mbr_image(self): """Test wic write --expand command for mbr image""" + import shutil # build an image config = 'IMAGE_FSTYPES = "wic"\nWKS_FILE = "directdisk.wks"\n' self.append_config(config) @@ -1306,8 +1307,14 @@ class Wic2(WicTestCase): result = runCmd("%s/usr/sbin/sfdisk -F %s" % (sysroot, new_image_path)) self.assertTrue("0 B, 0 bytes, 0 sectors" in result.output) - os.rename(image_path, image_path + '.bak') - os.rename(new_image_path, image_path) + try: + os.rename(image_path, image_path + '.bak') + except OSError: + shutil.move(image_path, image_path + '.bak') + try: + os.rename(new_image_path, image_path) + except OSError: + shutil.move(new_image_path, image_path) # Check if it boots in qemu with runqemu('core-image-minimal', ssh=False) as qemu: @@ -1318,7 +1325,10 @@ class Wic2(WicTestCase): if os.path.exists(new_image_path): os.unlink(new_image_path) if os.path.exists(image_path + '.bak'): - os.rename(image_path + '.bak', image_path) + try: + os.rename(image_path + '.bak', image_path) + except OSError: + shutil.move(image_path + '.bak', image_path) def test_wic_ls_ext(self): """Test listing content of the ext partition using 'wic ls'""" diff --git a/scripts/combo-layer b/scripts/combo-layer index a634dd69d2..1b88340b6e 100755 --- a/scripts/combo-layer +++ b/scripts/combo-layer @@ -508,7 +508,10 @@ def check_patch(patchfile): f.close() if of: of.close() - os.rename(patchfile + '.tmp', patchfile) + try: + os.rename(patchfile + '.tmp', patchfile) + except OSError: + shutil.move(patchfile + '.tmp', patchfile) def drop_to_shell(workdir=None): if not sys.stdin.isatty(): diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py index f364a45283..3854dfb3e8 100644 --- a/scripts/lib/devtool/standard.py +++ b/scripts/lib/devtool/standard.py @@ -746,7 +746,10 @@ def _check_preserve(config, recipename): os.remove(removefile) else: tf.write(line) - os.rename(newfile, origfile) + try: + os.rename(newfile, origfile) + except OSError: + shutil.move(newfile, origfile) def get_staging_kver(srcdir): # Kernel version from work-shared @@ -1094,10 +1097,16 @@ def rename(args, config, basepath, workspace): # Rename bbappend logger.info('Renaming %s to %s' % (append, newappend)) - os.rename(append, newappend) + try: + os.rename(append, newappend) + except OSError: + shutil.move(append, newappend) # Rename recipe file logger.info('Renaming %s to %s' % (recipefile, newfile)) - os.rename(recipefile, newfile) + try: + os.rename(recipefile, newfile) + except OSError: + shutil.move(recipefile, newfile) # Rename source tree if it's the default path appendmd5 = None @@ -1333,7 +1342,11 @@ def _export_patches(srctree, rd, start_rev, destdir, changed_revs=None): if match_name: # Rename patch files if new_patch != match_name: - os.rename(os.path.join(destdir, new_patch), + try: + os.rename(os.path.join(destdir, new_patch), + os.path.join(destdir, match_name)) + except OSError: + shutil.move(os.path.join(destdir, new_patch), os.path.join(destdir, match_name)) # Need to pop it off the list now before checking changed_revs oldpath = existing_patches.pop(old_patch) diff --git a/scripts/lib/devtool/upgrade.py b/scripts/lib/devtool/upgrade.py index 5a057e95f5..2bff69f5a2 100644 --- a/scripts/lib/devtool/upgrade.py +++ b/scripts/lib/devtool/upgrade.py @@ -71,7 +71,12 @@ def _rename_recipe_dirs(oldpv, newpv, path): if oldfile.find(oldpv) != -1: newfile = oldfile.replace(oldpv, newpv) if oldfile != newfile: - os.rename(os.path.join(path, oldfile), os.path.join(path, newfile)) + try: + os.rename(os.path.join(path, oldfile), + os.path.join(path, newfile)) + except OSError: + shutil.move(os.path.join(path, oldfile), + os.path.join(path, newfile)) def _rename_recipe_file(oldrecipe, bpn, oldpv, newpv, path): oldrecipe = os.path.basename(oldrecipe) diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py index ea709e8c54..fb5e52b2a6 100644 --- a/scripts/lib/wic/plugins/imager/direct.py +++ b/scripts/lib/wic/plugins/imager/direct.py @@ -616,5 +616,8 @@ class PartitionedImage(): part.start + part.size_sec - 1, part.size_sec) partimage = self.path + '.p%d' % part.num - os.rename(source, partimage) + try: + os.rename(source, partimage) + except OSError: + shutil.move(source, partimage) self.partimages.append(partimage) -- 2.29.2 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-19 18:43 ` Devendra Tewari @ 2021-04-21 5:21 ` Khem Raj 2021-04-21 14:43 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Khem Raj @ 2021-04-21 5:21 UTC (permalink / raw) To: Devendra Tewari; +Cc: Richard Purdie, Andre McCurdy, OE Core mailing list 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 > >> > >> > > > > > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 5:21 ` Khem Raj @ 2021-04-21 14:43 ` Devendra Tewari 2021-04-21 15:15 ` Khem Raj 2021-04-21 18:10 ` Richard Purdie 0 siblings, 2 replies; 34+ messages in thread From: Devendra Tewari @ 2021-04-21 14:43 UTC (permalink / raw) To: Khem Raj; +Cc: Richard Purdie, Andre McCurdy, OE Core mailing list [-- 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 --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 14:43 ` Devendra Tewari @ 2021-04-21 15:15 ` Khem Raj 2021-04-21 18:10 ` Richard Purdie 1 sibling, 0 replies; 34+ messages in thread From: Khem Raj @ 2021-04-21 15:15 UTC (permalink / raw) To: Devendra Tewari; +Cc: Richard Purdie, Andre McCurdy, OE Core mailing list On Wed, Apr 21, 2021 at 7:43 AM Devendra Tewari <devendra.tewari@gmail.com> wrote: > > I created an image for Raspberry Pi Zero W from code at 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. > I think its not this particular patch per se. But similar patch which is in oe-core/master-next which is causing it https://git.openembedded.org/openembedded-core/commit/?h=master-next&id=11657970ff8ff89d1a9760b1c75974ba5d149ddd > 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 > > > > > > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 14:43 ` Devendra Tewari 2021-04-21 15:15 ` Khem Raj @ 2021-04-21 18:10 ` Richard Purdie 2021-04-21 19:15 ` Devendra Tewari 1 sibling, 1 reply; 34+ messages in thread From: Richard Purdie @ 2021-04-21 18:10 UTC (permalink / raw) To: Devendra Tewari, Khem Raj; +Cc: Andre McCurdy, OE Core mailing list On Wed, 2021-04-21 at 11:43 -0300, Devendra Tewari wrote: > I created an image for Raspberry Pi Zero W from code at > 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 ? I think there is some element to this we're not understanding. I'm also worried that the exception is quite generic, OSError covers a lot of things and we probably need to make it more specific. I'd suggest we add a patch changing all the call sites to use a new wrapper function in bb.utils and then we can adjust that single site to handle issues as they arise and as we understand them. Cheers, Richard ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 18:10 ` Richard Purdie @ 2021-04-21 19:15 ` Devendra Tewari 2021-04-21 19:21 ` Richard Purdie 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-21 19:15 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 1570 bytes --] That's a neat idea - if the exception handler catches OSError with error number 18 (Invalid cross-device link) in os.rename, it should attempt shutil.move, otherwise it should re-throw the exception with raise. A wrapper function will make it easier to handle other situations as they arise. I'll update the patch. Thanks, Devendra > Em 21 de abr. de 2021, à(s) 15:10, Richard Purdie <richard.purdie@linuxfoundation.org> escreveu: > > On Wed, 2021-04-21 at 11:43 -0300, Devendra Tewari wrote: >> I created an image for Raspberry Pi Zero W from code at >> 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 ? > > I think there is some element to this we're not understanding. I'm also > worried that the exception is quite generic, OSError covers a lot of things > and we probably need to make it more specific. > > I'd suggest we add a patch changing all the call sites to use a new > wrapper function in bb.utils and then we can adjust that single site to > handle issues as they arise and as we understand them. > > Cheers, > > Richard > [-- Attachment #2: Type: text/html, Size: 2831 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 19:15 ` Devendra Tewari @ 2021-04-21 19:21 ` Richard Purdie 2021-04-21 21:08 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Richard Purdie @ 2021-04-21 19:21 UTC (permalink / raw) To: Devendra Tewari; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list On Wed, 2021-04-21 at 16:15 -0300, Devendra Tewari wrote: > That's a neat idea - if the exception handler catches OSError with error number > 18 (Invalid cross-device link) in os.rename, it should attempt shutil.move, > otherwise it should re-throw the exception with raise. A wrapper function > will make it easier to handle other situations as they arise. I'll update the patch. Sounds good. Could you also please split the patch into two, one for bitbake and one for OE-Core since they are two different repositories. Cheers, Richard ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 19:21 ` Richard Purdie @ 2021-04-21 21:08 ` Devendra Tewari 2021-04-27 11:57 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-21 21:08 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 849 bytes --] Separate patches for bitbake and oe-core, as requested I've assumed each will be applied to the master-next branch on top of the previous patch. Thanks, Devendra > On 21 Apr 2021, at 16:21, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Wed, 2021-04-21 at 16:15 -0300, Devendra Tewari wrote: >> That's a neat idea - if the exception handler catches OSError with error number >> 18 (Invalid cross-device link) in os.rename, it should attempt shutil.move, >> otherwise it should re-throw the exception with raise. A wrapper function >> will make it easier to handle other situations as they arise. I'll update the patch. > > Sounds good. Could you also please split the patch into two, one for bitbake > and one for OE-Core since they are two different repositories. > > Cheers, > > Richard > [-- Attachment #2: 0001-use-bb.utils.rename-to-rename-file-or-directory.patch --] [-- Type: application/octet-stream, Size: 5658 bytes --] From ec0d5beef8a121b1afeb6e8ae72484ba690ab9e6 Mon Sep 17 00:00:00 2001 From: Devendra Tewari <devendra.tewari@gmail.com> Date: Wed, 21 Apr 2021 17:57:48 -0300 Subject: [PATCH] use bb.utils.rename to rename file or directory bb.utils.rename handles cross-device link error with os.rename and retries with shutil.move. [YOCTO #14301] Signed-off-by: Devendra Tewari <devendra.tewari@gmail.com> --- lib/bb/siggen.py | 6 +----- lib/bb/tests/fetch.py | 13 ++----------- lib/bb/utils.py | 28 +++++++++++++++------------- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py index cf89e214..574beb30 100644 --- a/lib/bb/siggen.py +++ b/lib/bb/siggen.py @@ -341,7 +341,6 @@ class SignatureGeneratorBasic(SignatureGenerator): self.unihash_cache.save(self.unitaskhashes) def dump_sigtask(self, fn, task, stampbase, runtime): - import shutil tid = fn + ":" + task referencestamp = stampbase @@ -403,10 +402,7 @@ class SignatureGeneratorBasic(SignatureGenerator): p = pickle.dump(data, stream, -1) stream.flush() os.chmod(tmpfile, 0o664) - try: - os.rename(tmpfile, sigfile) - except OSError: - shutil.move(tmpfile, sigfile) + bb.utils.rename(tmpfile, sigfile) except (OSError, IOError) as err: try: os.unlink(tmpfile) diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 89207559..9291ce4a 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -1772,7 +1772,6 @@ class GitShallowTest(FetcherTest): self.assertRevCount(1, cwd=os.path.join(self.gitdir, 'gitsubmodule')) def test_shallow_submodule_mirrors(self): - import shutil self.add_empty_file('a') self.add_empty_file('b') @@ -1799,10 +1798,7 @@ class GitShallowTest(FetcherTest): # Set up the mirror mirrordir = os.path.join(self.tempdir, 'mirror') - try: - os.rename(self.dldir, mirrordir) - except OSError: - shutil.move(self.dldir, mirrordir) + bb.utils.rename(self.dldir, mirrordir) self.d.setVar('PREMIRRORS', 'gitsm://.*/.* file://%s/\n' % mirrordir) # Fetch from the mirror @@ -1907,7 +1903,6 @@ class GitShallowTest(FetcherTest): assert not os.path.exists(os.path.join(self.gitdir, '.git', 'shallow')) def test_shallow_mirrors(self): - import shutil self.add_empty_file('a') self.add_empty_file('b') @@ -1921,11 +1916,7 @@ class GitShallowTest(FetcherTest): bb.utils.mkdirhier(mirrordir) self.d.setVar('PREMIRRORS', 'git://.*/.* file://%s/\n' % mirrordir) - try: - os.rename(os.path.join(self.dldir, mirrortarball), - os.path.join(mirrordir, mirrortarball)) - except OSError: - shutil.move(os.path.join(self.dldir, mirrortarball), + bb.utils.rename(os.path.join(self.dldir, mirrortarball), os.path.join(mirrordir, mirrortarball)) # Fetch from the mirror diff --git a/lib/bb/utils.py b/lib/bb/utils.py index f289f566..0f68b0ae 100644 --- a/lib/bb/utils.py +++ b/lib/bb/utils.py @@ -736,7 +736,6 @@ def movefile(src, dest, newmtime = None, sstat = None): filesystems. Returns true on success and false on failure. Move is atomic. """ - import shutil #print "movefile(" + src + "," + dest + "," + str(newmtime) + "," + str(sstat) + ")" try: @@ -783,10 +782,7 @@ def movefile(src, dest, newmtime = None, sstat = None): if sstat[stat.ST_DEV] == dstat[stat.ST_DEV]: try: - try: - os.rename(src, destpath) - except OSError: - shutil.move(src, destpath) + bb.utils.rename(src, destpath) renamefailed = 0 except Exception as e: if e.errno != errno.EXDEV: @@ -800,10 +796,7 @@ def movefile(src, dest, newmtime = None, sstat = None): if stat.S_ISREG(sstat[stat.ST_MODE]): try: # For safety copy then move it over. shutil.copyfile(src, destpath + "#new") - try: - os.rename(destpath + "#new", destpath) - except OSError: - shutil.move(destpath + "#new", destpath) + bb.utils.rename(destpath + "#new", destpath) didcopy = 1 except Exception as e: print('movefile: copy', src, '->', dest, 'failed.', e) @@ -881,10 +874,7 @@ def copyfile(src, dest, newmtime = None, sstat = None): # For safety copy then move it over. shutil.copyfile(src, dest + "#new") - try: - os.rename(dest + "#new", dest) - except OSError: - shutil.move(dest + "#new", dest) + bb.utils.rename(dest + "#new", dest) except Exception as e: logger.warning("copyfile: copy %s to %s failed (%s)" % (src, dest, e)) return False @@ -1679,3 +1669,15 @@ def is_semver(version): return False return True + +def rename(src, dst): + import os + import shutil + try: + os.rename(src, dst) + except OSError as err: + if err.errno == 18: + # Invalid cross-device link error + shutil.move(src, dst) + else: + raise err -- 2.29.2 [-- Attachment #3: 0001-use-bb.utils.rename-to-rename-file-or-directory.patch --] [-- Type: application/octet-stream, Size: 20677 bytes --] From e5af200e902edfbec9e2017ff6b008dbb40b6379 Mon Sep 17 00:00:00 2001 From: Devendra Tewari <devendra.tewari@gmail.com> Date: Wed, 21 Apr 2021 17:49:48 -0300 Subject: [PATCH] use bb.utils.rename to rename file or directory bb.utils.rename handles cross-device link error with os.rename and retries with shutil.move. [YOCTO #14301] Signed-off-by: Devendra Tewari <devendra.tewari@gmail.com> --- meta/classes/buildhistory.bbclass | 6 +----- meta/classes/package.bbclass | 12 ++--------- meta/classes/populate_sdk_ext.bbclass | 10 ++-------- meta/classes/reproducible_build.bbclass | 6 +----- meta/classes/sstate.bbclass | 22 ++++----------------- meta/classes/update-alternatives.bbclass | 11 ++--------- meta/lib/oe/package_manager/deb/__init__.py | 18 +++-------------- meta/lib/oe/package_manager/ipk/__init__.py | 5 +---- meta/lib/oe/rootfs.py | 15 +++----------- meta/lib/oeqa/selftest/cases/wic.py | 16 +++------------ scripts/combo-layer | 5 +---- scripts/lib/devtool/standard.py | 21 ++++---------------- scripts/lib/devtool/upgrade.py | 8 ++------ scripts/lib/wic/plugins/imager/direct.py | 5 +---- 14 files changed, 30 insertions(+), 130 deletions(-) diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass index 554c8810ac..059de36a5d 100644 --- a/meta/classes/buildhistory.bbclass +++ b/meta/classes/buildhistory.bbclass @@ -875,11 +875,7 @@ python buildhistory_eventhandler() { entries = [ x for x in os.listdir(rootdir) if not x.startswith('.') ] bb.utils.mkdirhier(olddir) for entry in entries: - try: - os.rename(os.path.join(rootdir, entry), - os.path.join(olddir, entry)) - except OSError: - shutil.move(os.path.join(rootdir, entry), + bb.utils.rename(os.path.join(rootdir, entry), os.path.join(olddir, entry)) elif isinstance(e, bb.event.BuildCompleted): if reset: diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass index 6707c2e9db..cf30f33f3d 100644 --- a/meta/classes/package.bbclass +++ b/meta/classes/package.bbclass @@ -835,7 +835,6 @@ perform_packagecopy[dirs] = "${PKGD}" # the fs-perms.txt files python fixup_perms () { import pwd, grp - import shutil # init using a string with the same format as a line as documented in # the fs-perms.txt file @@ -1050,10 +1049,7 @@ python fixup_perms () { # Create path to move directory to, move it, and then setup the symlink bb.utils.mkdirhier(os.path.dirname(target)) #bb.note("Fixup Perms: Rename %s -> %s" % (dir, ptarget)) - try: - os.rename(origin, target) - except OSError: - shutil.move(origin, target) + bb.utils.rename(origin, target) #bb.note("Fixup Perms: Link %s -> %s" % (dir, link)) os.symlink(link, origin) @@ -1780,7 +1776,6 @@ python package_do_shlibs() { import itertools import re, pipes import subprocess - import shutil exclude_shlibs = d.getVar('EXCLUDE_FROM_SHLIBS', False) if exclude_shlibs: @@ -1972,10 +1967,7 @@ python package_do_shlibs() { for (old, new) in renames: bb.note("Renaming %s to %s" % (old, new)) - try: - os.rename(old, new) - except OSError: - shutil.move(old, new) + bb.utils.rename(old, new) pkgfiles[pkg].remove(old) shlibs_file = os.path.join(shlibswork_dir, pkg + ".list") diff --git a/meta/classes/populate_sdk_ext.bbclass b/meta/classes/populate_sdk_ext.bbclass index 12e758c2ad..fe840d9cfb 100644 --- a/meta/classes/populate_sdk_ext.bbclass +++ b/meta/classes/populate_sdk_ext.bbclass @@ -165,10 +165,7 @@ def create_filtered_tasklist(d, sdkbasepath, tasklistfile, conf_initpath): shutil.rmtree(temp_sdkbasepath) except FileNotFoundError: pass - try: - os.rename(sdkbasepath, temp_sdkbasepath) - except OSError: - shutil.move(sdkbasepath, temp_sdkbasepath) + bb.utils.rename(sdkbasepath, temp_sdkbasepath) cmdprefix = '. %s .; ' % conf_initpath logfile = d.getVar('WORKDIR') + '/tasklist_bb_log.txt' try: @@ -178,10 +175,7 @@ def create_filtered_tasklist(d, sdkbasepath, tasklistfile, conf_initpath): if 'attempted to execute unexpectedly and should have been setscened' in e.stdout: msg += '\n----------\n\nNOTE: "attempted to execute unexpectedly and should have been setscened" errors indicate this may be caused by missing sstate artifacts that were likely produced in earlier builds, but have been subsequently deleted for some reason.\n' bb.fatal(msg) - try: - os.rename(temp_sdkbasepath, sdkbasepath) - except OSError: - shutil.move(temp_sdkbasepath, sdkbasepath) + bb.utils.rename(temp_sdkbasepath, sdkbasepath) # Clean out residue of running bitbake, which check_sstate_task_list() # will effectively do clean_esdk_builddir(d, sdkbasepath) diff --git a/meta/classes/reproducible_build.bbclass b/meta/classes/reproducible_build.bbclass index 480d58f86d..f2d9324440 100644 --- a/meta/classes/reproducible_build.bbclass +++ b/meta/classes/reproducible_build.bbclass @@ -57,17 +57,13 @@ do_deploy_source_date_epoch () { } python do_deploy_source_date_epoch_setscene () { - import shutil sstate_setscene(d) bb.utils.mkdirhier(d.getVar('SDE_DIR')) sde_file = os.path.join(d.getVar('SDE_DEPLOYDIR'), '__source_date_epoch.txt') if os.path.exists(sde_file): target = d.getVar('SDE_FILE') bb.debug(1, "Moving setscene SDE file %s -> %s" % (sde_file, target)) - try: - os.rename(sde_file, target) - except OSError: - shutil.move(sde_file, target) + bb.utils.rename(sde_file, target) else: bb.debug(1, "%s not found!" % sde_file) } diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass index 60f7a94285..b1c608dcb1 100644 --- a/meta/classes/sstate.bbclass +++ b/meta/classes/sstate.bbclass @@ -384,7 +384,6 @@ 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']) @@ -402,10 +401,7 @@ def sstate_installpkgdir(ss, d): for state in ss['dirs']: prepdir(state[1]) - try: - os.rename(sstateinst + state[0], state[1]) - except OSError: - shutil.move(sstateinst + state[0], state[1]) + bb.utils.rename(sstateinst + state[0], state[1]) sstate_install(ss, d) for plain in ss['plaindirs']: @@ -417,10 +413,7 @@ def sstate_installpkgdir(ss, d): dest = plain bb.utils.mkdirhier(src) prepdir(dest) - try: - os.rename(src, dest) - except OSError: - shutil.move(src, dest) + bb.utils.rename(src, dest) return True @@ -645,7 +638,6 @@ python sstate_hardcode_path () { def sstate_package(ss, d): import oe.path - import shutil tmpdir = d.getVar('TMPDIR') @@ -672,10 +664,7 @@ 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])) - try: - os.rename(state[1], sstatebuild + state[0]) - except OSError: - shutil.move(state[1], sstatebuild + state[0]) + bb.utils.rename(state[1], sstatebuild + state[0]) workdir = d.getVar('WORKDIR') sharedworkdir = os.path.join(d.getVar('TMPDIR'), "work-shared") @@ -685,10 +674,7 @@ def sstate_package(ss, d): pdir = plain.replace(sharedworkdir, sstatebuild) bb.utils.mkdirhier(plain) bb.utils.mkdirhier(pdir) - try: - os.rename(plain, pdir) - except OSError: - shutil.move(plain, pdir) + bb.utils.rename(plain, pdir) d.setVar('SSTATE_BUILDDIR', sstatebuild) d.setVar('SSTATE_INSTDIR', sstatebuild) diff --git a/meta/classes/update-alternatives.bbclass b/meta/classes/update-alternatives.bbclass index 7e213472e3..000e4d5664 100644 --- a/meta/classes/update-alternatives.bbclass +++ b/meta/classes/update-alternatives.bbclass @@ -139,7 +139,6 @@ python apply_update_alternative_renames () { return import re - import shutil def update_files(alt_target, alt_target_rename, pkg, d): f = d.getVar('FILES_' + pkg) @@ -185,10 +184,7 @@ python apply_update_alternative_renames () { link_rename.append((alt_target, alt_target_rename)) else: bb.note('%s: Rename %s -> %s' % (pn, alt_target, alt_target_rename)) - try: - os.rename(src, dest) - except OSError: - shutil.move(src, dest) + bb.utils.rename(src, dest) update_files(alt_target, alt_target_rename, pkg, d) else: bb.warn("%s: alternative target (%s or %s) does not exist, skipping..." % (pn, alt_target, alt_target_rename)) @@ -205,10 +201,7 @@ python apply_update_alternative_renames () { if os.path.lexists(link_target): # Ok, the link_target exists, we can rename bb.note('%s: Rename (link) %s -> %s' % (pn, alt_target, alt_target_rename)) - try: - os.rename(src, dest) - except OSError: - shutil.move(src, dest) + bb.utils.rename(src, dest) else: # Try to resolve the broken link to link.${BPN} link_maybe = '%s.%s' % (os.readlink(src), pn) diff --git a/meta/lib/oe/package_manager/deb/__init__.py b/meta/lib/oe/package_manager/deb/__init__.py index 8edc653dff..a4b6b6f647 100644 --- a/meta/lib/oe/package_manager/deb/__init__.py +++ b/meta/lib/oe/package_manager/deb/__init__.py @@ -5,7 +5,6 @@ import re import subprocess from oe.package_manager import * -import shutil class DpkgIndexer(Indexer): def _create_configs(self): @@ -215,10 +214,7 @@ class DpkgPM(OpkgDpkgPM): tmp_sf.write(status) - try: - os.rename(status_file + ".tmp", status_file) - except OSError: - shutil.move(status_file + ".tmp", status_file) + bb.utils.rename(status_file + ".tmp", status_file) def run_pre_post_installs(self, package_name=None): """ @@ -303,21 +299,13 @@ class DpkgPM(OpkgDpkgPM): for dir in dirs: new_dir = re.sub(r"\.dpkg-new", "", dir) if dir != new_dir: - try: - os.rename(os.path.join(root, dir), - os.path.join(root, new_dir)) - except OSError: - shutil.move(os.path.join(root, dir), + bb.utils.rename(os.path.join(root, dir), os.path.join(root, new_dir)) for file in files: new_file = re.sub(r"\.dpkg-new", "", file) if file != new_file: - try: - os.rename(os.path.join(root, file), - os.path.join(root, new_file)) - except OSError: - shutil.move(os.path.join(root, file), + bb.utils.rename(os.path.join(root, file), os.path.join(root, new_file)) diff --git a/meta/lib/oe/package_manager/ipk/__init__.py b/meta/lib/oe/package_manager/ipk/__init__.py index 2aa21949f3..4cd3963111 100644 --- a/meta/lib/oe/package_manager/ipk/__init__.py +++ b/meta/lib/oe/package_manager/ipk/__init__.py @@ -213,10 +213,7 @@ class OpkgPM(OpkgDpkgPM): tmp_sf.write(status) - try: - os.rename(status_file + ".tmp", status_file) - except OSError: - shutil.move(status_file + ".tmp", status_file) + bb.utils.rename(status_file + ".tmp", status_file) def _create_custom_config(self): bb.note("Building from feeds activated!") diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py index ef00010047..a7cc4e084a 100644 --- a/meta/lib/oe/rootfs.py +++ b/meta/lib/oe/rootfs.py @@ -114,10 +114,7 @@ class Rootfs(object, metaclass=ABCMeta): shutil.rmtree(self.image_rootfs + '-orig') except: pass - try: - os.rename(self.image_rootfs, self.image_rootfs + '-orig') - except OSError: - shutil.move(self.image_rootfs, self.image_rootfs + '-orig') + bb.utils.rename(self.image_rootfs, self.image_rootfs + '-orig') bb.note(" Creating debug rootfs...") bb.utils.mkdirhier(self.image_rootfs) @@ -168,16 +165,10 @@ class Rootfs(object, metaclass=ABCMeta): shutil.rmtree(self.image_rootfs + '-dbg') except: pass - try: - os.rename(self.image_rootfs, self.image_rootfs + '-dbg') - except OSError: - shutil.move(self.image_rootfs, self.image_rootfs + '-dbg') + bb.utils.rename(self.image_rootfs, self.image_rootfs + '-dbg') bb.note(" Restoreing original rootfs...") - try: - os.rename(self.image_rootfs + '-orig', self.image_rootfs) - except OSError: - shutil.move(self.image_rootfs + '-orig', self.image_rootfs) + bb.utils.rename(self.image_rootfs + '-orig', self.image_rootfs) def _exec_shell_cmd(self, cmd): fakerootcmd = self.d.getVar('FAKEROOT') diff --git a/meta/lib/oeqa/selftest/cases/wic.py b/meta/lib/oeqa/selftest/cases/wic.py index fa93973087..a11e2d0781 100644 --- a/meta/lib/oeqa/selftest/cases/wic.py +++ b/meta/lib/oeqa/selftest/cases/wic.py @@ -1269,7 +1269,6 @@ class Wic2(WicTestCase): def test_expand_mbr_image(self): """Test wic write --expand command for mbr image""" - import shutil # build an image config = 'IMAGE_FSTYPES = "wic"\nWKS_FILE = "directdisk.wks"\n' self.append_config(config) @@ -1307,14 +1306,8 @@ class Wic2(WicTestCase): result = runCmd("%s/usr/sbin/sfdisk -F %s" % (sysroot, new_image_path)) self.assertTrue("0 B, 0 bytes, 0 sectors" in result.output) - try: - os.rename(image_path, image_path + '.bak') - except OSError: - shutil.move(image_path, image_path + '.bak') - try: - os.rename(new_image_path, image_path) - except OSError: - shutil.move(new_image_path, image_path) + bb.utils.rename(image_path, image_path + '.bak') + bb.utils.rename(new_image_path, image_path) # Check if it boots in qemu with runqemu('core-image-minimal', ssh=False) as qemu: @@ -1325,10 +1318,7 @@ class Wic2(WicTestCase): if os.path.exists(new_image_path): os.unlink(new_image_path) if os.path.exists(image_path + '.bak'): - try: - os.rename(image_path + '.bak', image_path) - except OSError: - shutil.move(image_path + '.bak', image_path) + bb.utils.rename(image_path + '.bak', image_path) def test_wic_ls_ext(self): """Test listing content of the ext partition using 'wic ls'""" diff --git a/scripts/combo-layer b/scripts/combo-layer index 1b88340b6e..045de65642 100755 --- a/scripts/combo-layer +++ b/scripts/combo-layer @@ -508,10 +508,7 @@ def check_patch(patchfile): f.close() if of: of.close() - try: - os.rename(patchfile + '.tmp', patchfile) - except OSError: - shutil.move(patchfile + '.tmp', patchfile) + bb.utils.rename(patchfile + '.tmp', patchfile) def drop_to_shell(workdir=None): if not sys.stdin.isatty(): diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py index 3854dfb3e8..5eba2191d9 100644 --- a/scripts/lib/devtool/standard.py +++ b/scripts/lib/devtool/standard.py @@ -746,10 +746,7 @@ def _check_preserve(config, recipename): os.remove(removefile) else: tf.write(line) - try: - os.rename(newfile, origfile) - except OSError: - shutil.move(newfile, origfile) + bb.utils.rename(newfile, origfile) def get_staging_kver(srcdir): # Kernel version from work-shared @@ -1097,16 +1094,10 @@ def rename(args, config, basepath, workspace): # Rename bbappend logger.info('Renaming %s to %s' % (append, newappend)) - try: - os.rename(append, newappend) - except OSError: - shutil.move(append, newappend) + bb.utils.rename(append, newappend) # Rename recipe file logger.info('Renaming %s to %s' % (recipefile, newfile)) - try: - os.rename(recipefile, newfile) - except OSError: - shutil.move(recipefile, newfile) + bb.utils.rename(recipefile, newfile) # Rename source tree if it's the default path appendmd5 = None @@ -1342,11 +1333,7 @@ def _export_patches(srctree, rd, start_rev, destdir, changed_revs=None): if match_name: # Rename patch files if new_patch != match_name: - try: - os.rename(os.path.join(destdir, new_patch), - os.path.join(destdir, match_name)) - except OSError: - shutil.move(os.path.join(destdir, new_patch), + bb.utils.rename(os.path.join(destdir, new_patch), os.path.join(destdir, match_name)) # Need to pop it off the list now before checking changed_revs oldpath = existing_patches.pop(old_patch) diff --git a/scripts/lib/devtool/upgrade.py b/scripts/lib/devtool/upgrade.py index 2bff69f5a2..24e3700ece 100644 --- a/scripts/lib/devtool/upgrade.py +++ b/scripts/lib/devtool/upgrade.py @@ -71,12 +71,8 @@ def _rename_recipe_dirs(oldpv, newpv, path): if oldfile.find(oldpv) != -1: newfile = oldfile.replace(oldpv, newpv) if oldfile != newfile: - try: - os.rename(os.path.join(path, oldfile), - os.path.join(path, newfile)) - except OSError: - shutil.move(os.path.join(path, oldfile), - os.path.join(path, newfile)) + bb.utils.rename(os.path.join(path, oldfile), + os.path.join(path, newfile)) def _rename_recipe_file(oldrecipe, bpn, oldpv, newpv, path): oldrecipe = os.path.basename(oldrecipe) diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py index fb5e52b2a6..96168aadb4 100644 --- a/scripts/lib/wic/plugins/imager/direct.py +++ b/scripts/lib/wic/plugins/imager/direct.py @@ -616,8 +616,5 @@ class PartitionedImage(): part.start + part.size_sec - 1, part.size_sec) partimage = self.path + '.p%d' % part.num - try: - os.rename(source, partimage) - except OSError: - shutil.move(source, partimage) + bb.utils.rename(source, partimage) self.partimages.append(partimage) -- 2.29.2 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-21 21:08 ` Devendra Tewari @ 2021-04-27 11:57 ` Devendra Tewari 2021-04-27 14:59 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-27 11:57 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 7936 bytes --] These patches are working well at my end, but I’m still seeing the following error in do_rootfs, when building incrementally in a container. I’m not sure if this is due to timing issues alluded to earlier or something else entirely. I do know for sure that this is not caused by the patches because it happens even without them. ERROR: core-image-base-1.0-r0 do_rootfs: Error executing a python function in exec_python_func() autogenerated: The stack trace of python calls that resulted in this exception/failure was: File: 'exec_python_func() autogenerated', lineno: 2, function: <module> 0001: *** 0002:do_rootfs(d) 0003: File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/classes/image.bbclass', lineno: 247, function: do_rootfs 0243: progress_reporter.next_stage() 0244: 0245: # generate rootfs 0246: d.setVarFlag('REPRODUCIBLE_TIMESTAMP_ROOTFS', 'export', '1') *** 0247: create_rootfs(d, progress_reporter=progress_reporter, logcatcher=logcatcher) 0248: 0249: progress_reporter.finish() 0250:} 0251:do_rootfs[dirs] = "${TOPDIR}" File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/rootfs.py', lineno: 375, function: create_rootfs 0371: 0372: img_type = d.getVar('IMAGE_PKGTYPE') 0373: 0374: cls = get_class_for_type(img_type) *** 0375: cls(d, manifest_dir, progress_reporter, logcatcher).create() 0376: os.environ.clear() 0377: os.environ.update(env_bkp) 0378: 0379: File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/rootfs.py', lineno: 203, function: create 0199: if self.progress_reporter: 0200: self.progress_reporter.next_stage() 0201: 0202: # call the package manager dependent create method *** 0203: self._create() 0204: 0205: sysconfdir = self.image_rootfs + self.d.getVar('sysconfdir') 0206: bb.utils.mkdirhier(sysconfdir) 0207: with open(sysconfdir + "/version", "w+") as ver: File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/rootfs.py', lineno: 70, function: _create 0066: rpm_pre_process_cmds = self.d.getVar('RPM_PREPROCESS_COMMANDS') 0067: rpm_post_process_cmds = self.d.getVar('RPM_POSTPROCESS_COMMANDS') 0068: 0069: # update PM index files *** 0070: self.pm.write_index() 0071: 0072: execute_pre_post_process(self.d, rpm_pre_process_cmds) 0073: 0074: if self.progress_reporter: File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/__init__.py', lineno: 141, function: write_index 0137: 0138: def write_index(self): 0139: lockfilename = self.d.getVar('DEPLOY_DIR_RPM') + "/rpm.lock" 0140: lf = bb.utils.lockfile(lockfilename, False) *** 0141: RpmIndexer(self.d, self.rpm_repo_dir).write_index() 0142: bb.utils.unlockfile(lf) 0143: 0144: def insert_feeds_uris(self, feed_uris, feed_base_paths, feed_archs): 0145: from urllib.parse import urlparse File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/__init__.py', lineno: 11, function: write_index 0007:from oe.package_manager import * 0008: 0009:class RpmIndexer(Indexer): 0010: def write_index(self): *** 0011: self.do_write_index(self.deploy_dir) 0012: 0013: def do_write_index(self, deploy_dir): 0014: if self.d.getVar('PACKAGE_FEED_SIGN') == '1': 0015: signer = get_signer(self.d, self.d.getVar('PACKAGE_FEED_GPG_BACKEND')) File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/__init__.py', lineno: 20, function: do_write_index 0016: else: 0017: signer = None 0018: 0019: createrepo_c = bb.utils.which(os.environ['PATH'], "createrepo_c") *** 0020: result = create_index("%s --update -q %s" % (createrepo_c, deploy_dir)) 0021: if result: 0022: bb.fatal(result) 0023: 0024: # Sign repomd File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/__init__.py', lineno: 26, function: create_index 0022:def create_index(arg): 0023: index_cmd = arg 0024: 0025: bb.note("Executing '%s' ..." % index_cmd) *** 0026: result = subprocess.check_output(index_cmd, stderr=subprocess.STDOUT, shell=True).decode("utf-8") 0027: if result: 0028: bb.note(result) 0029: 0030:def opkg_query(cmd_output): File: '/usr/lib/python3.8/subprocess.py', lineno: 411, function: check_output 0407: # Explicitly passing input=None was previously equivalent to passing an 0408: # empty string. That is maintained here for backwards compatibility. 0409: kwargs['input'] = '' if kwargs.get('universal_newlines', False) else b'' 0410: *** 0411: return run(*popenargs, stdout=PIPE, timeout=timeout, check=True, 0412: **kwargs).stdout 0413: 0414: 0415:class CompletedProcess(object): File: '/usr/lib/python3.8/subprocess.py', lineno: 512, function: run 0508: # We don't call process.wait() as .__exit__ does that for us. 0509: raise 0510: retcode = process.poll() 0511: if check and retcode: *** 0512: raise CalledProcessError(retcode, process.args, 0513: output=stdout, stderr=stderr) 0514: return CompletedProcess(process.args, retcode, stdout, stderr) 0515: 0516: Exception: subprocess.CalledProcessError: Command '/home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/recipe-sysroot-native/usr/bin/createrepo_c --update -q /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/oe-rootfs-repo' returned non-zero exit status 1. Subprocess output: Critical: Cannot rename /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/oe-rootfs-repo/.repodata/ -> /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/oe-rootfs-repo/repodata/: Directory not empty ERROR: Logfile of failure stored in: /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/temp/log.do_rootfs.165072 ERROR: Task (/home/pi/docker-meta-raspberrypi/layers/poky/meta/recipes-core/images/core-image-base.bb:do_rootfs) failed with exit code '1' > On 21 Apr 2021, at 18:08, Devendra Tewari <devendra.tewari@gmail.com> wrote: > > Separate patches for bitbake and oe-core, as requested > > <0001-use-bb.utils.rename-to-rename-file-or-directory.patch><0001-use-bb.utils.rename-to-rename-file-or-directory.patch> > > I've assumed each will be applied to the master-next branch on top of the previous patch. > > Thanks, > Devendra > >> On 21 Apr 2021, at 16:21, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: >> >> On Wed, 2021-04-21 at 16:15 -0300, Devendra Tewari wrote: >>> That's a neat idea - if the exception handler catches OSError with error number >>> 18 (Invalid cross-device link) in os.rename, it should attempt shutil.move, >>> otherwise it should re-throw the exception with raise. A wrapper function >>> will make it easier to handle other situations as they arise. I'll update the patch. >> >> Sounds good. Could you also please split the patch into two, one for bitbake >> and one for OE-Core since they are two different repositories. >> >> Cheers, >> >> Richard >> > [-- Attachment #2: Type: text/html, Size: 13067 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-27 11:57 ` Devendra Tewari @ 2021-04-27 14:59 ` Devendra Tewari 2021-04-27 22:39 ` Richard Purdie 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-27 14:59 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list [-- Attachment #1: Type: text/plain, Size: 8879 bytes --] I suspect this is an issue in upstream createrepo_c tool, which has code that does not work across overlay file systems if (g_rename(out_repo, old_repodata_path) == -1) { g_debug("Old repodata doesn't exists: Cannot rename %s -> %s: %s", out_repo, old_repodata_path, g_strerror(errno)); } else { g_debug("Renamed %s -> %s", out_repo, old_repodata_path); old_repodata_renamed = TRUE; } I’ll try patching and report an issue to upstream project. > On 27 Apr 2021, at 08:57, Devendra Tewari <devendra.tewari@gmail.com> wrote: > > These patches are working well at my end, but I’m still seeing the following error in do_rootfs, when building incrementally in a container. I’m not sure if this is due to timing issues alluded to earlier or something else entirely. I do know for sure that this is not caused by the patches because it happens even without them. > > ERROR: core-image-base-1.0-r0 do_rootfs: Error executing a python function in exec_python_func() autogenerated: > > The stack trace of python calls that resulted in this exception/failure was: > File: 'exec_python_func() autogenerated', lineno: 2, function: <module> > 0001: > *** 0002:do_rootfs(d) > 0003: > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/classes/image.bbclass', lineno: 247, function: do_rootfs > 0243: progress_reporter.next_stage() > 0244: > 0245: # generate rootfs > 0246: d.setVarFlag('REPRODUCIBLE_TIMESTAMP_ROOTFS', 'export', '1') > *** 0247: create_rootfs(d, progress_reporter=progress_reporter, logcatcher=logcatcher) > 0248: > 0249: progress_reporter.finish() > 0250:} > 0251:do_rootfs[dirs] = "${TOPDIR}" > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/rootfs.py', lineno: 375, function: create_rootfs > 0371: > 0372: img_type = d.getVar('IMAGE_PKGTYPE') > 0373: > 0374: cls = get_class_for_type(img_type) > *** 0375: cls(d, manifest_dir, progress_reporter, logcatcher).create() > 0376: os.environ.clear() > 0377: os.environ.update(env_bkp) > 0378: > 0379: > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/rootfs.py', lineno: 203, function: create > 0199: if self.progress_reporter: > 0200: self.progress_reporter.next_stage() > 0201: > 0202: # call the package manager dependent create method > *** 0203: self._create() > 0204: > 0205: sysconfdir = self.image_rootfs + self.d.getVar('sysconfdir') > 0206: bb.utils.mkdirhier(sysconfdir) > 0207: with open(sysconfdir + "/version", "w+") as ver: > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/rootfs.py', lineno: 70, function: _create > 0066: rpm_pre_process_cmds = self.d.getVar('RPM_PREPROCESS_COMMANDS') > 0067: rpm_post_process_cmds = self.d.getVar('RPM_POSTPROCESS_COMMANDS') > 0068: > 0069: # update PM index files > *** 0070: self.pm.write_index() > 0071: > 0072: execute_pre_post_process(self.d, rpm_pre_process_cmds) > 0073: > 0074: if self.progress_reporter: > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/__init__.py', lineno: 141, function: write_index > 0137: > 0138: def write_index(self): > 0139: lockfilename = self.d.getVar('DEPLOY_DIR_RPM') + "/rpm.lock" > 0140: lf = bb.utils.lockfile(lockfilename, False) > *** 0141: RpmIndexer(self.d, self.rpm_repo_dir).write_index() > 0142: bb.utils.unlockfile(lf) > 0143: > 0144: def insert_feeds_uris(self, feed_uris, feed_base_paths, feed_archs): > 0145: from urllib.parse import urlparse > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/__init__.py', lineno: 11, function: write_index > 0007:from oe.package_manager import * > 0008: > 0009:class RpmIndexer(Indexer): > 0010: def write_index(self): > *** 0011: self.do_write_index(self.deploy_dir) > 0012: > 0013: def do_write_index(self, deploy_dir): > 0014: if self.d.getVar('PACKAGE_FEED_SIGN') == '1': > 0015: signer = get_signer(self.d, self.d.getVar('PACKAGE_FEED_GPG_BACKEND')) > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/rpm/__init__.py', lineno: 20, function: do_write_index > 0016: else: > 0017: signer = None > 0018: > 0019: createrepo_c = bb.utils.which(os.environ['PATH'], "createrepo_c") > *** 0020: result = create_index("%s --update -q %s" % (createrepo_c, deploy_dir)) > 0021: if result: > 0022: bb.fatal(result) > 0023: > 0024: # Sign repomd > File: '/home/pi/docker-meta-raspberrypi/layers/poky/meta/lib/oe/package_manager/__init__.py', lineno: 26, function: create_index > 0022:def create_index(arg): > 0023: index_cmd = arg > 0024: > 0025: bb.note("Executing '%s' ..." % index_cmd) > *** 0026: result = subprocess.check_output(index_cmd, stderr=subprocess.STDOUT, shell=True).decode("utf-8") > 0027: if result: > 0028: bb.note(result) > 0029: > 0030:def opkg_query(cmd_output): > File: '/usr/lib/python3.8/subprocess.py', lineno: 411, function: check_output > 0407: # Explicitly passing input=None was previously equivalent to passing an > 0408: # empty string. That is maintained here for backwards compatibility. > 0409: kwargs['input'] = '' if kwargs.get('universal_newlines', False) else b'' > 0410: > *** 0411: return run(*popenargs, stdout=PIPE, timeout=timeout, check=True, > 0412: **kwargs).stdout > 0413: > 0414: > 0415:class CompletedProcess(object): > File: '/usr/lib/python3.8/subprocess.py', lineno: 512, function: run > 0508: # We don't call process.wait() as .__exit__ does that for us. > 0509: raise > 0510: retcode = process.poll() > 0511: if check and retcode: > *** 0512: raise CalledProcessError(retcode, process.args, > 0513: output=stdout, stderr=stderr) > 0514: return CompletedProcess(process.args, retcode, stdout, stderr) > 0515: > 0516: > Exception: subprocess.CalledProcessError: Command '/home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/recipe-sysroot-native/usr/bin/createrepo_c --update -q /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/oe-rootfs-repo' returned non-zero exit status 1. > > Subprocess output: > Critical: Cannot rename /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/oe-rootfs-repo/.repodata/ -> /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/oe-rootfs-repo/repodata/: Directory not empty > > ERROR: Logfile of failure stored in: /home/pi/docker-meta-raspberrypi/build/tmp/work/raspberrypi0_wifi-poky-linux-gnueabi/core-image-base/1.0-r0/temp/log.do_rootfs.165072 > ERROR: Task (/home/pi/docker-meta-raspberrypi/layers/poky/meta/recipes-core/images/core-image-base.bb:do_rootfs) failed with exit code '1' > > >> On 21 Apr 2021, at 18:08, Devendra Tewari <devendra.tewari@gmail.com <mailto:devendra.tewari@gmail.com>> wrote: >> >> Separate patches for bitbake and oe-core, as requested >> >> <0001-use-bb.utils.rename-to-rename-file-or-directory.patch><0001-use-bb.utils.rename-to-rename-file-or-directory.patch> >> >> I've assumed each will be applied to the master-next branch on top of the previous patch. >> >> Thanks, >> Devendra >> >>> On 21 Apr 2021, at 16:21, Richard Purdie <richard.purdie@linuxfoundation.org <mailto:richard.purdie@linuxfoundation.org>> wrote: >>> >>> On Wed, 2021-04-21 at 16:15 -0300, Devendra Tewari wrote: >>>> That's a neat idea - if the exception handler catches OSError with error number >>>> 18 (Invalid cross-device link) in os.rename, it should attempt shutil.move, >>>> otherwise it should re-throw the exception with raise. A wrapper function >>>> will make it easier to handle other situations as they arise. I'll update the patch. >>> >>> Sounds good. Could you also please split the patch into two, one for bitbake >>> and one for OE-Core since they are two different repositories. >>> >>> Cheers, >>> >>> Richard >>> >> > [-- Attachment #2: Type: text/html, Size: 14488 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-27 14:59 ` Devendra Tewari @ 2021-04-27 22:39 ` Richard Purdie 2021-04-27 22:44 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Richard Purdie @ 2021-04-27 22:39 UTC (permalink / raw) To: Devendra Tewari; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list On Tue, 2021-04-27 at 11:59 -0300, Devendra Tewari wrote: > I suspect this is an issue in upstream createrepo_c tool, which has code that does not work across overlay > file systems > > if (g_rename(out_repo, old_repodata_path) == -1) { > g_debug("Old repodata doesn't exists: Cannot rename %s -> %s: %s", > out_repo, old_repodata_path, g_strerror(errno)); > } else { > g_debug("Renamed %s -> %s", out_repo, old_repodata_path); > old_repodata_renamed = TRUE; > } > > I’ll try patching and report an issue to upstream project. I'm starting to worry that you're going to have this problem with many tools doing renames. It does sound like something the container filesystem should be handling rather than exposing as a difference in behaviour. I suspect it becomes hard to see predictably since builds writing in the same build instance are probably fine and it only likely happens for files created in one instance and then later renamed in another... Given the breadth of build systems we run, I'm not sure we want to try and support this? Cheers, Richard ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-27 22:39 ` Richard Purdie @ 2021-04-27 22:44 ` Devendra Tewari 2021-04-27 22:53 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-27 22:44 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list You’re absolutely right - I’ll investigate whether this is something the container overlay filesystem should be handling. Thanks! > On 27 Apr 2021, at 19:39, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Tue, 2021-04-27 at 11:59 -0300, Devendra Tewari wrote: >> I suspect this is an issue in upstream createrepo_c tool, which has code that does not work across overlay >> file systems >> >> if (g_rename(out_repo, old_repodata_path) == -1) { >> g_debug("Old repodata doesn't exists: Cannot rename %s -> %s: %s", >> out_repo, old_repodata_path, g_strerror(errno)); >> } else { >> g_debug("Renamed %s -> %s", out_repo, old_repodata_path); >> old_repodata_renamed = TRUE; >> } >> >> I’ll try patching and report an issue to upstream project. > > I'm starting to worry that you're going to have this problem with many tools doing > renames. It does sound like something the container filesystem should be handling rather > than exposing as a difference in behaviour. I suspect it becomes hard to see > predictably since builds writing in the same build instance are probably fine > and it only likely happens for files created in one instance and then later > renamed in another... > > Given the breadth of build systems we run, I'm not sure we want to try and > support this? > > Cheers, > > Richard > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-27 22:44 ` Devendra Tewari @ 2021-04-27 22:53 ` Devendra Tewari 2021-04-28 10:00 ` Devendra Tewari 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-27 22:53 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list Looks like an old issue with overlayfs - https://github.com/moby/moby/issues/25409. > On 27 Apr 2021, at 19:44, Devendra Tewari <devendra.tewari@gmail.com> wrote: > > You’re absolutely right - I’ll investigate whether this is something the container overlay filesystem should be handling. Thanks! > >> On 27 Apr 2021, at 19:39, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: >> >> On Tue, 2021-04-27 at 11:59 -0300, Devendra Tewari wrote: >>> I suspect this is an issue in upstream createrepo_c tool, which has code that does not work across overlay >>> file systems >>> >>> if (g_rename(out_repo, old_repodata_path) == -1) { >>> g_debug("Old repodata doesn't exists: Cannot rename %s -> %s: %s", >>> out_repo, old_repodata_path, g_strerror(errno)); >>> } else { >>> g_debug("Renamed %s -> %s", out_repo, old_repodata_path); >>> old_repodata_renamed = TRUE; >>> } >>> >>> I’ll try patching and report an issue to upstream project. >> >> I'm starting to worry that you're going to have this problem with many tools doing >> renames. It does sound like something the container filesystem should be handling rather >> than exposing as a difference in behaviour. I suspect it becomes hard to see >> predictably since builds writing in the same build instance are probably fine >> and it only likely happens for files created in one instance and then later >> renamed in another... >> >> Given the breadth of build systems we run, I'm not sure we want to try and >> support this? >> >> Cheers, >> >> Richard >> > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-27 22:53 ` Devendra Tewari @ 2021-04-28 10:00 ` Devendra Tewari 2021-04-28 21:42 ` Richard Purdie 0 siblings, 1 reply; 34+ messages in thread From: Devendra Tewari @ 2021-04-28 10:00 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list The gist of it is that Docker uses the overlay2 storage driver by default, previously it used aufs. Both drivers have the same limitation in rename, where file rename across layers is handled by moving, but directory rename across layers results in error EXDEV. It is up to user space to handle the error. > On 27 Apr 2021, at 19:53, Devendra Tewari <devendra.tewari@gmail.com> wrote: > > Looks like an old issue with overlayfs - https://github.com/moby/moby/issues/25409. > >> On 27 Apr 2021, at 19:44, Devendra Tewari <devendra.tewari@gmail.com> wrote: >> >> You’re absolutely right - I’ll investigate whether this is something the container overlay filesystem should be handling. Thanks! >> >>> On 27 Apr 2021, at 19:39, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: >>> >>> On Tue, 2021-04-27 at 11:59 -0300, Devendra Tewari wrote: >>>> I suspect this is an issue in upstream createrepo_c tool, which has code that does not work across overlay >>>> file systems >>>> >>>> if (g_rename(out_repo, old_repodata_path) == -1) { >>>> g_debug("Old repodata doesn't exists: Cannot rename %s -> %s: %s", >>>> out_repo, old_repodata_path, g_strerror(errno)); >>>> } else { >>>> g_debug("Renamed %s -> %s", out_repo, old_repodata_path); >>>> old_repodata_renamed = TRUE; >>>> } >>>> >>>> I’ll try patching and report an issue to upstream project. >>> >>> I'm starting to worry that you're going to have this problem with many tools doing >>> renames. It does sound like something the container filesystem should be handling rather >>> than exposing as a difference in behaviour. I suspect it becomes hard to see >>> predictably since builds writing in the same build instance are probably fine >>> and it only likely happens for files created in one instance and then later >>> renamed in another... >>> >>> Given the breadth of build systems we run, I'm not sure we want to try and >>> support this? >>> >>> Cheers, >>> >>> Richard >>> >> > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-28 10:00 ` Devendra Tewari @ 2021-04-28 21:42 ` Richard Purdie 2021-04-28 23:02 ` Devendra Tewari [not found] ` <90B42906-44DE-4D61-8210-669DC25BBD3F@gmail.com> 0 siblings, 2 replies; 34+ messages in thread From: Richard Purdie @ 2021-04-28 21:42 UTC (permalink / raw) To: Devendra Tewari; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list On Wed, 2021-04-28 at 07:00 -0300, Devendra Tewari wrote: > The gist of it is that Docker uses the overlay2 storage driver by default, > previously it used aufs. Both drivers have the same limitation in rename, > where file rename across layers is handled by moving, but directory rename > across layers results in error EXDEV. It is up to user space to handle the error. I'm still struggling to see how you're going to patch all the various pieces of software out there used during a build to handle EXDEV correctly. We have handled some cases of this for a long time where we're moving files over to different parts of a filesystem, e.g. /tmp/ to WORKDIR or vice versa. This new docker case triggers for renames even within a directory, e.g. /some/path/a -> /some/path/b which is not something that would often trigger this. I need to say clearly now that we are not carrying/taking patches to patch every bit of other software to try and avoid this. Cheers, Richard ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails 2021-04-28 21:42 ` Richard Purdie @ 2021-04-28 23:02 ` Devendra Tewari [not found] ` <90B42906-44DE-4D61-8210-669DC25BBD3F@gmail.com> 1 sibling, 0 replies; 34+ messages in thread From: Devendra Tewari @ 2021-04-28 23:02 UTC (permalink / raw) To: Richard Purdie; +Cc: Khem Raj, Andre McCurdy, OE Core mailing list Understood - carrying patches is a burden for sure. I’ll maintain patches for my own narrow use cases at my end, and perhaps submit them upstream. Thanks, Devendra > On 28 Apr 2021, at 18:42, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Wed, 2021-04-28 at 07:00 -0300, Devendra Tewari wrote: >> The gist of it is that Docker uses the overlay2 storage driver by default, >> previously it used aufs. Both drivers have the same limitation in rename, >> where file rename across layers is handled by moving, but directory rename >> across layers results in error EXDEV. It is up to user space to handle the error. > > I'm still struggling to see how you're going to patch all the various pieces of > software out there used during a build to handle EXDEV correctly. > > We have handled some cases of this for a long time where we're moving files > over to different parts of a filesystem, e.g. /tmp/ to WORKDIR or vice versa. > > This new docker case triggers for renames even within a directory, e.g. > /some/path/a -> /some/path/b which is not something that would often trigger > this. > > I need to say clearly now that we are not carrying/taking patches to patch > every bit of other software to try and avoid this. > > Cheers, > > Richard > > > ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <90B42906-44DE-4D61-8210-669DC25BBD3F@gmail.com>]
* Re: [OE-core] [PATCH] Use shutil.move when os.rename fails [not found] ` <90B42906-44DE-4D61-8210-669DC25BBD3F@gmail.com> @ 2022-08-22 12:25 ` Devendra Tewari 0 siblings, 0 replies; 34+ messages in thread From: Devendra Tewari @ 2022-08-22 12:25 UTC (permalink / raw) To: OE Core mailing list; +Cc: Khem Raj, Andre McCurdy, Richard Purdie [-- Attachment #1: Type: text/plain, Size: 1741 bytes --] I was asked about the createrepo_c patch in private. Replying here for closure. I had reported the issue upstream at https://github.com/rpm-software-management/createrepo_c/issues/266 and it has been fixed. Poky kirkstone and mainline should have the fix. Kind Regards, Devendra > On 28 Apr 2021, at 20:02, Devendra Tewari <devendra.tewari@gmail.com> wrote: > > Understood - carrying patches is a burden for sure. I’ll maintain patches for my own narrow use cases at my end, and perhaps submit them upstream. > > Thanks, > Devendra > >> On 28 Apr 2021, at 18:42, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: >> >> On Wed, 2021-04-28 at 07:00 -0300, Devendra Tewari wrote: >>> The gist of it is that Docker uses the overlay2 storage driver by default, >>> previously it used aufs. Both drivers have the same limitation in rename, >>> where file rename across layers is handled by moving, but directory rename >>> across layers results in error EXDEV. It is up to user space to handle the error. >> >> I'm still struggling to see how you're going to patch all the various pieces of >> software out there used during a build to handle EXDEV correctly. >> >> We have handled some cases of this for a long time where we're moving files >> over to different parts of a filesystem, e.g. /tmp/ to WORKDIR or vice versa. >> >> This new docker case triggers for renames even within a directory, e.g. >> /some/path/a -> /some/path/b which is not something that would often trigger >> this. >> >> I need to say clearly now that we are not carrying/taking patches to patch >> every bit of other software to try and avoid this. >> >> Cheers, >> >> Richard >> >> >> > > > [-- Attachment #2: Type: text/html, Size: 2839 bytes --] ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2022-08-22 12:25 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-29 15:14 [PATCH] Use shutil.move when os.rename fails devendra.tewari
2021-03-29 15:21 ` [OE-core] " Bruce Ashfield
2021-03-29 15:23 ` Konrad Weihmann
2021-03-29 15:35 ` Devendra Tewari
2021-03-29 15:37 ` Devendra Tewari
2021-03-30 21:59 ` Denys Dmytriyenko
2021-03-30 22:38 ` Devendra Tewari
2021-04-01 19:14 ` Devendra Tewari
2021-03-29 20:38 ` Richard Purdie
2021-03-29 22:45 ` Devendra Tewari
2021-03-29 23:00 ` Andre McCurdy
2021-03-29 23:07 ` Devendra Tewari
2021-03-29 23:10 ` Andre McCurdy
2021-03-29 23:13 ` Devendra Tewari
2021-03-30 10:16 ` Devendra Tewari
2021-03-30 11:10 ` Richard Purdie
2021-03-30 11:55 ` Devendra Tewari
2021-04-19 18:43 ` Devendra Tewari
2021-04-21 5:21 ` Khem Raj
2021-04-21 14:43 ` Devendra Tewari
2021-04-21 15:15 ` Khem Raj
2021-04-21 18:10 ` Richard Purdie
2021-04-21 19:15 ` Devendra Tewari
2021-04-21 19:21 ` Richard Purdie
2021-04-21 21:08 ` Devendra Tewari
2021-04-27 11:57 ` Devendra Tewari
2021-04-27 14:59 ` Devendra Tewari
2021-04-27 22:39 ` Richard Purdie
2021-04-27 22:44 ` Devendra Tewari
2021-04-27 22:53 ` Devendra Tewari
2021-04-28 10:00 ` Devendra Tewari
2021-04-28 21:42 ` Richard Purdie
2021-04-28 23:02 ` Devendra Tewari
[not found] ` <90B42906-44DE-4D61-8210-669DC25BBD3F@gmail.com>
2022-08-22 12:25 ` Devendra Tewari
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.