Re: [GBUILD] Can we get rid of recursively invoking custom targets ?

On 16/03/12 15:47, Matúš Kukan wrote:
> Hi,
>
> I'd like to propose removing gbuild_simple.mk over time.
> Surely there will be more custom targets as gbuild is growing and I
> can't see why we would need gbuild_simple.
> Instead we can write rules to Package_foo.mk makefiles or if that
> would be problem (because they will be ugly)
> We can start using CutomTarget_foo.mk which will be supposed to be ugly.
>
> The reasons are:
> 1, Sometimes we need to parse custom target's Makefile each time as in
> packimages case.
> I had to use:
> $(eval $(call gb_CustomTarget_add_outdir_dependencies,packimages/pack,\
>     $(gb_Helper_PHONY) \
> ))
> to achieve that.
>
> 2, It is enough then to write dependencies only once.
>
> 3, No need to maintain gbuild_simple.
>
> 4, No recursive make running.

i think nobody really likes the way custom targets are currently
implemented, and iirc Bjoern even denies being responsible for the design :)

having to write dependencies twice so the recursive make invocation
works is really annoying and error-prone.

there are some potential problems with having the custom stuff in the
same make process, but i think these are solvable:

1. var namespace collisions:
   because make doesn't have namespaces for global variables, it's
   possible that variables defined in different modules' custom
   makefiles have the same name and thus collide.
   this can be avoided by agreeing to a naming convention, such as
   prefixing all variables by the name of the module where they are
   defined, and reviewing all custom makefiles so that this convention
   is met.

2. pattern rule collisions:
   the pattern rules defined in a lot of custom makefiles also can lead
   to collisions, both with other custom makefiles, and with the
   pattern rules defined in solenv/gbuild, which is complicated due
   to different pattern rule matching in make 3.81 and 3.82.
   so the pattern rules in custom makefiles should all have targets of
   the form $(WORKDIR)/CustomTarget/$(MODULE)/... and this prefix
   should be different across modules and so the problem of getting
   these to work in 3.81 and 3.82 should be module local and hence
   hopefully manageable.

[for non-custom makefiles both of these problems are avoided by
designing gbuild so that variables and pattern rules are only defined in
solenv/gbuild, not in user makefiles.]

> I am sending two patches for packimages.
> Maybe we should use the second one and keep Package_foo.mk pretty ?
> One small disadvantage is that we need to depend on directory.
>
> It allows as to use compatible mode and keep old way of processing
> custom targets until they are replaced.
> There are 31 custom targets now, so that should not be a big problem.
> I can do it over time or maybe we could even write an easy hack for that.
> The important thing is to write new custom targets in
> CustomTarget_foo.mk makefile.
>
> What do you think ? Am I missing something ?

so overall i think your proposal is a good direction to go.

your second patch (with the CustomTarget_foo.mk) looks better to me, as
having the ugly custom stuff easily discoverable is a good idea;
but please rename the CDPI variable to packimages_CDPI or expand it
(also, CUSTOM_images, CUSTOM_PREFERRED_FALLBACK_[12])