Hi Tom,
On Thu, 6 Feb 2025 at 10:10, Tom Rini <tr...@konsulko.com> wrote:
>
> On Thu, Feb 06, 2025 at 06:05:36AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 3 Feb 2025 at 13:07, Tom Rini <tr...@konsulko.com> wrote:
> > >
> > > On Mon, Feb 03, 2025 at 10:41:54AM -0700, Simon Glass wrote:
> > >
> > > > Before this gets any longer, convert it to Python so it is easier to
> > > > maintain.
> > > >
> > > > Signed-off-by: Simon Glass <s...@chromium.org>
> > > > ---
> > > >
> > > > MAINTAINERS | 2 +-
> > > > doc/develop/uefi/u-boot_on_efi.rst | 4 +-
> > > > scripts/build-efi.py | 258 +++++++++++++++++++++++++++++
> > > > scripts/build-efi.sh | 207 -----------------------
> > > > 4 files changed, 261 insertions(+), 210 deletions(-)
> > > > create mode 100755 scripts/build-efi.py
> > > > delete mode 100755 scripts/build-efi.sh
> > >
> > > This should just go live in the u-boot-extras repository as another
> > > sample script. And as a bonus, there you can just push the script right
> > > now.
> >
> > But I can't be sure it will stay that way. In any case I see these
> > scripts as first-class citizens, in the docs, tested in CI, used by
> > people to report bugs, updated as new features appear, etc.
>
> Yup, I still think that's the wrong way of looking at it. It's a sample
> script on how to use this for QEMU which means it's useful as a debug
> and development aid. It's bad for CI since if we want to deal with "CI
> is slow" we need to pre-create test disk images, not do that on the fly.
> And you've got sudo and various external blobs being used. No, it does
> not belong in the main repository and I'm sorry I missed it the first
> time.
>
> And converting this to python is even worse as we have yet another tool
> that should have a proper requirements.txt file and keeping that up to
> date. And then oh, the inevitable "here's a security issue in U-Boot!"
> (by which they mean an issue in a support script and not what's deployed
> and used).
IMO development aids are quite important for efficient development. We
have things like scripts/gen_compile_commands.py and checkpatch.pl
which I use quite a bit. Cloning another repo just to be able to
develop on U-Boot efficiently seems daft to me.
We don't pre-create any test-disk images at present(?) I'm not even
sure that is a good idea, since it means that updating a test becomes
a lot harder. It would save a little time, but the thing which creates
the image only takes a few seconds on tui (although this happens on
each runner on the test.py stage):
2.6s test_ut_dm_init_bootstd
Python is easier to maintain than shell when it gets beyond 100 lines
or so. We could use the same requirements.txt we have for test/py and
I'm not sure how to deal with the security side. It sounds like
calling it a contrib/ directory might be enough.
>
> So please, lets just move this to the extras repository, and point to
> *that* in our docs enough that it's clear that we have tooling that can
> be useful in some cases there.
So long as I can point to it in docs and never lose access to the repo
in the future, I think I can live with this. At least I can try it.
I think I could add it as a repo in the contriib/ subdir of the U-Boot
directory (with a .gitignore in U-Boot), so perhaps I can mostly
forget that it is separate.
Regards,
Simon
