| Message ID | 20240713192207.873065-2-fiona.klute@gmx.de |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v3,1/2] utils/docker-run: mount and pass BR2_EXTERNAL dirs | expand |
On Sat Jul 13, 2024 at 7:22 PM UTC, Fiona Klute via buildroot wrote: > From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de> > > "make list-defconfigs" and "make nconfig" work in the container used > by utils/docker-run with external trees with spaces in the directory > name. Other build steps may or may not work. > > Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de> Reviewed-by: Brandon Maier <brandon.maier@collins.com> > --- > This is mostly a proof-of-concept to show the previous patch works, > there might still be things that break in other parts of the build. I > have not checked if "printf '%q'" works in Bash 3.1 (like a comment > claims the script needs to support, not sure if still current). > > Changes v1 -> v2: > * Added this patch > > support/scripts/br2-external | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/support/scripts/br2-external b/support/scripts/br2-external > index 8aea479d20..b8b6179aa8 100755 > --- a/support/scripts/br2-external > +++ b/support/scripts/br2-external > @@ -34,7 +34,8 @@ main() { > trap "error 'unexpected error while generating ${ofile}\n'" ERR > > mkdir -p "${outputdir}" > - do_validate "${outputdir}" ${@//:/ } > + IFS=":" read -r -a br2_extdirs <<< "${@}" > + do_validate "${outputdir}" "${br2_extdirs[@]}" > do_mk "${outputdir}" > do_kconfig "${outputdir}" > } > @@ -144,9 +145,9 @@ do_mk() { > eval br2_ver="\"\${BR2_EXT_VERS_${br2_name}}\"" > printf '\n' > printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}" > - printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}" > - printf 'BR2_EXTERNAL_MKS += %s/external.mk\n' "${br2_ext}" > - printf 'export BR2_EXTERNAL_%s_PATH = %s\n' "${br2_name}" "${br2_ext}" > + printf 'BR2_EXTERNAL_DIRS += %q\n' "${br2_ext}" > + printf 'BR2_EXTERNAL_MKS += %q/external.mk\n' "${br2_ext}" > + printf 'export BR2_EXTERNAL_%s_PATH = %q\n' "${br2_name}" "${br2_ext}" > printf 'export BR2_EXTERNAL_%s_DESC = %s\n' "${br2_name}" "${br2_desc}" > printf 'export BR2_EXTERNAL_%s_VERSION = %s\n' "${br2_name}" "${br2_ver}" > done > -- > 2.45.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Fiona, All, On 2024-07-13 21:22 +0200, Fiona Klute via buildroot spake thusly: > From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de> > "make list-defconfigs" and "make nconfig" work in the container used > by utils/docker-run with external trees with spaces in the directory > name. Other build steps may or may not work. As you state yourself, "Other build steps may or may not work". And indeed, that's probably not going to work, as BR2_EXTERNAL is expected to be a space-separated list, that will be split on spaces from Makefile context. So, no amount of quoting will make br2-external trees with spaces actually work (AFAICS). Regards, Yann E. MORIN. > Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de> > --- > This is mostly a proof-of-concept to show the previous patch works, > there might still be things that break in other parts of the build. I > have not checked if "printf '%q'" works in Bash 3.1 (like a comment > claims the script needs to support, not sure if still current). > > Changes v1 -> v2: > * Added this patch > > support/scripts/br2-external | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/support/scripts/br2-external b/support/scripts/br2-external > index 8aea479d20..b8b6179aa8 100755 > --- a/support/scripts/br2-external > +++ b/support/scripts/br2-external > @@ -34,7 +34,8 @@ main() { > trap "error 'unexpected error while generating ${ofile}\n'" ERR > > mkdir -p "${outputdir}" > - do_validate "${outputdir}" ${@//:/ } > + IFS=":" read -r -a br2_extdirs <<< "${@}" > + do_validate "${outputdir}" "${br2_extdirs[@]}" > do_mk "${outputdir}" > do_kconfig "${outputdir}" > } > @@ -144,9 +145,9 @@ do_mk() { > eval br2_ver="\"\${BR2_EXT_VERS_${br2_name}}\"" > printf '\n' > printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}" > - printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}" > - printf 'BR2_EXTERNAL_MKS += %s/external.mk\n' "${br2_ext}" > - printf 'export BR2_EXTERNAL_%s_PATH = %s\n' "${br2_name}" "${br2_ext}" > + printf 'BR2_EXTERNAL_DIRS += %q\n' "${br2_ext}" > + printf 'BR2_EXTERNAL_MKS += %q/external.mk\n' "${br2_ext}" > + printf 'export BR2_EXTERNAL_%s_PATH = %q\n' "${br2_name}" "${br2_ext}" > printf 'export BR2_EXTERNAL_%s_DESC = %s\n' "${br2_name}" "${br2_desc}" > printf 'export BR2_EXTERNAL_%s_VERSION = %s\n' "${br2_name}" "${br2_ver}" > done > -- > 2.45.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Am 30.08.24 um 23:38 schrieb Yann E. MORIN: > Fiona, All, > > On 2024-07-13 21:22 +0200, Fiona Klute via buildroot spake thusly: >> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de> >> "make list-defconfigs" and "make nconfig" work in the container used >> by utils/docker-run with external trees with spaces in the directory >> name. Other build steps may or may not work. > > As you state yourself, "Other build steps may or may not work". And > indeed, that's probably not going to work, as BR2_EXTERNAL is expected > to be a space-separated list, that will be split on spaces from Makefile > context. So, no amount of quoting will make br2-external trees with > spaces actually work (AFAICS). BR2_EXTERNAL is colon separated, the current code simply replaces colons with spaces and then applies word splitting: do_validate "${outputdir}" ${@//:/ } If writing the paths to the makefiles quoted isn't safe (it does work e.g. with "make list-defconfigs", but I don't know what would happen later in the build) then I think do_validate_one should check for spaces in the paths and error out if there are any. Sure, the "does the path exist" check is likely to fail anyway on a path split in the middle, but the error message would equally likely be a bit confusing. Best regards, Fiona > Regards, > Yann E. MORIN. > >> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de> >> --- >> This is mostly a proof-of-concept to show the previous patch works, >> there might still be things that break in other parts of the build. I >> have not checked if "printf '%q'" works in Bash 3.1 (like a comment >> claims the script needs to support, not sure if still current). >> >> Changes v1 -> v2: >> * Added this patch >> >> support/scripts/br2-external | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/support/scripts/br2-external b/support/scripts/br2-external >> index 8aea479d20..b8b6179aa8 100755 >> --- a/support/scripts/br2-external >> +++ b/support/scripts/br2-external >> @@ -34,7 +34,8 @@ main() { >> trap "error 'unexpected error while generating ${ofile}\n'" ERR >> >> mkdir -p "${outputdir}" >> - do_validate "${outputdir}" ${@//:/ } >> + IFS=":" read -r -a br2_extdirs <<< "${@}" >> + do_validate "${outputdir}" "${br2_extdirs[@]}" >> do_mk "${outputdir}" >> do_kconfig "${outputdir}" >> } >> @@ -144,9 +145,9 @@ do_mk() { >> eval br2_ver="\"\${BR2_EXT_VERS_${br2_name}}\"" >> printf '\n' >> printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}" >> - printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}" >> - printf 'BR2_EXTERNAL_MKS += %s/external.mk\n' "${br2_ext}" >> - printf 'export BR2_EXTERNAL_%s_PATH = %s\n' "${br2_name}" "${br2_ext}" >> + printf 'BR2_EXTERNAL_DIRS += %q\n' "${br2_ext}" >> + printf 'BR2_EXTERNAL_MKS += %q/external.mk\n' "${br2_ext}" >> + printf 'export BR2_EXTERNAL_%s_PATH = %q\n' "${br2_name}" "${br2_ext}" >> printf 'export BR2_EXTERNAL_%s_DESC = %s\n' "${br2_name}" "${br2_desc}" >> printf 'export BR2_EXTERNAL_%s_VERSION = %s\n' "${br2_name}" "${br2_ver}" >> done >> -- >> 2.45.2 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@buildroot.org >> https://lists.buildroot.org/mailman/listinfo/buildroot >
diff --git a/support/scripts/br2-external b/support/scripts/br2-external index 8aea479d20..b8b6179aa8 100755 --- a/support/scripts/br2-external +++ b/support/scripts/br2-external @@ -34,7 +34,8 @@ main() { trap "error 'unexpected error while generating ${ofile}\n'" ERR mkdir -p "${outputdir}" - do_validate "${outputdir}" ${@//:/ } + IFS=":" read -r -a br2_extdirs <<< "${@}" + do_validate "${outputdir}" "${br2_extdirs[@]}" do_mk "${outputdir}" do_kconfig "${outputdir}" } @@ -144,9 +145,9 @@ do_mk() { eval br2_ver="\"\${BR2_EXT_VERS_${br2_name}}\"" printf '\n' printf 'BR2_EXTERNAL_NAMES += %s\n' "${br2_name}" - printf 'BR2_EXTERNAL_DIRS += %s\n' "${br2_ext}" - printf 'BR2_EXTERNAL_MKS += %s/external.mk\n' "${br2_ext}" - printf 'export BR2_EXTERNAL_%s_PATH = %s\n' "${br2_name}" "${br2_ext}" + printf 'BR2_EXTERNAL_DIRS += %q\n' "${br2_ext}" + printf 'BR2_EXTERNAL_MKS += %q/external.mk\n' "${br2_ext}" + printf 'export BR2_EXTERNAL_%s_PATH = %q\n' "${br2_name}" "${br2_ext}" printf 'export BR2_EXTERNAL_%s_DESC = %s\n' "${br2_name}" "${br2_desc}" printf 'export BR2_EXTERNAL_%s_VERSION = %s\n' "${br2_name}" "${br2_ver}" done