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-08-31 18:57 +0200, Fiona Klute spake thusly:
> Am 30.08.24 um 23:35 schrieb Yann E. MORIN:
> > 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.
[--SNIP--]
> > 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...)
> 
> According to the manual multiple paths in BR2_EXTERNAL must be separated
> by colons, not spaces, example copied verbatim from
> docs/manual/customize-outside-br.adoc:
> 
> buildroot/ $ make BR2_EXTERNAL=/path/to/foo:/where/we/have/bar menuconfig

Supporting both colon- and space-separated lists was the worst idea I
had when I implemented and coded support for mutliple br2-externals.

In fact, the manual has an example with a colon, but nowhere does it
explicitly states that it can be either.

> support/scripts/br2-external splits BR2_EXTERNAL by colon, or rather
> replaces colons with spaces in the variable value and then uses the
> result unquoted to create a list of parameters.

Note that the code (which you quoted in your reply to patch 2) is:

    do_validate "${outputdir}" ${@//:/ }

So, it is going to split the words on spaces. E.g. passing:
    BR2_EXTERNAL=/path/to some/dir:/other/path

will result in this call:

    do_validate "${outputdir}" /path/to some/dir /other/path

which will be three arguments. And that would fail:

    $ BR2_EXTERNAL="/home/ymorin/dev/buildroot/misc/br2-external-tests/br2-external-1:/home/ymorin/dev/buildroot/misc/br2-external-tests/br2 external" make defconfig
    Makefile:194: *** '/home/ymorin/dev/buildroot/misc/br2-external-tests/br2': no such file or directory.  Stop.
    make: *** [Makefile:23: _all] Error 2

(I need to polish and push my br2-external test trees to gitlab...)

Also, internally, BR2_EXTERNAL is saved back as a space-separated list:

    https://gitlab.com/buildroot.org/buildroot/-/blob/master/support/scripts/br2-external?ref_type=heads#L124

    printf 'BR2_EXTERNAL ?='
    for br2_name in "${BR2_EXT_NAMES[@]}"; do
        eval br2_ext="\"\${BR2_EXT_PATHS_${br2_name}}\""
        printf ' %s' "${br2_ext}"
    done
    printf '\n'

... so that calling 'make' again without specifyig BR2_EXTERNAL will
still use the BR2_EXTERNAL trees used at configure time.

So, definitely, br2-external trees with spaces in them is not supported
and is not working.

> That means
> utils/docker-run must split by colon, too, no matter if we want to make
> the handling of the individual paths after splitting space-safe or not.

Damn right you are, damit! ;-) So the code could just look like:

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

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