[Buildroot] [PATCH 1/1] package/pkg-generic.mk: fix rule order for re{install, build, configure}

[James Hilliard]

On Sun, Feb 12, 2023 at 3:57 AM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
>
> James, All,
>
> On 2023-02-12 03:22 -0700, James Hilliard spake thusly:
> > On Sun, Feb 12, 2023 at 3:11 AM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
> > >
> > > James, All,
> > >
> > > On 2023-02-12 03:02 -0700, James Hilliard spake thusly:
> > > > On Sun, Feb 12, 2023 at 2:42 AM Yann E. MORIN <yann.morin.1998 at free.fr> wrote:
> > > > > On 2022-10-17 21:46 -0600, James Hilliard spake thusly:
> > > > > > To ensure the evaluation order is always correct use double colon
> > > > > > rules to make the evaluation order explicit as per make docs:
> > > > > >
> > > > > > The double-colon rules for a target are executed in the order they
> > > > > > appear in the makefile.
> > > [--SNIP--]
> > > > > So, I'd like that we get another way to fix that, if possible...
> > > > I have unfortunately not found any other method to fix the execution
> > > > ordering...
> > >
> > > What about something along the lines of (untestd!):
> > >
> > >     $(1)-rebuild: $(1): $(1)-clean-for-rebuild
> > >     $(1)-rebuild: $(1)
> >
> > That would appear to be invalid syntax.
> >
> > >
> > > We already use such a construct in package/pkg-kconfig.mk, but only
> > > once, so this is not as if it were widespread either...
> >
> > I don't see anything like that there,
>
>     240:  $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $(1)-%: $$($(2)_DIR)/.kconfig_editor_%
>
> So, this looks like it is named "target pattern", and it works with a
> pattern (eh!).
>
>     $ cat Makefile
>      1  all: a
>      2
>      3  a:
>      4          @echo a
>      5
>      6  a-clean:
>      7          @sleep 1
>      8          @echo a-clean
>      9
>     10  a-rebuild: %-rebuild: %-clean
>     11  a-rebuild: a
>
> However, that does not work with parallel builds:
>
>     $ make
>     a
>
>     $ make a
>     a
>
>     $ make a-rebuild
>     a-clean
>     a
>
>     $ make -j a-rebuild
>     a
>     a-clean
>
> However, you stated in a previous reply:
>
>     So unlike the legal-info issue using .NOTPARALLEL would not work
>     here as these make targets do need to work with parallel builds.
>
> I don't think that's true. Indeed, foo-rebuild is usually done after a
> ful build, when iterating over the foo package, iether to test a new
> source tree (with ocerride-ssrcdir) or to test fixes in foo.mk. In
> either case, we do not need a top-level parallel build, because only one
> package, foo, will be built.
>
> So, even in this case, we should be able to use .NOTPARALLEL, no?

Breaking parallel build for foo-rebuild would be rather unexpected behavior
as foo-rebuild does work for full builds as well and one may default to using
foo-rebuild instead of a foo target for various reasons(to ensure that a package
is always rsynced from an overridden source directory for example).

I see no good reason to disable parallel build on it when simply ordering the
dependency execution order fixes the issue.

>
> It also means that the kconfig rule I mentionned above, does not work
> with top-level parallel builds, but that's OK: it's only used to spawn
> kconfig configurators (menuconfig et al.), so it should not be used with
> top-level parallel, and thus should require .NOTPARALLEL as well.

Might be better to use double colon rules there as well, they are more
explicit than using .NOTPARALLEL IMO as .NOTPARALLEL doesn't
specify an execution order, also it can potentially introduce accidental
performance regressions.

>
> Meh, TLPB and shuffling are really uncovering issues everywhere... :-(

Better to make them very obvious so they can be fixed IMO.

Once the known issues are fixed I can run shuffle mode on my autobuilders
to uncover remaining issues.

>
> Regards,
> Yann E. MORIN.
>
> > just this which isn't doing anything
> > unusual with multiple prerequisites that require a specific execution
> > order like the re{install,build,configure} rules in pkg-generic.mk:
> >
> > # Force olddefconfig again on -reconfigure
> > $(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
> >
> > $(1)-clean-kconfig-for-reconfigure:
> >     rm -f $$($(2)_DIR)/$$($(2)_KCONFIG_STAMP_DOTCONFIG)
> >
> > >
> > > Still, I think it is more easy to read...
> > >
> > > Regards,
> > > Yann E. MORIN.
> > >
> > > --
> > > .-----------------.--------------------.------------------.--------------------.
> > > |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> > > | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> > > | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> > > '------------------------------^-------^------------------^--------------------'
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'