| Message ID | 20240904100748.3623308-1-yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] docs/manual: use space-separated list for BR2_EXTERNAL | expand |
Am 04.09.24 um 12:07 schrieb Yann E. MORIN: > Specifying a list of br2-external trees is poorly documented, and the > only example uses a colon to separate the br2-external paths. > > Adding the support for colon-separated list is the biggest mistake that > was made when introducing support for multiple br2-external [0]. Indeed, > both space and colon can be used to separate entries in the list, and it > is also possible to mix the two. However, internally, the list is stored > as a space-separated list, and all the code will split on spaces. > > So, using colons is odd. > > Change the documentation to only mention using a space-separated list. > > Of course, for backward compatibility, we keep the code as-is to accept > a colon-separated list, but we just do not advertise it. I think it might make sense to add a comment that explains this in support/scripts/br2-external where the colon substitution happens, to avoid confusing people who don't know the history. Best regards, Fiona > [0] in 20cd49738781 core: add support for multiple br2-external trees > > Reported-by: Fiona Klute (WIWA) <fiona.klute@gmx.de> > Reported-by: Brandon Maier <Brandon.Maier@collins.com> > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > > --- > Changes v1 -> v2: > - fix typoes > --- > docs/manual/customize-outside-br.adoc | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/docs/manual/customize-outside-br.adoc b/docs/manual/customize-outside-br.adoc > index 5e653f1603..081d302e42 100644 > --- a/docs/manual/customize-outside-br.adoc > +++ b/docs/manual/customize-outside-br.adoc > @@ -58,10 +58,11 @@ We can switch to another br2-external tree at any time: > buildroot/ $ make BR2_EXTERNAL=/where/we/have/bar xconfig > ---- > > -We can also use multiple br2-external trees: > +We can also use multiple br2-external trees, by specifying a space-separated > +list of paths to use: > > ---- > -buildroot/ $ make BR2_EXTERNAL=/path/to/foo:/where/we/have/bar menuconfig > +buildroot/ $ make BR2_EXTERNAL="/path/to/foo /where/we/have/bar" menuconfig > ---- > > Or disable the usage of any br2-external tree:
Fiona, All, On 2024-09-04 19:59 +0200, Fiona Klute via buildroot spake thusly: > Am 04.09.24 um 12:07 schrieb Yann E. MORIN: [--SNIP--] > > Change the documentation to only mention using a space-separated list. > > Of course, for backward compatibility, we keep the code as-is to accept > > a colon-separated list, but we just do not advertise it. > I think it might make sense to add a comment that explains this in > support/scripts/br2-external where the colon substitution happens, Good idea, I've done so in v3 that I just sent. Thanks for the review! > to > avoid confusing people who don't know the history. "git log" will give you the history! ;-) Regards, Yann E. MORIN.
diff --git a/docs/manual/customize-outside-br.adoc b/docs/manual/customize-outside-br.adoc index 5e653f1603..081d302e42 100644 --- a/docs/manual/customize-outside-br.adoc +++ b/docs/manual/customize-outside-br.adoc @@ -58,10 +58,11 @@ We can switch to another br2-external tree at any time: buildroot/ $ make BR2_EXTERNAL=/where/we/have/bar xconfig ---- -We can also use multiple br2-external trees: +We can also use multiple br2-external trees, by specifying a space-separated +list of paths to use: ---- -buildroot/ $ make BR2_EXTERNAL=/path/to/foo:/where/we/have/bar menuconfig +buildroot/ $ make BR2_EXTERNAL="/path/to/foo /where/we/have/bar" menuconfig ---- Or disable the usage of any br2-external tree:
Specifying a list of br2-external trees is poorly documented, and the only example uses a colon to separate the br2-external paths. Adding the support for colon-separated list is the biggest mistake that was made when introducing support for multiple br2-external [0]. Indeed, both space and colon can be used to separate entries in the list, and it is also possible to mix the two. However, internally, the list is stored as a space-separated list, and all the code will split on spaces. So, using colons is odd. Change the documentation to only mention using a space-separated list. Of course, for backward compatibility, we keep the code as-is to accept a colon-separated list, but we just do not advertise it. [0] in 20cd49738781 core: add support for multiple br2-external trees Reported-by: Fiona Klute (WIWA) <fiona.klute@gmx.de> Reported-by: Brandon Maier <Brandon.Maier@collins.com> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> --- Changes v1 -> v2: - fix typoes --- docs/manual/customize-outside-br.adoc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)