| Message ID | 20220527082923.3652488-1-jose.quaresma@foundries.io |
|---|---|
| State | New |
| Headers | show |
| Series | archiver: exit to avoid race conditions | expand |
On Fri, 2022-05-27 at 09:29 +0100, Jose Quaresma wrote: > The archiver doesn't support using multiconfig sharing the same TMPDIR > so rise an error when we have something that shouldn't exist. > > When in the multiconfig and using the same TMPDIR: the two machines can make > the same copy at the same time to the same destination, which will not work. > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > --- > meta/classes/archiver.bbclass | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass > index c19c770d11..5514e44a78 100644 > --- a/meta/classes/archiver.bbclass > +++ b/meta/classes/archiver.bbclass > @@ -486,6 +486,8 @@ python do_unpack_and_patch() { > if d.getVarFlag('ARCHIVER_MODE', 'diff') == '1': > src = d.getVar('S').rstrip('/') > src_orig = '%s.orig' % src > + if os.path.isdir(src_orig): > + bb.error("A previous copy of the original source already exist '%s'" % src_orig) > oe.path.copytree(src, src_orig) > > if bb.data.inherits_class('dos2unix', d): I suspect we need to make archiver hard error if multiconfig is being used in that case. This can work around some subset of the races but could also still result in broken trees of files :( I'd also note that the patch will still break since bb.error doesn't actually exit, it just prints an error and causes the task to fail. You probably meant bb.fatal(). Cheers, Richard
<richard.purdie@linuxfoundation.org> escreveu no dia sexta, 27/05/2022 à(s) 10:13: > On Fri, 2022-05-27 at 09:29 +0100, Jose Quaresma wrote: > > The archiver doesn't support using multiconfig sharing the same TMPDIR > > so rise an error when we have something that shouldn't exist. > > > > When in the multiconfig and using the same TMPDIR: the two machines can > make > > the same copy at the same time to the same destination, which will not > work. > > > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > > --- > > meta/classes/archiver.bbclass | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/meta/classes/archiver.bbclass > b/meta/classes/archiver.bbclass > > index c19c770d11..5514e44a78 100644 > > --- a/meta/classes/archiver.bbclass > > +++ b/meta/classes/archiver.bbclass > > @@ -486,6 +486,8 @@ python do_unpack_and_patch() { > > if d.getVarFlag('ARCHIVER_MODE', 'diff') == '1': > > src = d.getVar('S').rstrip('/') > > src_orig = '%s.orig' % src > > + if os.path.isdir(src_orig): > > + bb.error("A previous copy of the original source already > exist '%s'" % src_orig) > > oe.path.copytree(src, src_orig) > > > > if bb.data.inherits_class('dos2unix', d): > > I suspect we need to make archiver hard error if multiconfig is being > used in that case. This can work around some subset of the races > but could also still result in broken trees of files :( > > I'd also note that the patch will still break since bb.error doesn't > actually exit, it just prints an error and causes the task to fail. You > probably meant bb.fatal(). Sorry for that, it was bb.fatal() that I wanted to use, I will send a V2 But as you said it can solve some of the races but not all. The right solution can be a check to see if multicast is enabled and using the same TMPDIR but I don't know how to do that. I need to think a little more about this. Jose > Cheers, > > Richard > >
Jose Quaresma <quaresma.jose@gmail.com> escreveu no dia sexta, 27/05/2022 à(s) 11:07: > > > <richard.purdie@linuxfoundation.org> escreveu no dia sexta, 27/05/2022 > à(s) 10:13: > >> On Fri, 2022-05-27 at 09:29 +0100, Jose Quaresma wrote: >> > The archiver doesn't support using multiconfig sharing the same TMPDIR >> > so rise an error when we have something that shouldn't exist. >> > >> > When in the multiconfig and using the same TMPDIR: the two machines can >> make >> > the same copy at the same time to the same destination, which will not >> work. >> > >> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> >> > --- >> > meta/classes/archiver.bbclass | 2 ++ >> > 1 file changed, 2 insertions(+) >> > >> > diff --git a/meta/classes/archiver.bbclass >> b/meta/classes/archiver.bbclass >> > index c19c770d11..5514e44a78 100644 >> > --- a/meta/classes/archiver.bbclass >> > +++ b/meta/classes/archiver.bbclass >> > @@ -486,6 +486,8 @@ python do_unpack_and_patch() { >> > if d.getVarFlag('ARCHIVER_MODE', 'diff') == '1': >> > src = d.getVar('S').rstrip('/') >> > src_orig = '%s.orig' % src >> > + if os.path.isdir(src_orig): >> > + bb.error("A previous copy of the original source already >> exist '%s'" % src_orig) >> > oe.path.copytree(src, src_orig) >> > >> > if bb.data.inherits_class('dos2unix', d): >> >> I suspect we need to make archiver hard error if multiconfig is being >> used in that case. This can work around some subset of the races >> but could also still result in broken trees of files :( >> >> I'd also note that the patch will still break since bb.error doesn't >> actually exit, it just prints an error and causes the task to fail. You >> probably meant bb.fatal(). > > > Sorry for that, it was bb.fatal() that I wanted to use, I will send a V2 > > But as you said it can solve some of the races but not all. > The right solution can be a check to see if multicast is enabled > and using the same TMPDIR but I don't know how to do that. > I need to think a little more about this. > When we are using the bitbake multi-config we have the BBMULTICONFIG defined but afaik only bitbake knows if each machine has their TMPDIR so it is not possible to check this on the archiver.bbclass. bitbake docs [1] says: "Minimally, each configuration file must define the machine and the temporary directory BitBake uses for the build. Suggested practice dictates that you do not overlap the temporary directories used during the builds." Maybe this suggestion to not overlap the temporary directories could be made mandatory. [1] https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-intro.html#executing-a-multiple-configuration-build Jose > Jose > > > > >> Cheers, >> >> Richard >> >> > > -- > Best regards, > > José Quaresma >
diff --git a/meta/classes/archiver.bbclass b/meta/classes/archiver.bbclass index c19c770d11..5514e44a78 100644 --- a/meta/classes/archiver.bbclass +++ b/meta/classes/archiver.bbclass @@ -486,6 +486,8 @@ python do_unpack_and_patch() { if d.getVarFlag('ARCHIVER_MODE', 'diff') == '1': src = d.getVar('S').rstrip('/') src_orig = '%s.orig' % src + if os.path.isdir(src_orig): + bb.error("A previous copy of the original source already exist '%s'" % src_orig) oe.path.copytree(src, src_orig) if bb.data.inherits_class('dos2unix', d):
The archiver doesn't support using multiconfig sharing the same TMPDIR so rise an error when we have something that shouldn't exist. When in the multiconfig and using the same TMPDIR: the two machines can make the same copy at the same time to the same destination, which will not work. Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> --- meta/classes/archiver.bbclass | 2 ++ 1 file changed, 2 insertions(+)