[PATCH v2 1/2] vim: fix CVE-2021-3968 and CVE-2021-3973

[PATCH v2 1/2] vim: fix CVE-2021-3968 and CVE-2021-3973

[Ross Burton]

Backport a fix for -3972, and whitelist -3968: it isn't valid as it
fixes a bug which was introduced after 8.2.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 ...rash-when-using-CTRL-W-f-without-fin.patch | 92 +++++++++++++++++++
 meta/recipes-support/vim/vim.inc              |  4 +
 2 files changed, 96 insertions(+)
 create mode 100644 meta/recipes-support/vim/files/0002-patch-8.2.3611-crash-when-using-CTRL-W-f-without-fin.patch

diff --git a/meta/recipes-support/vim/files/0002-patch-8.2.3611-crash-when-using-CTRL-W-f-without-fin.patch b/meta/recipes-support/vim/files/0002-patch-8.2.3611-crash-when-using-CTRL-W-f-without-fin.patch
new file mode 100644
index 0000000000..58d3442677
--- /dev/null
+++ b/meta/recipes-support/vim/files/0002-patch-8.2.3611-crash-when-using-CTRL-W-f-without-fin.patch
@@ -0,0 +1,92 @@
+CVE: CVE-2021-3973
+Upstream-Status: Backport
+Signed-off-by: Ross Burton <ross.burton@arm.com>
+
+From b6154e9f530544ddc3130d981caae0dabc053757 Mon Sep 17 00:00:00 2001
+From: Bram Moolenaar <Bram@vim.org>
+Date: Wed, 17 Nov 2021 18:00:31 +0000
+Subject: [PATCH] patch 8.2.3611: crash when using CTRL-W f without finding a
+ file name  Problem:    Crash when using CTRL-W f without finding
+ a file name. Solution:   Bail out when the file name length is zero.
+
+---
+ src/findfile.c              | 8 ++++++++
+ src/normal.c                | 6 ++++--
+ src/testdir/test_visual.vim | 8 ++++++++
+ src/version.c               | 2 ++
+ 4 files changed, 22 insertions(+), 2 deletions(-)
+
+diff --git a/src/findfile.c b/src/findfile.c
+index dba547da1..5764fd7b8 100644
+--- a/src/findfile.c
++++ b/src/findfile.c
+@@ -1727,6 +1727,9 @@ find_file_in_path_option(
+     proc->pr_WindowPtr = (APTR)-1L;
+ # endif
+ 
++    if (len == 0)
++	return NULL;
++
+     if (first == TRUE)
+     {
+ 	// copy file name into NameBuff, expanding environment variables
+@@ -2094,7 +2097,12 @@ find_file_name_in_path(
+     int		c;
+ # if defined(FEAT_FIND_ID) && defined(FEAT_EVAL)
+     char_u	*tofree = NULL;
++# endif
+ 
++    if (len == 0)
++	return NULL;
++
++# if defined(FEAT_FIND_ID) && defined(FEAT_EVAL)
+     if ((options & FNAME_INCL) && *curbuf->b_p_inex != NUL)
+     {
+ 	tofree = eval_includeexpr(ptr, len);
+diff --git a/src/normal.c b/src/normal.c
+index 7cb959257..f0084f2ac 100644
+--- a/src/normal.c
++++ b/src/normal.c
+@@ -3778,8 +3778,10 @@ get_visual_text(
+ 	    *pp = ml_get_pos(&VIsual);
+ 	    *lenp = curwin->w_cursor.col - VIsual.col + 1;
+ 	}
+-	if (has_mbyte)
+-	    // Correct the length to include the whole last character.
++	if (**pp == NUL)
++	    *lenp = 0;
++	if (has_mbyte && *lenp > 0)
++	    // Correct the length to include all bytes of the last character.
+ 	    *lenp += (*mb_ptr2len)(*pp + (*lenp - 1)) - 1;
+     }
+     reset_VIsual_and_resel();
+diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
+index ae281238e..0705fdb57 100644
+--- a/src/testdir/test_visual.vim
++++ b/src/testdir/test_visual.vim
+@@ -894,4 +894,12 @@ func Test_block_insert_replace_tabs()
+   bwipe!
+ endfunc
+ 
++func Test_visual_block_ctrl_w_f()
++  " Emtpy block selected in new buffer should not result in an error.
++  au! BufNew foo sil norm \x16\x17f
++  edit foo
++
++  au! BufNew
++endfunc
++
+ " vim: shiftwidth=2 sts=2 expandtab
+diff --git a/src/version.c b/src/version.c
+index 52be3c39d..59a314b3a 100644
+--- a/src/version.c
++++ b/src/version.c
+@@ -742,6 +742,8 @@ static char *(features[]) =
+ 
+ static int included_patches[] =
+ {   /* Add new patch number below this line */
++/**/
++    3611,
+ /**/
+     3582,
+ /**/
diff --git a/meta/recipes-support/vim/vim.inc b/meta/recipes-support/vim/vim.inc
index d0957bfeae..6cdf157cb6 100644
--- a/meta/recipes-support/vim/vim.inc
+++ b/meta/recipes-support/vim/vim.inc
@@ -25,6 +25,7 @@ SRC_URI = "git://github.com/vim/vim.git;branch=master;protocol=https \
            file://0005-patch-8.2.3564-invalid-memory-access-when-scrolling-.patch \
            file://0001-patch-8.2.3581-reading-character-past-end-of-line.patch \
            file://0002-patch-8.2.3582-reading-uninitialized-memory-when-giv.patch \
+           file://0002-patch-8.2.3611-crash-when-using-CTRL-W-f-without-fin.patch \
            "
 
 SRCREV = "98056533b96b6b5d8849641de93185dd7bcadc44"
@@ -32,6 +33,9 @@ SRCREV = "98056533b96b6b5d8849641de93185dd7bcadc44"
 # Do not consider .z in x.y.z, as that is updated with every commit
 UPSTREAM_CHECK_GITTAGREGEX = "(?P<pver>\d+\.\d+)\.0"
 
+# CVE-2021-3968 is related to an issue which was introduced after 8.2, this can be removed after 8.3.
+CVE_CHECK_WHITELIST += "CVE-2021-3968"
+
 S = "${WORKDIR}/git"
 
 VIMDIR = "vim${@d.getVar('PV').split('.')[0]}${@d.getVar('PV').split('.')[1]}"
-- 
2.25.1

[PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Ross Burton]

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 ??= "\
     ${@bb.utils.filter('DISTRO_FEATURES', 'acl selinux', d)} \
     ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11 gtkgui', '', d)} \
     nls \
-- 
2.25.1

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Andre McCurdy]

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]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Ross Burton]

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

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Andre McCurdy]

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...

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Richard Purdie]

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

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Andre McCurdy]

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).

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Richard Purdie]

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

RE: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Peter Kjellerstedt]

> -----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

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Andre McCurdy]

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!

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Andre McCurdy]

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.

Re: [OE-core] [PATCH v2 2/2] vim: set PACKAGECONFIG idiomatically

[Andre McCurdy]

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.