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

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Fiona, All,

On 2024-07-13 21:22 +0200, Fiona Klute via buildroot spake thusly:
> 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>
> ---
> 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 | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/docker-run b/utils/docker-run
> index 1adb02d74e..dbb9c45fdd 100755
> --- a/utils/docker-run
> +++ b/utils/docker-run
> @@ -90,10 +90,17 @@ if [ "${BR2_DL_DIR}" ]; then
>      docker_opts+=( --env BR2_DL_DIR )
>  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
> +if [ -v BR2_EXTERNAL ]; then
> +    IFS=":" read -r -a br2_ext_arr <<< "${BR2_EXTERNAL}"
> +    for br2_ext in "${br2_ext_arr[@]}"; do
> +        mountpoints+=( "${br2_ext}" )
> +    done
> +    docker_opts+=( --env BR2_EXTERNAL )
> +fi
> +
> +while IFS=$'\n' read -r dir; do
>      docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
> -done
> +done < <(printf '%s\n' "${mountpoints[@]}" | LC_ALL=C sort -u)

This commit mixes multiple changes:
  - using process substitution with a while-loop, instead of the
    command substitution and a for-loop
  - passing br2-external and mounting trees in the container
  - handling br2-external trees with spaces in them

I see you have a second patch with support for br2-external trees with
spaces in their paths. If at all, it should come before this one.

However, I don't think we should support paths with spaces. We currently
do not, and the rest of the code base (as your second patch states) may
or may not work (hint: it does not, it's broken in a lot of places, and
the buildsystems of many packages do not support spaces in path either).

Also, BR2_EXTERNAL must be a space-separated list, as it needs to be
split on space from Makefile context, so no br2-external tree with
spaces in paths can even work... (unless I missed something...)

So, I'm OK with passing br2-external to the container, but not with the
rest of the changes in this file. I.e. the change whould be just that:

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

Regards,
Yann E. MORIN.

>  if tty -s; then
>      docker_opts+=( -t )
> --
> 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