* [Buildroot] [PATCH] support/dependencies: detect and bailout when PATH contains spaces/TABs
@ 2021-03-02 12:33 Yann E. MORIN
2021-03-02 15:43 ` [Buildroot] environment variables with spaces (request for comments) Dan Raymond
0 siblings, 1 reply; 3+ messages in thread
From: Yann E. MORIN @ 2021-03-02 12:33 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 importantluy, 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>
---
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 | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index b44d28682c..8cbfa984c7 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -2,6 +2,8 @@
# vi: set sw=4 ts=4:
export LC_ALL=C
+TAB="$(printf '\t')"
+NL="$(printf '\n')"
# Verify that grep works
echo "WORKS" | grep "WORKS" >/dev/null 2>&1
@@ -35,9 +37,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] environment variables with spaces (request for comments)
2021-03-02 12:33 [Buildroot] [PATCH] support/dependencies: detect and bailout when PATH contains spaces/TABs Yann E. MORIN
@ 2021-03-02 15:43 ` Dan Raymond
2021-03-03 16:35 ` Arnout Vandecappelle
0 siblings, 1 reply; 3+ messages in thread
From: Dan Raymond @ 2021-03-02 15:43 UTC (permalink / raw)
To: buildroot
I encountered a bug in Buildroot that resulted in a failure to run 'make
linux-menuconfig'.? I tracked down the problem to a bug in
package/pkg-kconfig.mk that was attempting to remove a set of variables
from an environment string.? The problem is that a naive technique was
being used that did not account for quoted variables that contained
embedded spaces.? To be clear, 'make menuconfig' from the linux kernel
tree works fine: this bug is in Buildroot only.? The reason I discovered
this bug (and others may not) is because my PATH environment variable
contains embedded spaces (which may be uncommon but valid and sometimes
necessary).
I implemented/tested/submitted a patch that reliably fixes the bug.
Yann MORIN objected to my patch because he believes some of the packages
Buildroot supports contain similar bugs.? I responded to his objection
but he ignored my response and submitted his own patch that undermines
mine by intentionally causing ALL make targets to fail when the PATH
environment variable contains embedded spaces.? I don't think that
starting a "patch war" is productive or contributes to the goals of the
Buildroot community.? Can we have a discussion instead and come to a
consensus on this matter?
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Buildroot] environment variables with spaces (request for comments)
2021-03-02 15:43 ` [Buildroot] environment variables with spaces (request for comments) Dan Raymond
@ 2021-03-03 16:35 ` Arnout Vandecappelle
0 siblings, 0 replies; 3+ messages in thread
From: Arnout Vandecappelle @ 2021-03-03 16:35 UTC (permalink / raw)
To: buildroot
On 02/03/2021 16:43, Dan Raymond wrote:
> I encountered a bug in Buildroot that resulted in a failure to run 'make
> linux-menuconfig'.? I tracked down the problem to a bug in
> package/pkg-kconfig.mk that was attempting to remove a set of variables from an
> environment string.? The problem is that a naive technique was being used that
> did not account for quoted variables that contained embedded spaces.? To be
> clear, 'make menuconfig' from the linux kernel tree works fine: this bug is in
> Buildroot only.? The reason I discovered this bug (and others may not) is
> because my PATH environment variable contains embedded spaces (which may be
> uncommon but valid and sometimes necessary).
>
> I implemented/tested/submitted a patch that reliably fixes the bug.
>
> Yann MORIN objected to my patch because he believes some of the packages
> Buildroot supports contain similar bugs.? I responded to his objection but he
> ignored my response and submitted his own patch that undermines mine by
> intentionally causing ALL make targets to fail when the PATH environment
> variable contains embedded spaces.? I don't think that starting a "patch war" is
> productive or contributes to the goals of the Buildroot community.? Can we have
> a discussion instead and come to a consensus on this matter?
Yann's point is absolutely valid: any configuration except the most trivial one
will fail to build when PATH contains spaces, and it will do so in a
spectacularly difficult to debug way. So Yann's patch is definitely needed.
However, that shouldn't stop us from fixing things where we can anyway.
Thus, I'm inclined to merge both patches. However, I have some reservations
about the fix as well - I'll ventilate those in the patch thread.
Regards,
Arnout
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-03 16:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 12:33 [Buildroot] [PATCH] support/dependencies: detect and bailout when PATH contains spaces/TABs Yann E. MORIN
2021-03-02 15:43 ` [Buildroot] environment variables with spaces (request for comments) Dan Raymond
2021-03-03 16:35 ` Arnout Vandecappelle
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.