| Message ID | 20220926090604.23397-1-andriy.danylovskyy@streamunlimited.com |
|---|---|
| State | New |
| Headers | show |
| Series | classes/patch: move QUILT_PC for patching consistency | expand |
I am aware of the "force-unpack + patch" scenario, discussed in https://lists.yoctoproject.org/g/yocto/topic/89948013#56602 , and this patch, sadly, doesn't fix that. But it does fix another face of it, which IMO is even worse than a failing build - a false positive, where updates to a source are ignored, and the package is "successfully" built from the previous version.
On Mon, 2022-09-26 at 11:06 +0200, Andriy Danylovskyy wrote: > This will move the quilt cache from the default location '$S/.pc' to > '$S/patches/.pc', to ensure source invalidation always wipes it out, > together with all patches. > > Recipes which set $S to $WORKDIR are susceptible to a weird issue: There are a number of problems with recipes which use S = WORKDIR unfortunately. Another is that rerunning fetch/unpack doesn't clean up files properly and can lead to build corruption. I'm leaning towards making S == WORKDIR a warning and migrating recipes to always use a subdir. That isn't entirely straight forward but probably the only way to solve all the issues. > if > a source file is patched by quilt (a .bbappend adds a patch), updates > to it are ignored by incremental builds, the first obsolete version is > picked again and again. This is because quilt keeps its own cache in > '$S/.pc', and this one survives source invalidation on do_unpack. > > This is a follow-up for a56fb90dc380 and 42a513489dc6 > > Signed-off-by: Andriy Danylovskyy <andriy.danylovskyy@streamunlimited.com> > --- > meta/classes-global/patch.bbclass | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/meta/classes-global/patch.bbclass b/meta/classes-global/patch.bbclass > index e3157c7b18..6fcac18d9c 100644 > --- a/meta/classes-global/patch.bbclass > +++ b/meta/classes-global/patch.bbclass > @@ -5,6 +5,9 @@ > # Point to an empty file so any user's custom settings don't break things > QUILTRCFILE ?= "${STAGING_ETCDIR_NATIVE}/quiltrc" > > +# Move quilt's cache to ensure it always gets removed together with "patches" > +export QUILT_PC = "${S}/patches/.pc" > + This would break all other commandline use of quilt without the right environment. Sadly that is a usecase I personally use quite heavily too :/. Cheers, Richard
> I am aware of the "force-unpack + patch" scenario, discussed in https://lists.yoctoproject.org/g/yocto/topic/89948013#56602, and this patch, sadly, doesn't fix that. Wasn't this scenario fixed by: c9d36882044b1c633d8611a77df54cd68c9bee25 ? It was for me. On Mon, Sep 26, 2022 at 11:13 AM Andriy Danylovskyy < andriy.danylovskyy@streamunlimited.com> wrote: > I am aware of the "force-unpack + patch" scenario, discussed in > https://lists.yoctoproject.org/g/yocto/topic/89948013#56602, > and this patch, sadly, doesn't fix that. > But it does fix another face of it, which IMO is even worse than a failing > build - a false positive, where updates to a source are ignored, > and the package is "successfully" built from the previous version. > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#171055): > https://lists.openembedded.org/g/openembedded-core/message/171055 > Mute This Topic: https://lists.openembedded.org/mt/93922956/3617156 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > Martin.Jansa@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On 26/09/2022 12:23, Richard Purdie wrote: > > On Mon, 2022-09-26 at 11:06 +0200, Andriy Danylovskyy wrote: > >> This will move the quilt cache from the default location '$S/.pc' to >> '$S/patches/.pc', to ensure source invalidation always wipes it out, >> together with all patches. >> >> Recipes which set $S to $WORKDIR are >> susceptible to a weird issue: >> > > There are a number of problems with recipes which use S = WORKDIR > unfortunately. Another is that rerunning fetch/unpack doesn't clean up > files properly and can lead to build corruption. > > I'm leaning towards > making S == WORKDIR a warning and migrating recipes > to always use a > subdir. That isn't entirely straight forward but > probably the only way to > solve all the issues. > Anything that doesn't let it "pass" silently would be already a big improvement. Although deprecation with a warning sounds like a few more years to go into all affected projects. > > >> if >> a source file is patched by quilt (a .bbappend adds a patch), updates >> to it are ignored by incremental builds, the first obsolete version is >> picked again and again. This is because quilt keeps its own cache in >> '$S/.pc', and this one survives source invalidation on do_unpack. >> >> This is >> a follow-up for a56fb90dc380 and 42a513489dc6 >> >> Signed-off-by: Andriy >> Danylovskyy <andriy.danylovskyy@streamunlimited.com> >> --- >> >> meta/classes-global/patch.bbclass | 3 +++ >> 1 file changed, 3 insertions(+) >> diff --git a/meta/classes-global/patch.bbclass >> b/meta/classes-global/patch.bbclass >> index e3157c7b18..6fcac18d9c 100644 >> --- a/meta/classes-global/patch.bbclass >> +++ >> b/meta/classes-global/patch.bbclass >> @@ -5,6 +5,9 @@ >> # Point to an empty >> file so any user's custom settings don't break things >> QUILTRCFILE ?= >> "${STAGING_ETCDIR_NATIVE}/quiltrc" >> >> +# Move quilt's cache to ensure it >> always gets removed together with "patches" >> +export QUILT_PC = >> "${S}/patches/.pc" >> + >> > > This would break all other commandline use of quilt without the right > environment. Sadly that is a usecase I personally use quite heavily too > :/. > > Cheers, > > Richard > I can't think of any patching in a recipe workdir, outside of do_patch and the devtool, so this wasn't taken into account. But what do I know about other people's workflows... Then another (dirtier?) option would be to patch quilt-native itself, setting QUILT_PC to the relative ./patches/.pc -- Best regards, Andriy
On Mon, 2022-09-26 at 04:46 -0700, Andriy Danylovskyy wrote: > On 26/09/2022 12:23, Richard Purdie wrote: > > On Mon, 2022-09-26 at 11:06 +0200, Andriy Danylovskyy wrote: > > > This will move the quilt cache from the default location '$S/.pc' > > > to > > > '$S/patches/.pc', to ensure source invalidation always wipes it > > > out, > > > together with all patches. > > > > > > Recipes which set $S to $WORKDIR are susceptible to a weird > > > issue: > > There are a number of problems with recipes which use S = WORKDIR > > unfortunately. Another is that rerunning fetch/unpack doesn't clean > > up > > files properly and can lead to build corruption. > > > > I'm leaning towards making S == WORKDIR a warning and migrating > > recipes > > to always use a subdir. That isn't entirely straight forward but > > probably the only way to solve all the issues. > Anything that doesn't let it "pass" silently would be already a big > improvement. Although deprecation with a warning sounds like a few > more years to go into all affected projects. It wouldn't be a quick fix for the stable releases certainly but it would hopefully fairly quickly filter down through from master if we did start showing warnings. I have the warnings patch running locally and have had for a while. Converting recipes isn't as easy as you'd think though so I'm try trying to work out a better way to handle that. We may also have to go a step further and fetch into a subdir in order to address all the issues so I haven't really found a good answer to all the problems that I'm happy to promote and suggest we change to. > > > > > > > if > > > a source file is patched by quilt (a .bbappend adds a patch), > > > updates > > > to it are ignored by incremental builds, the first obsolete > > > version is > > > picked again and again. This is because quilt keeps its own cache > > > in > > > '$S/.pc', and this one survives source invalidation on do_unpack. > > > > > > This is a follow-up for a56fb90dc380 and 42a513489dc6 > > > > > > Signed-off-by: Andriy Danylovskyy > > > <andriy.danylovskyy@streamunlimited.com> > > > --- > > > meta/classes-global/patch.bbclass | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/meta/classes-global/patch.bbclass b/meta/classes- > > > global/patch.bbclass > > > index e3157c7b18..6fcac18d9c 100644 > > > --- a/meta/classes-global/patch.bbclass > > > +++ b/meta/classes-global/patch.bbclass > > > @@ -5,6 +5,9 @@ > > > # Point to an empty file so any user's custom settings don't > > > break things > > > QUILTRCFILE ?= "${STAGING_ETCDIR_NATIVE}/quiltrc" > > > > > > +# Move quilt's cache to ensure it always gets removed together > > > with "patches" > > > +export QUILT_PC = "${S}/patches/.pc" > > > + > > This would break all other commandline use of quilt without the > > right > > environment. Sadly that is a usecase I personally use quite heavily > > too > > :/. > > > > Cheers, > > > > Richard > I can't think of any patching in a recipe workdir, outside of > do_patch and the devtool, so this wasn't taken into account. But what > do I know about other people's workflows... I'm old school and have used quilt for patching kernels for a couple of decades and have used it before devtool (or even git) existed! There are probably better ways now but quilt is muscle memory. > Then another (dirtier?) option would be to patch quilt-native > itself, setting QUILT_PC to the relative ./patches/.pc Sadly it doesn't help my use case as I'm used to changing to some random WORKDIR and using the host's copy of quilt to save messing with PATH :/. Cheers, Richard
Thanks Martin. Yes, it does, just tried it on the latest master. When I cross-checked this before, I must have done it on an older project, with quilt 0.66 (and applying c9d36882044 there doesn't help, either). On 26/09/2022 13:21, Martin Jansa wrote: > > > I am aware of the "force-unpack + patch" scenario, discussed in https://lists.yoctoproject.org/g/yocto/topic/89948013#56602 > , and this patch, sadly, doesn't fix that. > > Wasn't this scenario fixed by: c9d36882044b1c633d8611a77df54cd68c9bee25 ? > > It was for me. >
On 26/09/2022 14:21, Richard Purdie wrote: > > I'm old school and have used quilt for patching kernels for a couple of > decades and have used it before devtool (or even git) existed! There > are > probably better ways now but quilt is muscle memory. > > >> Then another (dirtier?) option would be to patch quilt-native >> itself, >> setting QUILT_PC to the relative ./patches/.pc >> > > Sadly it doesn't help my use case as I'm used to changing to some > random > WORKDIR and using the host's copy of quilt to save messing with > PATH :/. > > Cheers, > > Richard > I see, you never imagine all use cases until you ask. But the issue needs a solution, one way or another. What's especially bad is this can go unnoticed for quite a while. -- Best regards, Andriy
diff --git a/meta/classes-global/patch.bbclass b/meta/classes-global/patch.bbclass index e3157c7b18..6fcac18d9c 100644 --- a/meta/classes-global/patch.bbclass +++ b/meta/classes-global/patch.bbclass @@ -5,6 +5,9 @@ # Point to an empty file so any user's custom settings don't break things QUILTRCFILE ?= "${STAGING_ETCDIR_NATIVE}/quiltrc" +# Move quilt's cache to ensure it always gets removed together with "patches" +export QUILT_PC = "${S}/patches/.pc" + PATCHDEPENDENCY = "${PATCHTOOL}-native:do_populate_sysroot" # There is a bug in patch 2.7.3 and earlier where index lines
This will move the quilt cache from the default location '$S/.pc' to '$S/patches/.pc', to ensure source invalidation always wipes it out, together with all patches. Recipes which set $S to $WORKDIR are susceptible to a weird issue: if a source file is patched by quilt (a .bbappend adds a patch), updates to it are ignored by incremental builds, the first obsolete version is picked again and again. This is because quilt keeps its own cache in '$S/.pc', and this one survives source invalidation on do_unpack. This is a follow-up for a56fb90dc380 and 42a513489dc6 Signed-off-by: Andriy Danylovskyy <andriy.danylovskyy@streamunlimited.com> --- meta/classes-global/patch.bbclass | 3 +++ 1 file changed, 3 insertions(+)