All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCHv2] support/dependencies: detect and bailout when PATH contains spaces/TABs
@ 2021-03-06  9:19 Yann E. MORIN
  2021-03-14 22:30 ` Thomas Petazzoni
  2021-03-21 19:00 ` Peter Korsgaard
  0 siblings, 2 replies; 3+ messages in thread
From: Yann E. MORIN @ 2021-03-06  9:19 UTC (permalink / raw)
  To: buildroot

In Makefiles, variables are split, filtered, and otherwise mangled on a
space as a separator. In a shell, they will also be split on TABs.

We split and filter and iterate of variables in a lot of places, and
most importantly, spaces in PATH is very seldom tested, if at all, so a
lot of packages will not be working properly in such a situation.

For example, the config.guess contains constructs that are not resilient
to a space in PATH:

    PATH=$PATH:/.attbin ; export PATH

Also, our fakedate will iterate over PATH:

    for P in `echo $PATH | tr ':' ' '`; do

Those are only two cases, but the first means basically all
autotools-based packages are susceptible to subtle breakage.

Furthermore, Buildroot itself does not support that the top-level or
output directories are in a path with spaces anyway.

So, instead of chasing all cases that might be potentially broken,
let's just detect the case and bail out, like we already do when PATH
contains a \n, or when it contains the current working directory.

Reported-by: Dan Raymond <draymond@foxvalley.net>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
Changes v1 -> v2:
  - fix the newline case

---
Notes: Dan had provided a patch [0] that would fix their own specific
issue when runing linux-menuconfig, but I believe this is by far
insufficient to properly solve the issues with spaces in PATH, and I
believe it can't be reliably fixed (at least not in the foreseeable
future with our available manpower), so I prefer that we detect the
situation and bail out.

[0] https://patchwork.ozlabs.org/project/buildroot/patch/16100eb6-bac8-2e4d-65f2-26333179f3b8 at foxvalley.net/
---
 support/dependencies/dependencies.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index b44d28682c..1954f038be 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -2,6 +2,9 @@
 # vi: set sw=4 ts=4:
 
 export LC_ALL=C
+TAB="$(printf '\t')"
+NL="
+"
 
 # Verify that grep works
 echo "WORKS" | grep "WORKS" >/dev/null 2>&1
@@ -35,9 +38,9 @@ case ":${PATH:-unset}:" in
 	echo "PATH environment variable. This doesn't work."
 	exit 1
 	;;
-(*"
-"*)	printf "\n"
-	printf "Your PATH contains a newline (\\\n) character.\n"
+(*" "*|*"${TAB}"*|*"${NL}"*)
+	printf "\n"
+	printf "Your PATH contains spaces, TABs, and/or newline (\\\n) characters.\n"
 	printf "This doesn't work. Fix you PATH.\n"
 	exit 1
 	;;
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [Buildroot] [PATCHv2] support/dependencies: detect and bailout when PATH contains spaces/TABs
  2021-03-06  9:19 [Buildroot] [PATCHv2] support/dependencies: detect and bailout when PATH contains spaces/TABs Yann E. MORIN
@ 2021-03-14 22:30 ` Thomas Petazzoni
  2021-03-21 19:00 ` Peter Korsgaard
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Petazzoni @ 2021-03-14 22:30 UTC (permalink / raw)
  To: buildroot

On Sat,  6 Mar 2021 10:19:30 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> In Makefiles, variables are split, filtered, and otherwise mangled on a
> space as a separator. In a shell, they will also be split on TABs.
> 
> We split and filter and iterate of variables in a lot of places, and
> most importantly, spaces in PATH is very seldom tested, if at all, so a
> lot of packages will not be working properly in such a situation.
> 
> For example, the config.guess contains constructs that are not resilient
> to a space in PATH:
> 
>     PATH=$PATH:/.attbin ; export PATH
> 
> Also, our fakedate will iterate over PATH:
> 
>     for P in `echo $PATH | tr ':' ' '`; do
> 
> Those are only two cases, but the first means basically all
> autotools-based packages are susceptible to subtle breakage.
> 
> Furthermore, Buildroot itself does not support that the top-level or
> output directories are in a path with spaces anyway.
> 
> So, instead of chasing all cases that might be potentially broken,
> let's just detect the case and bail out, like we already do when PATH
> contains a \n, or when it contains the current working directory.
> 
> Reported-by: Dan Raymond <draymond@foxvalley.net>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> ---
> Changes v1 -> v2:
>   - fix the newline case

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Buildroot] [PATCHv2] support/dependencies: detect and bailout when PATH contains spaces/TABs
  2021-03-06  9:19 [Buildroot] [PATCHv2] support/dependencies: detect and bailout when PATH contains spaces/TABs Yann E. MORIN
  2021-03-14 22:30 ` Thomas Petazzoni
@ 2021-03-21 19:00 ` Peter Korsgaard
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Korsgaard @ 2021-03-21 19:00 UTC (permalink / raw)
  To: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > In Makefiles, variables are split, filtered, and otherwise mangled on a
 > space as a separator. In a shell, they will also be split on TABs.

 > We split and filter and iterate of variables in a lot of places, and
 > most importantly, spaces in PATH is very seldom tested, if at all, so a
 > lot of packages will not be working properly in such a situation.

 > For example, the config.guess contains constructs that are not resilient
 > to a space in PATH:

 >     PATH=$PATH:/.attbin ; export PATH

 > Also, our fakedate will iterate over PATH:

 >     for P in `echo $PATH | tr ':' ' '`; do

 > Those are only two cases, but the first means basically all
 > autotools-based packages are susceptible to subtle breakage.

 > Furthermore, Buildroot itself does not support that the top-level or
 > output directories are in a path with spaces anyway.

 > So, instead of chasing all cases that might be potentially broken,
 > let's just detect the case and bail out, like we already do when PATH
 > contains a \n, or when it contains the current working directory.

 > Reported-by: Dan Raymond <draymond@foxvalley.net>
 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
 > Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 > ---
 > Changes v1 -> v2:
 >   - fix the newline case

 > ---
 > Notes: Dan had provided a patch [0] that would fix their own specific
 > issue when runing linux-menuconfig, but I believe this is by far
 > insufficient to properly solve the issues with spaces in PATH, and I
 > believe it can't be reliably fixed (at least not in the foreseeable
 > future with our available manpower), so I prefer that we detect the
 > situation and bail out.

 > [0] https://patchwork.ozlabs.org/project/buildroot/patch/16100eb6-bac8-2e4d-65f2-26333179f3b8 at foxvalley.net/

Committed to 2020.02.x, 2020.11.x and 2021.02.x, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-03-21 19:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-06  9:19 [Buildroot] [PATCHv2] support/dependencies: detect and bailout when PATH contains spaces/TABs Yann E. MORIN
2021-03-14 22:30 ` Thomas Petazzoni
2021-03-21 19:00 ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.