From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AD9F4C433EF for ; Mon, 31 Jan 2022 22:15:23 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7B69581D5D; Mon, 31 Jan 2022 23:15:21 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="nxKG8cjn"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9D80380615; Mon, 31 Jan 2022 23:15:19 +0100 (CET) Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 390AD80615 for ; Mon, 31 Jan 2022 23:15:16 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qv1-xf31.google.com with SMTP id e20so14245455qvu.7 for ; Mon, 31 Jan 2022 14:15:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=UpdrngaFE/+q2DAdMgO4dfBTYa1XqVH+EBv/rEqT4O8=; b=nxKG8cjnNvaUVErZAHtQY7hjIonJCTh7rCG1qzj18opaZXaznho3SfOAxHfS+RVT6W a5zds7Sz+Nuaqu0FpnBvjIDXpDOkn4vAMvNWXVIftuyP8tIQ8tdX7SRMCMZKhn9e+2Es 9GqADzK1885ZD6IN+lMPw/Zint9GmQwkfBbiM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=UpdrngaFE/+q2DAdMgO4dfBTYa1XqVH+EBv/rEqT4O8=; b=vKm2SToPspm61TLCheiZMApCL7/9i3pY0tUu0o9d9uyYrGaakmHZtrQtBfiigX8R2d QOIhvlrWPvyNemKmNNjpInZRghtf915j3OBFnz2m8LpOJubliDR4DTrsf/iJM0907/ZJ nQLrqoRoDMDZpS9cC2lk1k8ddT/X0GCw3LnXy2xzXRIokzt5Flh2A2MkOI4sjCrmh1oQ L17kcXVlevHGBFFqc227rL9OlD/ASGs66XKmWvWnfJrLw6wqkDWh/3a71PiSymExLVcG vnYpLCOYKBYRsZ2IKGMq/5DDGViraPorxLr0GIQ9OrQgPQpczxxPqQyxs5BEU+wLnxfL SD1Q== X-Gm-Message-State: AOAM531zKhmxFnMAwn6YbiRT1FccGUuhvqM17zojacPi/tcFPy6NIi7b kFcx+/kBv3pXyvhZyjVLYKiKwA== X-Google-Smtp-Source: ABdhPJwglB+RXE1yWpg63urIMov1VXrFuuEce2KtEHaHdwO+gICcnUf9AwkJrswuJAvkTuC0OTVOMw== X-Received: by 2002:a05:6214:19ce:: with SMTP id j14mr20297158qvc.43.1643667314902; Mon, 31 Jan 2022 14:15:14 -0800 (PST) Received: from bill-the-cat (2603-6081-7b01-cbda-2ef0-5dff-fedb-a8ba.res6.spectrum.com. [2603:6081:7b01:cbda:2ef0:5dff:fedb:a8ba]) by smtp.gmail.com with ESMTPSA id f20sm3851929qte.14.2022.01.31.14.15.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 31 Jan 2022 14:15:14 -0800 (PST) Date: Mon, 31 Jan 2022 17:15:12 -0500 From: Tom Rini To: Francis Laniel Cc: u-boot@lists.denx.de, Marek Behun , Michael Nazzareno Trimarchi , Simon Glass , Wolfgang Denk , Harald Seiler Subject: Re: [RFC PATCH v1 00/21] Modernize U-Boot shell Message-ID: <20220131221512.GI7515@bill-the-cat> References: <20211231161327.24918-1-francis.laniel@amarulasolutions.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="FfXVnwzHaJ+SBUCn" Content-Disposition: inline In-Reply-To: <20211231161327.24918-1-francis.laniel@amarulasolutions.com> X-Clacks-Overhead: GNU Terry Pratchett X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean --FfXVnwzHaJ+SBUCn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 31, 2021 at 05:13:06PM +0100, Francis Laniel wrote: > Hi. >=20 > First I hope you are fine and the same for your relatives. >=20 > During 2021 summer, Sean Anderson wrote a contribution to add a new shell= , based > on LIL, to U-Boot [1][2]. > While one of the goals of this contribution was to address the fact actual > U-Boot shell, which is based on Busybox hush, is old there was a discussi= on > about adding a new shell versus updating the actual one [3][4]. >=20 > So, in this series, with Harald Seiler, we updated the actual U-Boot shel= l to > reflect what is currently in Busybox source code. > Basically, this contribution is about taking a snapshot of Busybox shell/= hush.c > file (as it exists in commit 37460f5da) and adapt it to suit U-Boot needs. >=20 > This contribution was written to be as backward-compatible as possible to= avoid > breaking the existing. > So, the 2021 hush flavor offers the same as the actual, that is to say: > 1. Variable expansion. > 2. Instruction lists (;, && and ||). > 3. If, then and else. > 4. Loops (for, while and until). > No new features offered by Busybox hush were implemented (e.g. functions). >=20 > In terms of testing, new unit tests were added to ut to ensure the new be= havior > is the same as the old one and it does not add regression. > Nonetheless, if old behavior was buggy and fixed upstream, the fix is the= n added > to U-Boot [5]. > In sandbox, all of these tests pass smoothly: > 2021> printenv board > board=3Dsandbox > 2021> ut hush > Running 20 hush tests > ... > Failures: 0 > Thanks to the effort of Harald Seiler, I was successful booting a board: > 2021> printenv board_rev > board_rev=3DiMX8MP > 2021> boot > ... > root@iMX8MPboard:~# fw_printenv board_rev > board_rev=3DiMX8MP >=20 > I marked this contribution as RFC to indeed collect your opinion. > My goal is not to change suddenly actual shell to this one, we clearly ne= ed a > transition period to think about it. > I think it is better to see this contribution as a proof of concept which= shows > it is possible to update the actual shell. >=20 > If you want to review it - your review will really be appreciated - here = are > some information regarding the commits: > * commits marked as "test:" deal with unit tests. > * commit "cli: Add Busybox upstream hush.c file." copies Busybox shell/hu= sh.c > into U-Boot tree, this explain why this commit contains around 12000 addi= tions. > * commit "cli: Port Busybox 2021 hush to U-Boot." modifies previously add= ed file > to permit us to use this as new shell. > The really good idea of #include'ing Busybox code into a wrapper file to = define > some particular functions while minimizing modifications to upstream code= comes > from Harald Seiler. > * Other commits focus on enabling features we need (e.g. if). >=20 > Francis Laniel (21): > test: Add framework to test hush behavior. > test: hush: Test hush if/else. > test/py: hush_if_test: Remove the test file. > test: hush: Test hush variable expansion. > test: hush: Test hush commands list. > test: hush: Test hush loops. > cli: Add Busybox upstream hush.c file. > cli: Port Busybox 2021 hush to U-Boot. > cli: Add choice for hush parser. > cli: Add HUSH_2021_PARSER to hush parser choice . > cli: Enables using hush 2021 parser as command line parser. > cli: hush_2021: Enable variables expansion for hush 2021. > cli: hush_2021: Add functions to be called from run_command(). > cli: Modify run_command() to add hush 2021 as parser. > test: hush: Fix instructions list tests for hush 2021. > test: hush: Fix variable expansion tests for hush 2021. > cli: hush_2021: Enable using \< and \> as string compare operators. > cli: hush_2021: Enable if keyword. > test: hush: Fix if tests for hush 2021. > cli: hush_2021: Enable loops. > test: hush: Fix loop tests for hush 2021. >=20 > cmd/Kconfig | 22 + > common/Makefile | 3 +- > common/cli.c | 27 +- > common/cli_hush_2021.c | 309 + > common/cli_hush_2021_upstream.c | 12791 +++++++++++++++++++++++++++ > include/cli_hush.h | 8 + > include/test/hush.h | 15 + > include/test/suites.h | 1 + > test/Makefile | 3 + > test/cmd_ut.c | 6 + > test/hush/Makefile | 10 + > test/hush/cmd_ut_hush.c | 20 + > test/hush/dollar.c | 242 + > test/hush/if.c | 351 + > test/hush/list.c | 127 + > test/hush/loop.c | 84 + > test/py/tests/test_hush_if_test.py | 184 - > 17 files changed, 14015 insertions(+), 188 deletions(-) > create mode 100644 common/cli_hush_2021.c > create mode 100644 common/cli_hush_2021_upstream.c > create mode 100644 include/test/hush.h > create mode 100644 test/hush/Makefile > create mode 100644 test/hush/cmd_ut_hush.c > create mode 100644 test/hush/dollar.c > create mode 100644 test/hush/if.c > create mode 100644 test/hush/list.c > create mode 100644 test/hush/loop.c > delete mode 100644 test/py/tests/test_hush_if_test.py >=20 >=20 > Best regards, thank you in advance for your reviews and happy new year! Sorry for the extremely delayed response here. I'm glad you did this. The first thing I see is that there's still some fail to build problems to sort out: https://source.denx.de/u-boot/u-boot/-/commits/TEST/RFC-modernize-u-boot-sh= ell and doing a world build locally shows a few more also fall out so please give https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html a read on how to get at least an Azure run for the next iteration. When enabled by default there's a few unused function warnings (I enabled the option for all boards for a test) that also need to be addressed. Then the next step would be to get all of the current tests to pass, for me I get: FAILED test/py/tests/test_md.py::test_md_repeat - AssertionError: assert '0= 0200040: ' in '' FAILED test/py/tests/test_ut.py::test_ut[ut_hush_hush_test_simple_dollar] -= Exception: Bad p... FAILED test/py/tests/test_ut.py::test_ut[ut_lib_lib_test_hush_echo] - Excep= tion: Bad pattern... after dropping the forced SYS_PROMPT. Again, thanks for doing this and I do look forward to the next posting. --=20 Tom --FfXVnwzHaJ+SBUCn Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmH4X3AACgkQFHw5/5Y0 tyxWJwv/RlgGHtZtNYTJf1mDTggvA9+bEWknXrogtPjrQCKLmFMQvIMkjCKq5WlD 7ImMAaeHtRhOHU+oc8TiylVIyB8WFvqR0jUntglnvojSExiN4Cen2fOf/ANKwzT6 o+SVaAxp0bIwg21DzjOLjuJTQdscBiR29jA7bUukuHUU/SiVFP76oKkELhzBKmNK i45BVqYOro8wH2ejc3R0CrmpVC0SQH20nxj0dNzSbKzhCEOJm8OITcmOfjY58B3/ YgvAM4+7mEdvjBBB0dJZM+IovW47Alimnc6lFZikb5cnsHhbrLbzJBnIu3G3aAPt IV92SImSiNS7psR36ZgkgbbPWp5vSEURLBSArNCQ3q2bbf5ng9TSZUor2KoEcBMj BRvvDwDCjogM+bPnPEjZlI07Wjcv18JAgqfhVQRBvgrE1en40bNuS1QVoWwtG0P0 pRYlvo1zJcT2Y5byHIlNB+ULizoRMxhPp+5t3yuke0ZlYv3pduGOlHThUUVayEfg av6jirwO =IsPf -----END PGP SIGNATURE----- --FfXVnwzHaJ+SBUCn--