| Message ID | 20211130165313.3816904-2-ross.burton@arm.com |
|---|---|
| State | Accepted, archived |
| Commit | d7565241437487618a57d8f3f21da6fed69f6b8a |
| Headers | show |
| Series | [v2,1/2] vim: fix CVE-2021-3968 and CVE-2021-3973 | expand |
On Tue, Nov 30, 2021 at 8:53 AM Ross Burton <ross@burtonini.com> wrote: > > Don't set an empty default value and them immediately assign to it. > > Signed-off-by: Ross Burton <ross.burton@arm.com> > --- > meta/recipes-support/vim/vim.inc | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/meta/recipes-support/vim/vim.inc b/meta/recipes-support/vim/vim.inc > index 6cdf157cb6..a0692755b6 100644 > --- a/meta/recipes-support/vim/vim.inc > +++ b/meta/recipes-support/vim/vim.inc > @@ -67,9 +67,7 @@ do_compile() { > autotools_do_compile > } > > -#Available PACKAGECONFIG options are gtkgui, acl, x11, tiny selinux, elfutils, nls > -PACKAGECONFIG ??= "" > -PACKAGECONFIG += " \ > +PACKAGECONFIG ??= "\ This isn't equivalent - it will cause a change in behaviour for anyone using PACKAGECONFIG += "foo" from a .bbappend. > ${@bb.utils.filter('DISTRO_FEATURES', 'acl selinux', d)} \ > ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11 gtkgui', '', d)} \ > nls \ > -- > 2.25.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#158982): https://lists.openembedded.org/g/openembedded-core/message/158982 > Mute This Topic: https://lists.openembedded.org/mt/87406894/3619030 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [armccurdy@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > This isn't equivalent - it will cause a change in behaviour for anyone > using PACKAGECONFIG += "foo" from a .bbappend. Correct, but this is likely the only recipe in the greater ecosystem which has this behaviour, so I'm not that bothered to be honest. :) Ross
On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> wrote: > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > > This isn't equivalent - it will cause a change in behaviour for anyone > > using PACKAGECONFIG += "foo" from a .bbappend. > > Correct, but this is likely the only recipe in the greater ecosystem > which has this behaviour, so I'm not that bothered to be honest. :) The only recipe? There are many recipes which set a default PACKAGECONFIG with ?= and many which set it with ??=. Your change is effectively switching the vim recipe from one approach to the other. The fact that adding PACKAGECONFIG options from a .bbappend with += sometimes works OK and sometimes not is a source of confusion for new users. You are right that no one seems to care though...
On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> wrote: > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > > > This isn't equivalent - it will cause a change in behaviour for anyone > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > Correct, but this is likely the only recipe in the greater ecosystem > > which has this behaviour, so I'm not that bothered to be honest. :) > > The only recipe? There are many recipes which set a default > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > effectively switching the vim recipe from one approach to the other. > The fact that adding PACKAGECONFIG options from a .bbappend with += > sometimes works OK and sometimes not is a source of confusion for new > users. > > You are right that no one seems to care though... Some of us very much do care, it is actually bothering me a lot and I've posted several times on the architecture list about this kind of issue. We haven't worked out what we can agree to do about it though :(. Cheers, Richard
On Wed, Dec 1, 2021 at 2:33 PM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> wrote: > > > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > > > > This isn't equivalent - it will cause a change in behaviour for anyone > > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > > > Correct, but this is likely the only recipe in the greater ecosystem > > > which has this behaviour, so I'm not that bothered to be honest. :) > > > > The only recipe? There are many recipes which set a default > > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > > effectively switching the vim recipe from one approach to the other. > > The fact that adding PACKAGECONFIG options from a .bbappend with += > > sometimes works OK and sometimes not is a source of confusion for new > > users. > > > > You are right that no one seems to care though... > > Some of us very much do care, it is actually bothering me a lot and I've posted > several times on the architecture list about this kind of issue. > > We haven't worked out what we can agree to do about it though :(. As a first, very easy, step, make a statement here on the mailing list that all PACKAGECONFIG defaults should be assigned with ?= instead of ??= and fix the recipes in oe-core accordingly. As a second step, the parser could generate a warning (or even an error) if any variable is assigned to with only ??= and += (the end result of that combination is not what any user would expect and I doubt if any legitimate use case relies on it).
On Fri, 2021-12-03 at 01:39 -0800, Andre McCurdy wrote: > On Wed, Dec 1, 2021 at 2:33 PM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > > > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> wrote: > > > > > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > > > > > This isn't equivalent - it will cause a change in behaviour for anyone > > > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > > > > > Correct, but this is likely the only recipe in the greater ecosystem > > > > which has this behaviour, so I'm not that bothered to be honest. :) > > > > > > The only recipe? There are many recipes which set a default > > > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > > > effectively switching the vim recipe from one approach to the other. > > > The fact that adding PACKAGECONFIG options from a .bbappend with += > > > sometimes works OK and sometimes not is a source of confusion for new > > > users. > > > > > > You are right that no one seems to care though... > > > > Some of us very much do care, it is actually bothering me a lot and I've posted > > several times on the architecture list about this kind of issue. > > > > We haven't worked out what we can agree to do about it though :(. > > As a first, very easy, step, make a statement here on the mailing list > that all PACKAGECONFIG defaults should be assigned with ?= instead of > ??= and fix the recipes in oe-core accordingly. The question is whether we all agree on that and I'm not sure we all do. > As a second step, the parser could generate a warning (or even an > error) if any variable is assigned to with only ??= and += (the end > result of that combination is not what any user would expect and I > doubt if any legitimate use case relies on it). Would be interesting to see if there is valid use so it is probably worth some tests/analysis. The ??= operator never did what I'd hoped it would in reality sadly. Cheers, Richard
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded- > core@lists.openembedded.org> On Behalf Of Richard Purdie > Sent: den 3 december 2021 11:07 > To: Andre McCurdy <armccurdy@gmail.com> > Cc: Ross Burton <ross@burtonini.com>; OE Core mailing list <openembedded- > core@lists.openembedded.org> > Subject: Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically > > On Fri, 2021-12-03 at 01:39 -0800, Andre McCurdy wrote: > > On Wed, Dec 1, 2021 at 2:33 PM Richard Purdie > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > > > > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> > wrote: > > > > > > > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> > wrote: > > > > > > This isn't equivalent - it will cause a change in behaviour for > anyone > > > > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > > > > > > > Correct, but this is likely the only recipe in the greater > ecosystem > > > > > which has this behaviour, so I'm not that bothered to be honest. > :) > > > > > > > > The only recipe? There are many recipes which set a default > > > > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > > > > effectively switching the vim recipe from one approach to the other. > > > > The fact that adding PACKAGECONFIG options from a .bbappend with += > > > > sometimes works OK and sometimes not is a source of confusion for > new > > > > users. > > > > > > > > You are right that no one seems to care though... > > > > > > Some of us very much do care, it is actually bothering me a lot and > I've posted > > > several times on the architecture list about this kind of issue. > > > > > > We haven't worked out what we can agree to do about it though :(. > > > > As a first, very easy, step, make a statement here on the mailing list > > that all PACKAGECONFIG defaults should be assigned with ?= instead of > > ??= and fix the recipes in oe-core accordingly. > > The question is whether we all agree on that and I'm not sure we all do. I definitely agree that using "??=" in the recipe for PACKAGECONFIG is a bad idea. In all our own recipes we use "=" so that is what I would prefer, but "?=" is ok and it would alleviate the need to use PACKAGECONFIG:append in bbappends instead of "PACKAGECONFIG +=". The reason I think "=" is better than "?=" is that if you want to override the PACKAGECONFIG in a bbappend, using "=" a second time will work fine, and if you want to do the override in a configuration file like local.conf, you would use PACKAGECONFIG:pn-foo, which also would override whatever the recipe set using "=". So unless I am missing a use case, there really isn't a need to use "?=". > > As a second step, the parser could generate a warning (or even an > > error) if any variable is assigned to with only ??= and += (the end > > result of that combination is not what any user would expect and I > > doubt if any legitimate use case relies on it). > > Would be interesting to see if there is valid use so it is probably worth > some tests/analysis. The ??= operator never did what I'd hoped it would in > reality sadly. > > Cheers, > > Richard //Peter
On Fri, Dec 3, 2021 at 2:06 AM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Fri, 2021-12-03 at 01:39 -0800, Andre McCurdy wrote: > > On Wed, Dec 1, 2021 at 2:33 PM Richard Purdie > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > > > > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> wrote: > > > > > > > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > > > > > > This isn't equivalent - it will cause a change in behaviour for anyone > > > > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > > > > > > > Correct, but this is likely the only recipe in the greater ecosystem > > > > > which has this behaviour, so I'm not that bothered to be honest. :) > > > > > > > > The only recipe? There are many recipes which set a default > > > > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > > > > effectively switching the vim recipe from one approach to the other. > > > > The fact that adding PACKAGECONFIG options from a .bbappend with += > > > > sometimes works OK and sometimes not is a source of confusion for new > > > > users. > > > > > > > > You are right that no one seems to care though... > > > > > > Some of us very much do care, it is actually bothering me a lot and I've posted > > > several times on the architecture list about this kind of issue. > > > > > > We haven't worked out what we can agree to do about it though :(. > > > > As a first, very easy, step, make a statement here on the mailing list > > that all PACKAGECONFIG defaults should be assigned with ?= instead of > > ??= and fix the recipes in oe-core accordingly. > > The question is whether we all agree on that and I'm not sure we all do. What are the possible objections? > > As a second step, the parser could generate a warning (or even an > > error) if any variable is assigned to with only ??= and += (the end > > result of that combination is not what any user would expect and I > > doubt if any legitimate use case relies on it). > > Would be interesting to see if there is valid use so it is probably worth some > tests/analysis. The ??= operator never did what I'd hoped it would in reality > sadly. I agree ??= is way overused and very often in places where ?= or a direct assignment would be better. I'm not the one accepting and merging patches though... you are!
On Fri, Dec 3, 2021 at 5:59 AM Peter Kjellerstedt <peter.kjellerstedt@axis.com> wrote: > > > -----Original Message----- > > From: openembedded-core@lists.openembedded.org <openembedded- > > core@lists.openembedded.org> On Behalf Of Richard Purdie > > Sent: den 3 december 2021 11:07 > > To: Andre McCurdy <armccurdy@gmail.com> > > Cc: Ross Burton <ross@burtonini.com>; OE Core mailing list <openembedded- > > core@lists.openembedded.org> > > Subject: Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically > > > > On Fri, 2021-12-03 at 01:39 -0800, Andre McCurdy wrote: > > > On Wed, Dec 1, 2021 at 2:33 PM Richard Purdie > > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > > > On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > > > > > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> > > wrote: > > > > > > > > > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> > > wrote: > > > > > > > This isn't equivalent - it will cause a change in behaviour for > > anyone > > > > > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > > > > > > > > > Correct, but this is likely the only recipe in the greater > > ecosystem > > > > > > which has this behaviour, so I'm not that bothered to be honest. > > :) > > > > > > > > > > The only recipe? There are many recipes which set a default > > > > > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > > > > > effectively switching the vim recipe from one approach to the other. > > > > > The fact that adding PACKAGECONFIG options from a .bbappend with += > > > > > sometimes works OK and sometimes not is a source of confusion for > > new > > > > > users. > > > > > > > > > > You are right that no one seems to care though... > > > > > > > > Some of us very much do care, it is actually bothering me a lot and > > I've posted > > > > several times on the architecture list about this kind of issue. > > > > > > > > We haven't worked out what we can agree to do about it though :(. > > > > > > As a first, very easy, step, make a statement here on the mailing list > > > that all PACKAGECONFIG defaults should be assigned with ?= instead of > > > ??= and fix the recipes in oe-core accordingly. > > > > The question is whether we all agree on that and I'm not sure we all do. > > I definitely agree that using "??=" in the recipe for PACKAGECONFIG is > a bad idea. In all our own recipes we use "=" so that is what I would > prefer, but "?=" is ok and it would alleviate the need to use > PACKAGECONFIG:append in bbappends instead of "PACKAGECONFIG +=". > > The reason I think "=" is better than "?=" is that if you want to > override the PACKAGECONFIG in a bbappend, using "=" a second time will > work fine, and if you want to do the override in a configuration file > like local.conf, you would use PACKAGECONFIG:pn-foo, which also would > override whatever the recipe set using "=". So unless I am missing a > use case, there really isn't a need to use "?=". Using = would certainly be OK and an improvement over the current mess. The reason I'd still argue that ?= is better is that it gives a clear hint that PACKAGECONFIG values in recipes are something a user may want to review and consider changing.
On Fri, Dec 3, 2021 at 10:07 AM Andre McCurdy <armccurdy@gmail.com> wrote: > > On Fri, Dec 3, 2021 at 2:06 AM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Fri, 2021-12-03 at 01:39 -0800, Andre McCurdy wrote: > > > On Wed, Dec 1, 2021 at 2:33 PM Richard Purdie > > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > > > On Tue, 2021-11-30 at 13:45 -0800, Andre McCurdy wrote: > > > > > On Tue, Nov 30, 2021 at 1:20 PM Ross Burton <ross@burtonini.com> wrote: > > > > > > > > > > > > On Tue, 30 Nov 2021 at 19:32, Andre McCurdy <armccurdy@gmail.com> wrote: > > > > > > > This isn't equivalent - it will cause a change in behaviour for anyone > > > > > > > using PACKAGECONFIG += "foo" from a .bbappend. > > > > > > > > > > > > Correct, but this is likely the only recipe in the greater ecosystem > > > > > > which has this behaviour, so I'm not that bothered to be honest. :) > > > > > > > > > > The only recipe? There are many recipes which set a default > > > > > PACKAGECONFIG with ?= and many which set it with ??=. Your change is > > > > > effectively switching the vim recipe from one approach to the other. > > > > > The fact that adding PACKAGECONFIG options from a .bbappend with += > > > > > sometimes works OK and sometimes not is a source of confusion for new > > > > > users. > > > > > > > > > > You are right that no one seems to care though... > > > > > > > > Some of us very much do care, it is actually bothering me a lot and I've posted > > > > several times on the architecture list about this kind of issue. > > > > > > > > We haven't worked out what we can agree to do about it though :(. > > > > > > As a first, very easy, step, make a statement here on the mailing list > > > that all PACKAGECONFIG defaults should be assigned with ?= instead of > > > ??= and fix the recipes in oe-core accordingly. > > > > The question is whether we all agree on that and I'm not sure we all do. > > What are the possible objections? Any more feedback? The misconception that no one cares about improving this could be because discussions on the topic always seem to peter out leaving these unanswered questions.
diff --git a/meta/recipes-support/vim/vim.inc b/meta/recipes-support/vim/vim.inc index 6cdf157cb6..a0692755b6 100644 --- a/meta/recipes-support/vim/vim.inc +++ b/meta/recipes-support/vim/vim.inc @@ -67,9 +67,7 @@ do_compile() { autotools_do_compile } -#Available PACKAGECONFIG options are gtkgui, acl, x11, tiny selinux, elfutils, nls -PACKAGECONFIG ??= "" -PACKAGECONFIG += " \ +PACKAGECONFIG ??= "\ ${@bb.utils.filter('DISTRO_FEATURES', 'acl selinux', d)} \ ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11 gtkgui', '', d)} \ nls \
Don't set an empty default value and them immediately assign to it. Signed-off-by: Ross Burton <ross.burton@arm.com> --- meta/recipes-support/vim/vim.inc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)