* [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL
@ 2024-09-04 20:53 Yann E. MORIN
2024-09-05 19:35 ` Thomas Petazzoni via buildroot
0 siblings, 1 reply; 5+ messages in thread
From: Yann E. MORIN @ 2024-09-04 20:53 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>
---
Note: support/scripts/br2-external has shellcheck issues; this commit
does not try to address those on purpose, as it only adds a comment.
---
Changes v2 -> v3:
- add comment in support/scripts/br2-external (Fiona)
Changes v1 -> v2:
- fix typoes
---
docs/manual/customize-outside-br.adoc | 5 +++--
support/scripts/br2-external | 3 +++
2 files changed, 6 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:
diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index 8aea479d20..5659761153 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -34,6 +34,9 @@ main() {
trap "error 'unexpected error while generating ${ofile}\n'" ERR
mkdir -p "${outputdir}"
+ # Historically, BR2_EXTERNAL could also be colon-separated, so for
+ # backward compatibility, keep slitting on colons (in addition to
+ # spaces).
do_validate "${outputdir}" ${@//:/ }
do_mk "${outputdir}"
do_kconfig "${outputdir}"
--
2.46.0
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL
2024-09-04 20:53 [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL Yann E. MORIN
@ 2024-09-05 19:35 ` Thomas Petazzoni via buildroot
2024-09-05 19:56 ` Yann E. MORIN
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-09-05 19:35 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Fiona Klute (WIWA), Brandon Maier, buildroot
On Wed, 4 Sep 2024 22:53:54 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 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>
>
> ---
> Note: support/scripts/br2-external has shellcheck issues; this commit
> does not try to address those on purpose, as it only adds a comment.
I'm afraid I'm personally finding that this not going in the right
direction. I didn't follow the discussions on v1 and v2, but on my
side, I would very, very, very much prefer to use colon as a separator
in the BR2_EXTERNAL variable. The PATH variable is like this, it works
well, and I believe the principle of least surprise should encourage us
to follow that and use colon as a separator.
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL
2024-09-05 19:35 ` Thomas Petazzoni via buildroot
@ 2024-09-05 19:56 ` Yann E. MORIN
2024-09-05 20:17 ` Thomas Petazzoni via buildroot
0 siblings, 1 reply; 5+ messages in thread
From: Yann E. MORIN @ 2024-09-05 19:56 UTC (permalink / raw)
To: Thomas Petazzoni; +Cc: Fiona Klute (WIWA), Brandon Maier, buildroot
Thomas, All,
+Arnout whom I forgot to Cc in my patch.
On 2024-09-05 21:35 +0200, Thomas Petazzoni spake thusly:
> On Wed, 4 Sep 2024 22:53:54 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > Change the documentation to only mention using a space-separated list.
[--SNIP--]
> I'm afraid I'm personally finding that this not going in the right
> direction. I didn't follow the discussions on v1 and v2, but on my
> side, I would very, very, very much prefer to use colon as a separator
> in the BR2_EXTERNAL variable. The PATH variable is like this, it works
> well, and I believe the principle of least surprise should encourage us
> to follow that and use colon as a separator.
The thing is that all the other lists are space-separated:
BR2_ROOTFS_DEVICE_TABLE
BR2_ROOTFS_STATIC_DEVICE_TABLE
BR2_TARGET_TZ_ZONELIST
BR2_ROOTFS_USERS_TABLES
BR2_ROOTFS_OVERLAY
BR2_ROOTFS_PRE_BUILD_SCRIPT
BR2_ROOTFS_POST_BUILD_SCRIPT
BR2_ROOTFS_POST_FAKEROOT_SCRIPT
BR2_ROOTFS_POST_IMAGE_SCRIPT
The list goes on and on, notably the options to specify the lists of
.config fragments for the kernel, uclibc, uboot etc...
In fact, BR2_EXTERNAL is the odd one; it's the only one were we expect
a colon-separated list. Also, lists in Makefile are necessarily space-
separated, we can't handle paths with spaces in them anyway. I think we
should just accept that space is the splitting character for lists.
(v1 had too many typoes that I was ashamed enough to quickly respin v2,
which added the comment in the script, as explained in the post-commit
changelogs ;-) )
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] 5+ messages in thread* Re: [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL
2024-09-05 19:56 ` Yann E. MORIN
@ 2024-09-05 20:17 ` Thomas Petazzoni via buildroot
2024-09-06 7:32 ` Arnout Vandecappelle via buildroot
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-09-05 20:17 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Fiona Klute (WIWA), Brandon Maier, buildroot
On Thu, 5 Sep 2024 21:56:26 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> The thing is that all the other lists are space-separated:
> BR2_ROOTFS_DEVICE_TABLE
> BR2_ROOTFS_STATIC_DEVICE_TABLE
> BR2_TARGET_TZ_ZONELIST
> BR2_ROOTFS_USERS_TABLES
> BR2_ROOTFS_OVERLAY
> BR2_ROOTFS_PRE_BUILD_SCRIPT
> BR2_ROOTFS_POST_BUILD_SCRIPT
> BR2_ROOTFS_POST_FAKEROOT_SCRIPT
> BR2_ROOTFS_POST_IMAGE_SCRIPT
>
> The list goes on and on, notably the options to specify the lists of
> .config fragments for the kernel, uclibc, uboot etc...
I agree, but these are Buildroot menuconfig options, not something
passed on the command line.
That being said, it's a matter of taste, so I will not oppose to the
change.
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL
2024-09-05 20:17 ` Thomas Petazzoni via buildroot
@ 2024-09-06 7:32 ` Arnout Vandecappelle via buildroot
0 siblings, 0 replies; 5+ messages in thread
From: Arnout Vandecappelle via buildroot @ 2024-09-06 7:32 UTC (permalink / raw)
To: Thomas Petazzoni, Yann E. MORIN
Cc: Fiona Klute (WIWA), Brandon Maier, buildroot
On 05/09/2024 22:17, Thomas Petazzoni wrote:
> On Thu, 5 Sep 2024 21:56:26 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>
>> The thing is that all the other lists are space-separated:
>> BR2_ROOTFS_DEVICE_TABLE
>> BR2_ROOTFS_STATIC_DEVICE_TABLE
>> BR2_TARGET_TZ_ZONELIST
>> BR2_ROOTFS_USERS_TABLES
>> BR2_ROOTFS_OVERLAY
>> BR2_ROOTFS_PRE_BUILD_SCRIPT
>> BR2_ROOTFS_POST_BUILD_SCRIPT
>> BR2_ROOTFS_POST_FAKEROOT_SCRIPT
>> BR2_ROOTFS_POST_IMAGE_SCRIPT
>>
>> The list goes on and on, notably the options to specify the lists of
>> .config fragments for the kernel, uclibc, uboot etc...
>
> I agree, but these are Buildroot menuconfig options, not something
> passed on the command line.
Indeed, since you pass it on the command line, you have to quote it to be able
to use spaces. While in menuconfig, the quoting is added by menuconfig itself.
> That being said, it's a matter of taste, so I will not oppose to the
> change.
I don't think the comparison with $PATH is very relevant in this case - unlike
PYTHONPATH or LD_LIBRARY_PATH there is no PATH anywhere in the variable name. So
I don't think people will automatically make the association with the colon
separator used in PATH. Thus, I wouldn't say that use colon is the path of least
surprise. Quite the reverse in fact, because, as Yann said, *everything else* in
Buildroot is space-separated.
So I think the proposed situation is fine: documentation says to use spaces
(like everywhere else in Buildroot), but colons work as well.
Note that this means that colon is not allowed in BR2_EXTERNAL directory names
- but that is not so surprising, colons in file/directory names are a problem
for Make anyway.
Regards,
Arnout
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-06 7:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-04 20:53 [Buildroot] [PATCH v3] docs/manual: use space-separated list for BR2_EXTERNAL Yann E. MORIN
2024-09-05 19:35 ` Thomas Petazzoni via buildroot
2024-09-05 19:56 ` Yann E. MORIN
2024-09-05 20:17 ` Thomas Petazzoni via buildroot
2024-09-06 7:32 ` Arnout Vandecappelle via buildroot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox