[Buildroot] [PATCH next v4 1/1] utils/docker-run: mount and pass BR2_EXTERNAL dirs

[Buildroot] [PATCH next v4 1/1] utils/docker-run: mount and pass BR2_EXTERNAL dirs

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

The BR2_EXTERNAL environment variable is passed into the container,
and each path listed in it mounted. This allows using external trees
when running a build using utils/docker-run. Testing the existence of
the variable instead of a non-empty value allows passing an empty
BR2_EXTERNAL variable to disable currently set external trees.

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
Brandon, I've dropped your reviewed-by because the changes from v3 are
significant and counter to what you suggested in previous
reviews. Feel free to re-add it (or not).

Changes v3 -> v4:
* Split BR2_EXTERNAL on spaces OR colons
* Revert changes to mountpoints processing loop

Changes v2 -> v3:
* Use read to make the loop that creates mount options more robust,
  removing pre-exisiting shellcheck override. Suggested by Brandon
  Maier.

Changes v1 -> v2:
* Correctly handle spaces in external tree paths

 utils/docker-run | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/utils/docker-run b/utils/docker-run
index 1adb02d74e..9579f83ce2 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -90,6 +90,13 @@ if [ "${BR2_DL_DIR}" ]; then
     docker_opts+=( --env BR2_DL_DIR )
 fi

+if [ -v BR2_EXTERNAL ]; then
+    docker_opts+=( --env BR2_EXTERNAL )
+    for br2_ext in ${BR2_EXTERNAL//:/ }; do
+        mountpoints+=( "${br2_ext}" )
+    done
+fi
+
 # shellcheck disable=SC2013 # can't use while-read because of the assignment
 for dir in $(printf '%s\n' "${mountpoints[@]}" |LC_ALL=C sort -u); do
     docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
--
2.45.2

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

Re: [Buildroot] [PATCH next v4 1/1] utils/docker-run: mount and pass BR2_EXTERNAL dirs

[Fiona Klute via buildroot]

Hi everyone,

with the documentation clarification in 
f9cdca48a57f24f2babb8f72652e099e08a59cd5 merged (BR2_EXTERNAL is space 
separated, with colons supported for backwards compatibility), can we 
get this merged? As far as I remember proper handling of spaces was the 
main point of contention in review, and with the decision for space 
separated list the patch should be fine as-is.

Best regards,
Fiona

Am 31.08.24 um 21:19 schrieb Fiona Klute via buildroot:
> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> The BR2_EXTERNAL environment variable is passed into the container,
> and each path listed in it mounted. This allows using external trees
> when running a build using utils/docker-run. Testing the existence of
> the variable instead of a non-empty value allows passing an empty
> BR2_EXTERNAL variable to disable currently set external trees.
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
> Brandon, I've dropped your reviewed-by because the changes from v3 are
> significant and counter to what you suggested in previous
> reviews. Feel free to re-add it (or not).
> 
> Changes v3 -> v4:
> * Split BR2_EXTERNAL on spaces OR colons
> * Revert changes to mountpoints processing loop
> 
> Changes v2 -> v3:
> * Use read to make the loop that creates mount options more robust,
>    removing pre-exisiting shellcheck override. Suggested by Brandon
>    Maier.
> 
> Changes v1 -> v2:
> * Correctly handle spaces in external tree paths
> 
>   utils/docker-run | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/utils/docker-run b/utils/docker-run
> index 1adb02d74e..9579f83ce2 100755
> --- a/utils/docker-run
> +++ b/utils/docker-run
> @@ -90,6 +90,13 @@ if [ "${BR2_DL_DIR}" ]; then
>       docker_opts+=( --env BR2_DL_DIR )
>   fi
> 
> +if [ -v BR2_EXTERNAL ]; then
> +    docker_opts+=( --env BR2_EXTERNAL )
> +    for br2_ext in ${BR2_EXTERNAL//:/ }; do
> +        mountpoints+=( "${br2_ext}" )
> +    done
> +fi
> +
>   # shellcheck disable=SC2013 # can't use while-read because of the assignment
>   for dir in $(printf '%s\n' "${mountpoints[@]}" |LC_ALL=C sort -u); do
>       docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
> --
> 2.45.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

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

Re: [Buildroot] [PATCH next v4 1/1] utils/docker-run: mount and pass BR2_EXTERNAL dirs

[Yann E. MORIN via buildroot]

Fiona, All,

On 2026-04-05 17:48 +0200, Fiona Klute spake thusly:
> Am 31.08.24 um 21:19 schrieb Fiona Klute via buildroot:
> > From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> > The BR2_EXTERNAL environment variable is passed into the container,
> > and each path listed in it mounted. This allows using external trees
> > when running a build using utils/docker-run. Testing the existence of
> > the variable instead of a non-empty value allows passing an empty
> > BR2_EXTERNAL variable to disable currently set external trees.
[--SNIP--]
> > diff --git a/utils/docker-run b/utils/docker-run
> > index 1adb02d74e..9579f83ce2 100755
> > --- a/utils/docker-run
> > +++ b/utils/docker-run
> > @@ -90,6 +90,13 @@ if [ "${BR2_DL_DIR}" ]; then
> >       docker_opts+=( --env BR2_DL_DIR )
> >   fi
> > 
> > +if [ -v BR2_EXTERNAL ]; then

As this is a bit unusual (but perfectly legit!), maybe a little comment
above the code to explain the intent:

    # Test the variable exists, not just that it is non-empty,
    # to be able to propagate an empty BR2_EXTERNAL.

But -v is a bashism (that's OK, we're using bash in the shabang), but
the more usual (and POSIXly correct [0]) way is to use something like:

    if [ "${BR2_EXTERNAL+set}" ]; then

(an empty string evaluates to false).

I prefer we use standard constructs when they exist, and are not too
cumbersome to use, rather than bashisms that do not provie that much of
an improvement.

> > +    docker_opts+=( --env BR2_EXTERNAL )
> > +    for br2_ext in ${BR2_EXTERNAL//:/ }; do
> > +        mountpoints+=( "${br2_ext}" )
> > +    done

Why the loop? You can just append-assign the expansion:

    mountpoints+=( ${BR2_EXTERNAL//:/ } )

[0] https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_06_02

Regards,
Yann E. MORIN.

> > +fi
> > +
> >   # shellcheck disable=SC2013 # can't use while-read because of the assignment
> >   for dir in $(printf '%s\n' "${mountpoints[@]}" |LC_ALL=C sort -u); do
> >       docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
> > --
> > 2.45.2
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> 

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

Re: [Buildroot] [PATCH next v4 1/1] utils/docker-run: mount and pass BR2_EXTERNAL dirs

[Fiona Klute via buildroot]

Hi Yann,

thanks for your review!

Am 05.04.26 um 18:59 schrieb Yann E. MORIN:
> Fiona, All,
> 
> On 2026-04-05 17:48 +0200, Fiona Klute spake thusly:
>> Am 31.08.24 um 21:19 schrieb Fiona Klute via buildroot:
>>> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
>>> The BR2_EXTERNAL environment variable is passed into the container,
>>> and each path listed in it mounted. This allows using external trees
>>> when running a build using utils/docker-run. Testing the existence of
>>> the variable instead of a non-empty value allows passing an empty
>>> BR2_EXTERNAL variable to disable currently set external trees.
> [--SNIP--]
>>> diff --git a/utils/docker-run b/utils/docker-run
>>> index 1adb02d74e..9579f83ce2 100755
>>> --- a/utils/docker-run
>>> +++ b/utils/docker-run
>>> @@ -90,6 +90,13 @@ if [ "${BR2_DL_DIR}" ]; then
>>>        docker_opts+=( --env BR2_DL_DIR )
>>>    fi
>>>
>>> +if [ -v BR2_EXTERNAL ]; then
> 
> As this is a bit unusual (but perfectly legit!), maybe a little comment
> above the code to explain the intent:
> 
>      # Test the variable exists, not just that it is non-empty,
>      # to be able to propagate an empty BR2_EXTERNAL.
> 
> But -v is a bashism (that's OK, we're using bash in the shabang), but
> the more usual (and POSIXly correct [0]) way is to use something like:
> 
>      if [ "${BR2_EXTERNAL+set}" ]; then
> 
> (an empty string evaluates to false).
> 
> I prefer we use standard constructs when they exist, and are not too
> cumbersome to use, rather than bashisms that do not provie that much of
> an improvement.

Reasonable point, I've changed that in v5.

>>> +    docker_opts+=( --env BR2_EXTERNAL )
>>> +    for br2_ext in ${BR2_EXTERNAL//:/ }; do
>>> +        mountpoints+=( "${br2_ext}" )
>>> +    done
> 
> Why the loop? You can just append-assign the expansion:
> 
>      mountpoints+=( ${BR2_EXTERNAL//:/ } )

I discovered a reason while testing your suggestion: We need to call 
"realpath" on each element (which my previous version didn't do either). 
BR2_EXTERNAL may contain relative paths, but neither Podman nor Docker 
allow those for bind mount source paths. So that's fixed in v5, too.

Best regards,
Fiona

> [0] https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_06_02
> 
> Regards,
> Yann E. MORIN.
> 
>>> +fi
>>> +
>>>    # shellcheck disable=SC2013 # can't use while-read because of the assignment
>>>    for dir in $(printf '%s\n' "${mountpoints[@]}" |LC_ALL=C sort -u); do
>>>        docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
>>> --
>>> 2.45.2
>>>
>>> _______________________________________________
>>> buildroot mailing list
>>> buildroot@buildroot.org
>>> https://lists.buildroot.org/mailman/listinfo/buildroot
>>
> 

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