Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] docs/manual: use space-separated list for BR2_EXTERNAL
@ 2024-09-04 10:07 Yann E. MORIN
  2024-09-04 17:59 ` Fiona Klute via buildroot
  0 siblings, 1 reply; 3+ messages in thread
From: Yann E. MORIN @ 2024-09-04 10:07 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN, Brandon Maier, Fiona Klute (WIWA)

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

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

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Buildroot] [PATCH v2] docs/manual: use space-separated list for BR2_EXTERNAL
  2024-09-04 10:07 [Buildroot] [PATCH v2] docs/manual: use space-separated list for BR2_EXTERNAL Yann E. MORIN
@ 2024-09-04 17:59 ` Fiona Klute via buildroot
  2024-09-04 20:58   ` Yann E. MORIN
  0 siblings, 1 reply; 3+ messages in thread
From: Fiona Klute via buildroot @ 2024-09-04 17:59 UTC (permalink / raw)
  To: Yann E. MORIN, buildroot; +Cc: Brandon Maier

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:

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Buildroot] [PATCH v2] docs/manual: use space-separated list for BR2_EXTERNAL
  2024-09-04 17:59 ` Fiona Klute via buildroot
@ 2024-09-04 20:58   ` Yann E. MORIN
  0 siblings, 0 replies; 3+ messages in thread
From: Yann E. MORIN @ 2024-09-04 20:58 UTC (permalink / raw)
  To: Fiona Klute; +Cc: Brandon Maier, buildroot

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-09-04 20:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-04 10:07 [Buildroot] [PATCH v2] docs/manual: use space-separated list for BR2_EXTERNAL Yann E. MORIN
2024-09-04 17:59 ` Fiona Klute via buildroot
2024-09-04 20:58   ` Yann E. MORIN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox