| Message ID | f9cbc137f1fb11e5c64cd2217a65d9727d740c4f.1546412309.git.baruch@tkos.co.il |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,1/2] tools/imx8m_image.sh: remove bashism | expand |
On Wed, 2 Jan 2019 at 07:00, Baruch Siach <baruch@tkos.co.il> wrote: > Use a single '=' to test string equality for compatibility with non-bash > shells. Otherwise, if /bin/sh is dash, build fails: > > ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> Tested-by: Chris Spencer <christopher.spencer@sea.co.uk> Thanks, Chris Disclaimer: This email has been scanned for all known viruses by the MessageLabs Email Security System. The contents of this email (including any attachments) are intended for the use of the mail addressee(s) shown, hence is private and in addition may include commercially sensitive information. If you are not the intended recipient of this email any disclosure, copying, distribution or use of its contents is strictly prohibited. You should notify the sender immediately and then delete it (including any attachments) from your system. The information contained in or attached to this message may also be subject to the export control laws and regulations of the United Kingdom and the United States. This specifically includes, but is not limited to, the Arms Export Control Act (22 U.S.C. 2751-2794) and the International Traffic in Arms Regulation (22 C.F.R 120-130) as well as the Export Administration Act (50 U.S.C. App. 2401-2420), the Export Administration Regulation (15 C.F.R. 730-774) and the UK Export Control Act 2002. Please help out the environment by only printing this e-mail if absolutely necessary - Thank you. SEA is the brand name of Systems Engineering & Assessment Ltd (registered office: Beckington Castle, 17 Castle Corner, Beckington, Frome, Somerset, BA11 6TA, UK - company number 02302168).
Hi Baruch and Stefano, On Wed, Jan 2, 2019 at 4:59 AM Baruch Siach <baruch@tkos.co.il> wrote: > > Use a single '=' to test string equality for compatibility with non-bash > shells. Otherwise, if /bin/sh is dash, build fails: > > ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> I don't see this patch applied yet. Do we have other solution? Thanks
Hi Fabio, On Thu, Feb 28 2019, Fabio Estevam wrote: > On Wed, Jan 2, 2019 at 4:59 AM Baruch Siach <baruch@tkos.co.il> wrote: >> >> Use a single '=' to test string equality for compatibility with non-bash >> shells. Otherwise, if /bin/sh is dash, build fails: >> >> ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator >> ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator >> ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator >> ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator >> WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional >> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > I don't see this patch applied yet. > > Do we have other solution? Stefano suggested an alternative in reply to the other patch: https://lists.denx.de/pipermail/u-boot/2019-January/356941.html baruch
Hello everyone, On Wed, Feb 27, 2019 at 10:44 PM Fabio Estevam <festevam@gmail.com> wrote: > On Wed, Jan 2, 2019 at 4:59 AM Baruch Siach <baruch@tkos.co.il> wrote: > > Use a single '=' to test string equality for compatibility with non-bash > > shells. Otherwise, if /bin/sh is dash, build fails: > > > > ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > > ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > > ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > > ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > > WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional > > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > I don't see this patch applied yet. > > Do we have other solution? This is still broken and the proposed patch does fix it. We should get this merged for 2019.04 release. Please Stefano, consider this for release (be it through your branch for directly by Tom). Best Regards,
Hi Otavio, On 01/04/19 15:20, Otavio Salvador wrote: > Hello everyone, > > On Wed, Feb 27, 2019 at 10:44 PM Fabio Estevam <festevam@gmail.com> wrote: >> On Wed, Jan 2, 2019 at 4:59 AM Baruch Siach <baruch@tkos.co.il> wrote: >>> Use a single '=' to test string equality for compatibility with non-bash >>> shells. Otherwise, if /bin/sh is dash, build fails: >>> >>> ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator >>> ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator >>> ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator >>> ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator >>> WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional >>> >>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >> >> I don't see this patch applied yet. >> >> Do we have other solution? > > This is still broken and the proposed patch does fix it. We should get > this merged for 2019.04 release. > > Please Stefano, consider this for release (be it through your branch > for directly by Tom). > The thing is in which conditions this is broken and why it should be fixed here and in this way. Yes, I admit, I have not pondered this as a big issue - it could be also fixed replacing dash with bash on Debian-like distros, and I guess also on not-debian. And I am quite sure we cannot ensure compatibility with all possible shell that a customer will try , from csh to some exotic variant. Why is this so important to be rapidely fixed here ? Best regards, Stefano
Hi Stefano, On Mon, Apr 1, 2019 at 10:46 AM Stefano Babic <sbabic@denx.de> wrote: > The thing is in which conditions this is broken and why it should be > fixed here and in this way. Yes, I admit, I have not pondered this as a > big issue - it could be also fixed replacing dash with bash on > Debian-like distros, and I guess also on not-debian. And I am quite sure > we cannot ensure compatibility with all possible shell that a customer > will try , from csh to some exotic variant. Why is this so important to > be rapidely fixed here ? On my Ubuntu 18.04 host I cannot build U-Boot for imx8mq-evk without this patch, so it would be really nice if we could get Baruch's or any other solution that fixes this issue. Thanks
On Mon, Apr 1, 2019 at 10:46 AM Stefano Babic <sbabic@denx.de> wrote: > > This is still broken and the proposed patch does fix it. We should get > > this merged for 2019.04 release. > > > > Please Stefano, consider this for release (be it through your branch > > for directly by Tom). > > > > The thing is in which conditions this is broken and why it should be > fixed here and in this way. Yes, I admit, I have not pondered this as a > big issue - it could be also fixed replacing dash with bash on > Debian-like distros, and I guess also on not-debian. And I am quite sure > we cannot ensure compatibility with all possible shell that a customer > will try , from csh to some exotic variant. Why is this so important to > be rapidely fixed here ? The dash uses POSIX syntax and it failing on this is a bad signal. Asking people to change the default system shell to build U-Boot is no sense. We cannot ensure compatibility, for sure, but using bashism makes it worse and non POSIX compatible.
Hi Fabio, On 01/04/19 15:49, Fabio Estevam wrote: > Hi Stefano, > > On Mon, Apr 1, 2019 at 10:46 AM Stefano Babic <sbabic@denx.de> wrote: > >> The thing is in which conditions this is broken and why it should be >> fixed here and in this way. Yes, I admit, I have not pondered this as a >> big issue - it could be also fixed replacing dash with bash on >> Debian-like distros, and I guess also on not-debian. And I am quite sure >> we cannot ensure compatibility with all possible shell that a customer >> will try , from csh to some exotic variant. Why is this so important to >> be rapidely fixed here ? > > On my Ubuntu 18.04 host I cannot build U-Boot for imx8mq-evk without > this patch, so it would be really nice if we could get Baruch's or any > other solution that fixes this issue. Well, it looks like too much because I have 18.04, too, and I can build. I cannot say that Ubuntu users cannot build u-boot anymore. I guess you have /bin/sh -> dash, and /bin/sh -> bash solves the problem, too. Or using update-alternatives, as we are in Ubuntu. I can imagine it is disturbing in docker because a further step in docker-compose to set up the shell is required, too. As far as I know, default is dash (but was not dash default a long of time ago ? I wonder this has become an issue recently, I have missed something in the between). Nevertheless, I am still missing why this is so important to push here when the user can set the shell and build without issues. And travis works, too, it is also based on Ubuntu : trusty, quite old, but I was convinced that dash was the default in Ubuntu since a long time. Maybe I am missing the point here...(to be honest: I put the patch in "Changes requested" a lot of time ago and I have forgotten about it..) Stefano
On Mon, Apr 01, 2019 at 03:46:20PM +0200, Stefano Babic wrote: > Hi Otavio, > > On 01/04/19 15:20, Otavio Salvador wrote: > > Hello everyone, > > > > On Wed, Feb 27, 2019 at 10:44 PM Fabio Estevam <festevam@gmail.com> wrote: > >> On Wed, Jan 2, 2019 at 4:59 AM Baruch Siach <baruch@tkos.co.il> wrote: > >>> Use a single '=' to test string equality for compatibility with non-bash > >>> shells. Otherwise, if /bin/sh is dash, build fails: > >>> > >>> ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > >>> ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator > >>> ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > >>> ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator > >>> WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional > >>> > >>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > >> > >> I don't see this patch applied yet. > >> > >> Do we have other solution? > > > > This is still broken and the proposed patch does fix it. We should get > > this merged for 2019.04 release. > > > > Please Stefano, consider this for release (be it through your branch > > for directly by Tom). > > > > The thing is in which conditions this is broken and why it should be > fixed here and in this way. Yes, I admit, I have not pondered this as a > big issue - it could be also fixed replacing dash with bash on > Debian-like distros, and I guess also on not-debian. And I am quite sure > we cannot ensure compatibility with all possible shell that a customer > will try , from csh to some exotic variant. Why is this so important to > be rapidely fixed here ? The script says it uses /bin/sh and not /bin/bash, so you cannot do bashism in there.
Hi Stefano, On Mon, Apr 1, 2019 at 11:06 AM Stefano Babic <sbabic@denx.de> wrote: > Well, it looks like too much because I have 18.04, too, and I can build. > I cannot say that Ubuntu users cannot build u-boot anymore. > I guess you have /bin/sh -> dash, and /bin/sh -> bash solves the > problem, too. Or using update-alternatives, as we are in Ubuntu. I can > imagine it is disturbing in docker because a further step in > docker-compose to set up the shell is required, too. > As far as I know, default is dash (but was not dash default a long of > time ago ? I wonder this has become an issue recently, I have missed > something in the between). Nevertheless, I am still missing why this is > so important to push here when the user can set the shell and build > without issues. And travis works, too, it is also based on Ubuntu : Travis does not report an error, but Travis does not try to generate a real 'flash.bin' with the required DDR, HDMI and ATF firmwares. The error comes when we try to generate flash.bin with all these firmwares. Without Baruch's patch I can not generate a flash.bin that can boot on im8mq-evk, so that's why I see the lack of this patch as a big issue. Thanks
Hi Fabio, Otavio, On 01/04/19 16:14, Fabio Estevam wrote: > Hi Stefano, > > On Mon, Apr 1, 2019 at 11:06 AM Stefano Babic <sbabic@denx.de> wrote: > >> Well, it looks like too much because I have 18.04, too, and I can build. >> I cannot say that Ubuntu users cannot build u-boot anymore. >> I guess you have /bin/sh -> dash, and /bin/sh -> bash solves the >> problem, too. Or using update-alternatives, as we are in Ubuntu. I can >> imagine it is disturbing in docker because a further step in >> docker-compose to set up the shell is required, too. >> As far as I know, default is dash (but was not dash default a long of >> time ago ? I wonder this has become an issue recently, I have missed >> something in the between). Nevertheless, I am still missing why this is >> so important to push here when the user can set the shell and build >> without issues. And travis works, too, it is also based on Ubuntu : > > Travis does not report an error, but Travis does not try to generate a > real 'flash.bin' with the required DDR, HDMI and ATF firmwares. > > The error comes when we try to generate flash.bin with all these firmwares. > > Without Baruch's patch I can not generate a flash.bin that can boot on > im8mq-evk, so that's why I see the lack of this patch as a big issue. > Ok - convinced. I pick it up. Regards, Stefano
diff --git a/tools/imx8m_image.sh b/tools/imx8m_image.sh index 6346fb64d8c5..ec0881a12812 100755 --- a/tools/imx8m_image.sh +++ b/tools/imx8m_image.sh @@ -12,7 +12,7 @@ blobs=`awk '/^SIGNED_HDMI/ {print $2} /^LOADER/ {print $2} /^SECOND_LOADER/ {pri for f in $blobs; do tmp=$srctree/$f - if [ $f == "spl/u-boot-spl-ddr.bin" ] || [ $f == "u-boot.itb" ]; then + if [ $f = "spl/u-boot-spl-ddr.bin" ] || [ $f = "u-boot.itb" ]; then continue fi @@ -28,7 +28,7 @@ for f in $blobs; do sed -in "s;$f;$tmp;" $file done -if [ $post_process == 1 ]; then +if [ $post_process = 1 ]; then if [ -f $srctree/lpddr4_pmu_train_1d_imem.bin ]; then objcopy -I binary -O binary --pad-to 0x8000 --gap-fill=0x0 $srctree/lpddr4_pmu_train_1d_imem.bin lpddr4_pmu_train_1d_imem_pad.bin objcopy -I binary -O binary --pad-to 0x4000 --gap-fill=0x0 $srctree/lpddr4_pmu_train_1d_dmem.bin lpddr4_pmu_train_1d_dmem_pad.bin
Use a single '=' to test string equality for compatibility with non-bash shells. Otherwise, if /bin/sh is dash, build fails: ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator ./tools/imx8m_image.sh: 15: [: signed_hdmi_imx8m.bin: unexpected operator ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator ./tools/imx8m_image.sh: 15: [: spl/u-boot-spl-ddr.bin: unexpected operator WARNING './spl/u-boot-spl-ddr.bin' not found, resulting binary is not-functional Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- tools/imx8m_image.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)