diff mbox series

[3/3] mediatek: mt7623: use bash for generating bootable images

Message ID 20200908082634.1898371-3-dwmw2@infradead.org
State Accepted
Delegated to: Daniel Golle
Headers show
Series [1/3] mediatek: mt7623n-preloader: add preloader for Banana Pi R64 | expand

Commit Message

David Woodhouse Sept. 8, 2020, 8:26 a.m. UTC
It turns out that 'echo -e' isn't portable; it doesn't work in the dash
builtin echo and Ubuntu users are complaining.

I can't even get octal (specified by POSIX) to work consistently because
those  variants of 'echo' which *do* support -e don't seem to interpret
octalwithout it.

I could switch to /bin/echo but using -e with that isn't actually
portable *either* even though it works today.

For now just stick with bash, and use its builtin. We may end up using
something else entirely; perhaps perl.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---
 target/linux/mediatek/image/gen_mtk_mmc_img.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Felix Fietkau Sept. 8, 2020, 8:31 a.m. UTC | #1
On 2020-09-08 10:26, David Woodhouse wrote:
> It turns out that 'echo -e' isn't portable; it doesn't work in the dash
> builtin echo and Ubuntu users are complaining.
> 
> I can't even get octal (specified by POSIX) to work consistently because
> those  variants of 'echo' which *do* support -e don't seem to interpret
> octalwithout it.
> 
> I could switch to /bin/echo but using -e with that isn't actually
> portable *either* even though it works today.
> 
> For now just stick with bash, and use its builtin. We may end up using
> something else entirely; perhaps perl.
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Have you considered using printf instead of echo?

- Felix
David Woodhouse Sept. 8, 2020, 8:39 a.m. UTC | #2
On Tue, 2020-09-08 at 10:31 +0200, Felix Fietkau wrote:
> On 2020-09-08 10:26, David Woodhouse wrote:
> > It turns out that 'echo -e' isn't portable; it doesn't work in the dash
> > builtin echo and Ubuntu users are complaining.
> > 
> > I can't even get octal (specified by POSIX) to work consistently because
> > those  variants of 'echo' which *do* support -e don't seem to interpret
> > octalwithout it.
> > 
> > I could switch to /bin/echo but using -e with that isn't actually
> > portable *either* even though it works today.
> > 
> > For now just stick with bash, and use its builtin. We may end up using
> > something else entirely; perhaps perl.
> > 
> > Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> 
> Have you considered using printf instead of echo?

I played with that for a while a few weeks ago and didn't manage to get
it working in all environments either. Sometimes I need '\\x' and
sometimes just '\x'.

At the time I gave up as it was mostly a theoretical problem and our
own builds worked, and I had more important things to fix.

But now users are hitting this in practice¹ so I need to at least come
up with a stopgap solution, and this was it.

Better solutions welcome — and we do have a handful of other places in
the tree that need fixing too, when we work out what the best approach
is.

¹ http://forum.banana-pi.org/t/banana-pi-r2-openwrt-19-07-image/11323/80
Felix Fietkau Sept. 8, 2020, 8:42 a.m. UTC | #3
On 2020-09-08 10:39, David Woodhouse wrote:
> On Tue, 2020-09-08 at 10:31 +0200, Felix Fietkau wrote:
>> On 2020-09-08 10:26, David Woodhouse wrote:
>> > It turns out that 'echo -e' isn't portable; it doesn't work in the dash
>> > builtin echo and Ubuntu users are complaining.
>> > 
>> > I can't even get octal (specified by POSIX) to work consistently because
>> > those  variants of 'echo' which *do* support -e don't seem to interpret
>> > octalwithout it.
>> > 
>> > I could switch to /bin/echo but using -e with that isn't actually
>> > portable *either* even though it works today.
>> > 
>> > For now just stick with bash, and use its builtin. We may end up using
>> > something else entirely; perhaps perl.
>> > 
>> > Signed-off-by: David Woodhouse <dwmw2@infradead.org>
>> 
>> Have you considered using printf instead of echo?
> 
> I played with that for a while a few weeks ago and didn't manage to get
> it working in all environments either. Sometimes I need '\\x' and
> sometimes just '\x'.
> 
> At the time I gave up as it was mostly a theoretical problem and our
> own builds worked, and I had more important things to fix.
> 
> But now users are hitting this in practice¹ so I need to at least come
> up with a stopgap solution, and this was it.
> 
> Better solutions welcome — and we do have a handful of other places in
> the tree that need fixing too, when we work out what the best approach
> is.
Sure, using bash is fine. We already require it for many other things.

- Felix
David Woodhouse Sept. 8, 2020, 10:11 a.m. UTC | #4
On Tue, 2020-09-08 at 10:42 +0200, Felix Fietkau wrote:
> Sure, using bash is fine. We already require it for many other
> things.

I did make an effort to remove bashisms as part of the review before
adding this script, but yeah, we can live with bash for now.

For the time being I do still need someone to commit it for me...
please.
diff mbox series

Patch

diff --git a/target/linux/mediatek/image/gen_mtk_mmc_img.sh b/target/linux/mediatek/image/gen_mtk_mmc_img.sh
index ea8a9c63a1..2dacb9019d 100755
--- a/target/linux/mediatek/image/gen_mtk_mmc_img.sh
+++ b/target/linux/mediatek/image/gen_mtk_mmc_img.sh
@@ -1,4 +1,4 @@ 
-#!/bin/sh
+#!/bin/bash
 #
 # Copyright © 2019 Alexey Loukianov <lx2@lexa2.ru>
 # Copyright © 2020 David Woodhouse <dwmw2@infradead.org>