Re: [PATCH v2 1/1] Makefile: rework u-boot-initial-env target

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]

On Tue, Nov 01, 2022 at 08:55:09PM +0100, Pali Rohár wrote:
> On Tuesday 01 November 2022 15:48:34 Tom Rini wrote:
> > On Tue, Nov 01, 2022 at 06:35:04PM +0100, Pali Rohár wrote:
> > > On Monday 31 October 2022 11:51:45 Max Krummenacher wrote:
> > > > On Fri, Oct 28, 2022 at 6:40 PM Pali Rohár <pali@kernel.org> wrote:
> > > > > On Friday 28 October 2022 18:18:49 Max Krummenacher wrote:
> > > > > >  quiet_cmd_genenv = GENENV  $@
> > > > > > -cmd_genenv = $(OBJCOPY) --dump-section .rodata.default_environment=$@ env/common.o; \
> > > > > > -     sed --in-place -e 's/\x00/\x0A/g' $@; sed --in-place -e '/^\s*$$/d' $@; \
> > > > > > -     sort --field-separator== -k1,1 --stable $@ -o $@
> > > > > > +cmd_genenv = \
> > > > > > +     scripts/printinitialenv | \
> > > > > > +     sed -e 's/\x00/\x0A/g' -e '/^\s*$$/d' | \
> > > > >
> > > > > I think that you do not need this sed anymore as you print newline in
> > > > > host tool.
> > > > 
> > > > Missed that one, will change in a V3.
> > > > 
> > > > >
> > > > > > +     sort --field-separator== -k1,1 --stable -o $@
> > > > > >
> > > > > >  u-boot-initial-env: u-boot.bin
> > > > >
> > > > > It is needed to update dependencies for u-boot-initial-env target. Now
> > > > > it does not depend on u-boot.bin but rather on printinitialenv tool.
> > > > 
> > > > I'm unsure if that is the best way forward. The initial solution would
> > > > also not need to depend on u-boot.bin but rather on env/common.o.
> > > 
> > > Yea, it was wrong too.
> > > 
> > > > I guess that the intention was that the U-Boot binary and the
> > > > u-boot-initial-env file should not be out of sync.
> > > 
> > > This is not possible to achieve with Makefile build system and is
> > > probably also against the logic of make to achieve the best parallelism
> > > during build of the project.
> > > 
> > > In _my_ opinion, Makefile targets should have correct dependencies, on
> > > which they depend. This will prevent circular dependences and also
> > > faster building as make tool can run in parallel as much jobs as
> > > possible -- and do not have to wait until main u-boot.bin is built and
> > > then call other jobs.
> > > 
> > > Tom, or do you think something different?
> > 
> > Well, this tool has the same environment requirements as tools/env does,
> > so we should be able to use the same dependency info?
> 
> Yes, build tool is _able_ to use it. (I think we all agree on this
> ability of make)
> 
> But the question is if it _should_ be used. Because Max wrote some
> arguments why not to use this env dependency info.

Er, I'm lost I guess, sorry. Yes, it should not depend on env/common.o
nor u-boot.bin. I guess looking again, we can't borrow just the envtools
rules as we also would need to update the section that adds
$(defaultenv_h) to envtools would also need to be done for this tool.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]