[U-Boot] U-Boot: Environment flags broken for U-Boot
joe.hershberger at gmail.com
Wed Sep 4 18:30:02 UTC 2019
On Wed, Sep 4, 2019 at 1:01 PM Tom Rini <trini at konsulko.com> wrote:
> On Tue, Sep 03, 2019 at 10:04:42AM +0200, Wolfgang Denk wrote:
> > Dear Tom,
> > In message <a78f0b04-c3f7-45d5-e9ac-90522dbefc2e at denx.de> Heiko Schocher wrote:
> > >
> > > I am just testing U-Boot Environment flags and they do not work anymore with
> > > current mainline U-Boot ...
> > ...
> > > reason is your commit:
> > >
> > > commit 7d4776545b0f8a8827e5d061206faf61c9ba6ea9
> > > Author: Patrick Delaunay <patrick.delaunay at st.com>
> > > Date: Thu Apr 18 17:32:49 2019 +0200
> > >
> > > env: solve compilation error in SPL
> > Looking into the history of this, I wonder if we could / should
> > have prevented this.
> Looking over my scripts, yes, I overlooked the problem. The 'edison'
> platform shows the same issue that Heiko's platform does but I
> overlooked the size change. I'm modifying my script currently so it
> will show more details and this should jump out more rather than the
> size noise of "changes in a general area". Now interesting enough,
> sandbox didn't blow up here but does also enable the env flags options.
> > As far as I can see, Patrick's patch series has not been reviewed by
> > others, probably because general intetest in STM32 is not that big
> > at the moment. I can see no Acked-by:, Reviewed-by: nor Tested-by:
> > tags - nothing.
> > The whole patch series was then pulled from the u-boot-stm
> > repository.
> > However, there was not only STM related code in there. There were
> > changes to common code like the environment handling. common code
> > was changed without review and without testing.
> To be clear, it was tested, but sadly the environment flags code is not
> heavily used / enabled. More in a moment.
> > Are there ways to prevent this?
> > Yes, we can appeal to the custodians to be more careful, but I
> > assume they are already doing their best.
> > It might have even been better if this had been a sub-system with a
> > clear maintainer, but there is no such person for the environment
> > code.
> > How can we prevent this in the future?
> > Should we define "interested developers" for such areas that have no
> > custodian (the "Designated reviewer") entry in the MAINTAINERS file
> > could be used for this, for example)?
> This, along with some other environment related patches were things I
> was keeping an eye on to see if perhaps Joe would have had time to look
> at before it went in (as the env flag stuff came from him). I also try
I wasn't aware of it as I wasn't Cc'ed on this series. I generally
don't have time to troll the list in general, which is a bit of a
problem since I also missed the discussions on the UEFI env changes,
some of which are already in, and are not how I would have implemented
it. I only found out that it was in work from Grant Likely at his talk
in San Diego.
> and make use of the "Needs Review / ACK" flag in patchwork for things
> that stand out. Looking over the merge contents again, that particular
> one would not have.
> So, things that would help in the future:
> - An explicit environment maintainer
I would gladly volunteer for this role if Wolfgang would co-maintain
to keep me in line. He seems to have an uncanny ability to keep all
the cases in his head.
> - test.py tests for the environment flags, but only if they're also run
> on some platform(s) that also would have failed here. Perhaps we need
> to enable more functionality in something like qemu-x86 that is less
> of a special case build than sandbox is? In fact, since I know we
> have the QEMU targets in for "real" uses and not just testing,
> and while I worry about adding in more complex logic, we might want to
> rework the "build and run test.py in QEMU" parts of CI to first make
> use of scripts/kconfig/merge_config.sh to turn ON a whole bunch of
> testing related options.
> U-Boot mailing list
> U-Boot at lists.denx.de
More information about the U-Boot