Sometimes it's useful to be able to explicitly ensure certain headers remain self-contained, i.e. that they are compilable as standalone units, by including and/or forward declaring everything they depend on. Add special target header-test-y where individual Makefiles can add headers to be tested if CONFIG_HEADER_TEST is enabled. This will generate a dummy C file per header that gets built as part of extra-y. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Michal Marek <michal.lkml@markovi.net> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- Documentation/kbuild/makefiles.txt | 7 +++++++ init/Kconfig | 9 +++++++++ scripts/Makefile.build | 10 ++++++++++ scripts/Makefile.lib | 3 +++ 4 files changed, 29 insertions(+) diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt index 03c065855eaf..73df58e5ea0c 100644 --- a/Documentation/kbuild/makefiles.txt +++ b/Documentation/kbuild/makefiles.txt @@ -1036,6 +1036,13 @@ When kbuild executes, the following steps are followed (roughly): In this example, extra-y is used to list object files that shall be built, but shall not be linked as part of built-in.a. + header-test-y + + header-test-y specifies headers (*.h) in the current directory that + should be compile tested to ensure they are self-contained, + i.e. compilable as standalone units. If CONFIG_HEADER_TEST is enabled, + this autogenerates dummy sources to include the headers, and builds them + as part of extra-y. --- 6.7 Commands useful for building a boot image diff --git a/init/Kconfig b/init/Kconfig index 4592bf7997c0..d91b157201b1 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -95,6 +95,15 @@ config COMPILE_TEST here. If you are a user/distributor, say N here to exclude useless drivers to be distributed. +config HEADER_TEST + bool "Compile test headers that should be standalone compilable" + help + Compile test headers listed in header-test-y target to ensure they are + self-contained, i.e. compilable as standalone units. + + If you are a developer or tester and want to ensure the requested + headers are self-contained, say Y here. Otherwise, choose N. + config LOCALVERSION string "Local version - append to kernel release" help diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 76ca30cc4791..4d4bf698467a 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -291,6 +291,16 @@ quiet_cmd_cc_lst_c = MKLST $@ $(obj)/%.lst: $(src)/%.c FORCE $(call if_changed_dep,cc_lst_c) +# Dummy C sources for header test (header-test-y target) +# --------------------------------------------------------------------------- + +quiet_cmd_header_test = HDRTEST $@ + cmd_header_test = echo "\#include \"$(<F)\"" > $@ + +# FIXME: would be nice to be able to limit this implicit rule to header-test-y +$(obj)/%.header_test.c: $(src)/%.h FORCE + $(call if_changed,header_test) + # Compile assembler sources (.S) # --------------------------------------------------------------------------- diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 8a1f64f17740..c2839de06485 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -66,6 +66,9 @@ extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) endif +# Test self-contained headers +extra-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.o,$(header-test-y)) + # Add subdir path extra-y := $(addprefix $(obj)/,$(extra-y)) -- 2.20.1
Use the new header test facility. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Michal Marek <michal.lkml@markovi.net> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 1787e1299b1b..05d01a3830d0 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -204,3 +204,6 @@ i915-y += intel_lpe_audio.o obj-$(CONFIG_DRM_I915) += i915.o obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o + +header-test-y := \ + i915_drv.h -- 2.20.1
Demonstrate build failure on a header that is not self-contained. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Michal Marek <michal.lkml@markovi.net> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 05d01a3830d0..fcebf453c9ed 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -206,4 +206,5 @@ obj-$(CONFIG_DRM_I915) += i915.o obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o header-test-y := \ + i915_fixed.h \ i915_drv.h -- 2.20.1
Quoting Jani Nikula (2019-05-16 20:48:16)
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 8a1f64f17740..c2839de06485 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -66,6 +66,9 @@ extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
> extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
> endif
>
> +# Test self-contained headers
> +extra-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.o,$(header-test-y))
This didn't get pulled into clean-files.
clean-files-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.c,$(header-test-y))
?
Not enough. Nor is clean-files-y +=...
And it should also be put into the global gitignore I think.
-Chris
On Fri, May 17, 2019 at 4:48 AM Jani Nikula <jani.nikula@intel.com> wrote:
>
> Sometimes it's useful to be able to explicitly ensure certain headers
> remain self-contained, i.e. that they are compilable as standalone
> units, by including and/or forward declaring everything they depend on.
>
> Add special target header-test-y where individual Makefiles can add
> headers to be tested if CONFIG_HEADER_TEST is enabled. This will
> generate a dummy C file per header that gets built as part of extra-y.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> Documentation/kbuild/makefiles.txt | 7 +++++++
> init/Kconfig | 9 +++++++++
> scripts/Makefile.build | 10 ++++++++++
> scripts/Makefile.lib | 3 +++
> 4 files changed, 29 insertions(+)
>
> diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
> index 03c065855eaf..73df58e5ea0c 100644
> --- a/Documentation/kbuild/makefiles.txt
> +++ b/Documentation/kbuild/makefiles.txt
> @@ -1036,6 +1036,13 @@ When kbuild executes, the following steps are followed (roughly):
> In this example, extra-y is used to list object files that
> shall be built, but shall not be linked as part of built-in.a.
>
> + header-test-y
> +
> + header-test-y specifies headers (*.h) in the current directory that
> + should be compile tested to ensure they are self-contained,
> + i.e. compilable as standalone units. If CONFIG_HEADER_TEST is enabled,
> + this autogenerates dummy sources to include the headers, and builds them
> + as part of extra-y.
>
> --- 6.7 Commands useful for building a boot image
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 4592bf7997c0..d91b157201b1 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -95,6 +95,15 @@ config COMPILE_TEST
> here. If you are a user/distributor, say N here to exclude useless
> drivers to be distributed.
>
> +config HEADER_TEST
> + bool "Compile test headers that should be standalone compilable"
> + help
> + Compile test headers listed in header-test-y target to ensure they are
> + self-contained, i.e. compilable as standalone units.
> +
> + If you are a developer or tester and want to ensure the requested
> + headers are self-contained, say Y here. Otherwise, choose N.
> +
> config LOCALVERSION
> string "Local version - append to kernel release"
> help
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 76ca30cc4791..4d4bf698467a 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -291,6 +291,16 @@ quiet_cmd_cc_lst_c = MKLST $@
> $(obj)/%.lst: $(src)/%.c FORCE
> $(call if_changed_dep,cc_lst_c)
>
> +# Dummy C sources for header test (header-test-y target)
> +# ---------------------------------------------------------------------------
> +
> +quiet_cmd_header_test = HDRTEST $@
> + cmd_header_test = echo "\#include \"$(<F)\"" > $@
> +
> +# FIXME: would be nice to be able to limit this implicit rule to header-test-y
> +$(obj)/%.header_test.c: $(src)/%.h FORCE
> + $(call if_changed,header_test)
> +
> # Compile assembler sources (.S)
> # ---------------------------------------------------------------------------
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 8a1f64f17740..c2839de06485 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -66,6 +66,9 @@ extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
> extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
> endif
>
> +# Test self-contained headers
> +extra-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.o,$(header-test-y))
> +
> # Add subdir path
>
> extra-y := $(addprefix $(obj)/,$(extra-y))
> --
> 2.20.1
>
Thanks, probably we should do this.
At least, this check will be useful
for uapi headers since the kernel does not
test the self-containedness of
exported headers, (then turned out be problematic
later in user-space).
I will take a little time to considier
how far we can extend the idea about
"headers should be self-contained".
Thank you.
--
Best Regards
Masahiro Yamada
On Fri, May 17, 2019 at 5:35 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Jani Nikula (2019-05-16 20:48:16) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index 8a1f64f17740..c2839de06485 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -66,6 +66,9 @@ extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) > > extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) > > endif > > > > +# Test self-contained headers > > +extra-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.o,$(header-test-y)) > > This didn't get pulled into clean-files. > > clean-files-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.c,$(header-test-y)) > ? > > Not enough. Nor is clean-files-y +=... The correct syntax is 'clean-files +=' instead of 'clean-files-y +=' If Kbuild supports this in the core Makefiles, we can add "-o -name '*.header_test.c'" to the top Makefile. > And it should also be put into the global gitignore I think. Documentation/dontdiff too. > -Chris -- Best Regards Masahiro Yamada
On Sat, 18 May 2019, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> On Fri, May 17, 2019 at 4:48 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>> Sometimes it's useful to be able to explicitly ensure certain headers
>> remain self-contained, i.e. that they are compilable as standalone
>> units, by including and/or forward declaring everything they depend on.
>>
>> Add special target header-test-y where individual Makefiles can add
>> headers to be tested if CONFIG_HEADER_TEST is enabled. This will
>> generate a dummy C file per header that gets built as part of extra-y.
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Cc: Michal Marek <michal.lkml@markovi.net>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> Documentation/kbuild/makefiles.txt | 7 +++++++
>> init/Kconfig | 9 +++++++++
>> scripts/Makefile.build | 10 ++++++++++
>> scripts/Makefile.lib | 3 +++
>> 4 files changed, 29 insertions(+)
>>
>> diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
>> index 03c065855eaf..73df58e5ea0c 100644
>> --- a/Documentation/kbuild/makefiles.txt
>> +++ b/Documentation/kbuild/makefiles.txt
>> @@ -1036,6 +1036,13 @@ When kbuild executes, the following steps are followed (roughly):
>> In this example, extra-y is used to list object files that
>> shall be built, but shall not be linked as part of built-in.a.
>>
>> + header-test-y
>> +
>> + header-test-y specifies headers (*.h) in the current directory that
>> + should be compile tested to ensure they are self-contained,
>> + i.e. compilable as standalone units. If CONFIG_HEADER_TEST is enabled,
>> + this autogenerates dummy sources to include the headers, and builds them
>> + as part of extra-y.
>>
>> --- 6.7 Commands useful for building a boot image
>>
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 4592bf7997c0..d91b157201b1 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -95,6 +95,15 @@ config COMPILE_TEST
>> here. If you are a user/distributor, say N here to exclude useless
>> drivers to be distributed.
>>
>> +config HEADER_TEST
>> + bool "Compile test headers that should be standalone compilable"
>> + help
>> + Compile test headers listed in header-test-y target to ensure they are
>> + self-contained, i.e. compilable as standalone units.
>> +
>> + If you are a developer or tester and want to ensure the requested
>> + headers are self-contained, say Y here. Otherwise, choose N.
>> +
>> config LOCALVERSION
>> string "Local version - append to kernel release"
>> help
>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>> index 76ca30cc4791..4d4bf698467a 100644
>> --- a/scripts/Makefile.build
>> +++ b/scripts/Makefile.build
>> @@ -291,6 +291,16 @@ quiet_cmd_cc_lst_c = MKLST $@
>> $(obj)/%.lst: $(src)/%.c FORCE
>> $(call if_changed_dep,cc_lst_c)
>>
>> +# Dummy C sources for header test (header-test-y target)
>> +# ---------------------------------------------------------------------------
>> +
>> +quiet_cmd_header_test = HDRTEST $@
>> + cmd_header_test = echo "\#include \"$(<F)\"" > $@
>> +
>> +# FIXME: would be nice to be able to limit this implicit rule to header-test-y
>> +$(obj)/%.header_test.c: $(src)/%.h FORCE
>> + $(call if_changed,header_test)
>> +
>> # Compile assembler sources (.S)
>> # ---------------------------------------------------------------------------
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 8a1f64f17740..c2839de06485 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -66,6 +66,9 @@ extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y))
>> extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-))
>> endif
>>
>> +# Test self-contained headers
>> +extra-$(CONFIG_HEADER_TEST) += $(patsubst %.h,%.header_test.o,$(header-test-y))
>> +
>> # Add subdir path
>>
>> extra-y := $(addprefix $(obj)/,$(extra-y))
>> --
>> 2.20.1
>>
>
>
> Thanks, probably we should do this.
>
> At least, this check will be useful
> for uapi headers since the kernel does not
> test the self-containedness of
> exported headers, (then turned out be problematic
> later in user-space).
>
> I will take a little time to considier
> how far we can extend the idea about
> "headers should be self-contained".
Thanks! Please let me know if/when you need further action from me, I
won't post new versions until then.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
Hi Jani > Sometimes it's useful to be able to explicitly ensure certain headers > remain self-contained, i.e. that they are compilable as standalone > units, by including and/or forward declaring everything they depend on. > > Add special target header-test-y where individual Makefiles can add > headers to be tested if CONFIG_HEADER_TEST is enabled. This will > generate a dummy C file per header that gets built as part of extra-y. Very useful, thanks. I have cooked up something ad-hoc a couple of times but having it as a standard feature in the build system is much better. The we can let some of our infrastructure pick up an issues automatically. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Michal Marek <michal.lkml@markovi.net> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > Documentation/kbuild/makefiles.txt | 7 +++++++ > init/Kconfig | 9 +++++++++ > scripts/Makefile.build | 10 ++++++++++ > scripts/Makefile.lib | 3 +++ > 4 files changed, 29 insertions(+) > > diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt > index 03c065855eaf..73df58e5ea0c 100644 > --- a/Documentation/kbuild/makefiles.txt > +++ b/Documentation/kbuild/makefiles.txt > @@ -1036,6 +1036,13 @@ When kbuild executes, the following steps are followed (roughly): > In this example, extra-y is used to list object files that > shall be built, but shall not be linked as part of built-in.a. > > + header-test-y > + > + header-test-y specifies headers (*.h) in the current directory that > + should be compile tested to ensure they are self-contained, > + i.e. compilable as standalone units. If CONFIG_HEADER_TEST is enabled, > + this autogenerates dummy sources to include the headers, and builds them > + as part of extra-y. Do we want to restrict this to current directory only? Sometimes we could use this for headers in include/ but let it trigger for the relevant subsystem. So for example drivers/gpu/drm/Makefile will include the rules for all headers in include/drm/* The alternative would be Makefiles (of Kbuild files) scattered in the directories with headers and then some infrastructure to visit those. Follow patch extend the header-test feature to work with headers in include/ Example: # Header files from this directory header-test-y += drm_crtc_helper_internal.h header-test-y += drm_crtc_internal.h .. . # Header files from include/drm header-test-y += drm/amd_asic_type.h header-test-y += drm/ati_pcigart.h ... In the patch $* is used to get the "stem" from the pattern. This is the filname of the header file without extension. Sam diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 4d4bf698467a..ca132ab3a551 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -295,11 +295,10 @@ $(obj)/%.lst: $(src)/%.c FORCE # --------------------------------------------------------------------------- quiet_cmd_header_test = HDRTEST $@ - cmd_header_test = echo "\#include \"$(<F)\"" > $@ + cmd_header_test = echo "\#include <$(2).h>" > $@ -# FIXME: would be nice to be able to limit this implicit rule to header-test-y -$(obj)/%.header_test.c: $(src)/%.h FORCE - $(call if_changed,header_test) +$(obj)/%.header_test.c: + $(call cmd,header_test,$*) # Compile assembler sources (.S) # ---------------------------------------------------------------------------
Hi Sam, On Sat, May 25, 2019 at 2:40 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Jani > > > Sometimes it's useful to be able to explicitly ensure certain headers > > remain self-contained, i.e. that they are compilable as standalone > > units, by including and/or forward declaring everything they depend on. > > > > Add special target header-test-y where individual Makefiles can add > > headers to be tested if CONFIG_HEADER_TEST is enabled. This will > > generate a dummy C file per header that gets built as part of extra-y. > > Very useful, thanks. > I have cooked up something ad-hoc a couple of times but having it as a > standard feature in the build system is much better. > The we can let some of our infrastructure pick up an issues > automatically. > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Cc: Michal Marek <michal.lkml@markovi.net> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > Documentation/kbuild/makefiles.txt | 7 +++++++ > > init/Kconfig | 9 +++++++++ > > scripts/Makefile.build | 10 ++++++++++ > > scripts/Makefile.lib | 3 +++ > > 4 files changed, 29 insertions(+) > > > > diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt > > index 03c065855eaf..73df58e5ea0c 100644 > > --- a/Documentation/kbuild/makefiles.txt > > +++ b/Documentation/kbuild/makefiles.txt > > @@ -1036,6 +1036,13 @@ When kbuild executes, the following steps are followed (roughly): > > In this example, extra-y is used to list object files that > > shall be built, but shall not be linked as part of built-in.a. > > > > + header-test-y > > + > > + header-test-y specifies headers (*.h) in the current directory that > > + should be compile tested to ensure they are self-contained, > > + i.e. compilable as standalone units. If CONFIG_HEADER_TEST is enabled, > > + this autogenerates dummy sources to include the headers, and builds them > > + as part of extra-y. > Do we want to restrict this to current directory only? > Sometimes we could use this for headers in include/ but let it > trigger for the relevant subsystem. > So for example drivers/gpu/drm/Makefile will include the rules > for all headers in include/drm/* > > The alternative would be Makefiles (of Kbuild files) > scattered in the directories with headers and then some > infrastructure to visit those. > > Follow patch extend the header-test feature to work with > headers in include/ Following the obj-y pattern, I want to make header-test-y relative to $(obj). > Example: > # Header files from this directory > header-test-y += drm_crtc_helper_internal.h > header-test-y += drm_crtc_internal.h These are described in drivers/gpu/drm/Makefile. > .. > . > # Header files from include/drm > header-test-y += drm/amd_asic_type.h > header-test-y += drm/ati_pcigart.h These are described in $(srctree)/include/Makefile. > ... > > > In the patch $* is used to get the "stem" from the pattern. > This is the filname of the header file without extension. > > > Sam > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 4d4bf698467a..ca132ab3a551 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -295,11 +295,10 @@ $(obj)/%.lst: $(src)/%.c FORCE > # --------------------------------------------------------------------------- > > quiet_cmd_header_test = HDRTEST $@ > - cmd_header_test = echo "\#include \"$(<F)\"" > $@ > + cmd_header_test = echo "\#include <$(2).h>" > $@ > > -# FIXME: would be nice to be able to limit this implicit rule to header-test-y > -$(obj)/%.header_test.c: $(src)/%.h FORCE > - $(call if_changed,header_test) > +$(obj)/%.header_test.c: > + $(call cmd,header_test,$*) > > # Compile assembler sources (.S) > # --------------------------------------------------------------------------- > Agree, this is much better, and it is what scripts/Makefile.asm-generic does. But, you do not need to pass '$*' via the argument. I prefer this: quiet_cmd_header_test = HDRTEST $@ cmd_header_test = echo "\#include \"$*.h\"" > $@ $(obj)/%.header_test.c: $(call cmd,header_test) -- Best Regards Masahiro Yamada
Hi Jani,
On Mon, May 20, 2019 at 6:16 PM Jani Nikula <jani.nikula@intel.com> wrote:
>
> >
> > I will take a little time to considier
> > how far we can extend the idea about
> > "headers should be self-contained".
>
> Thanks! Please let me know if/when you need further action from me, I
> won't post new versions until then.
Could you send v2 with the following changes ?
[1] Could you rename *.header_test.c to *.hdrtest.c ?
(I just thought .header_test.c was a bit too long.)
[2] %.hdrtest.c should not depend on the header
This will avoid unnecessary regeneration of *.hdrtest.c
quiet_cmd_header_test = HDRTEST $@
cmd_header_test = echo "\#include \"$*.h\"" > $@
$(obj)/%.hdrtest.c:
$(call cmd,header_test)
[3] Please add '*.hdrtest.c' to .gitignore, Documentation/dontdiff
[4] Please add '*.hdrtest.c' to 'make clean' (around line 1640 of top Makefile)
--
Best Regards
Masahiro Yamada
Hi Masahiro/Jani. > > Following the obj-y pattern, > I want to make header-test-y relative to $(obj). I also considered this and agree this is better. Otherwise we end up with a spaghetti of dependencies across the tree. What I made just fit the purpose I had that day, which is no excuse for bad design. > I prefer this: > > quiet_cmd_header_test = HDRTEST $@ > cmd_header_test = echo "\#include \"$*.h\"" > $@ > > $(obj)/%.header_test.c: > $(call cmd,header_test) Even better - good. We call it HDRTEST - so why not just go for that name: hdrtest-y += headerfile.h ?? The current proposal with "header-test-y" hurts the eye a little with two '-', and all other variables uses only one '-' as is today. (generic-y, obj-y etc). This is bikeshedding but is was itcing me a little. Sam
Hi Sam, Jani,
On Tue, Jun 4, 2019 at 2:33 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Masahiro/Jani.
>
> >
> > Following the obj-y pattern,
> > I want to make header-test-y relative to $(obj).
>
> I also considered this and agree this is better.
>
> Otherwise we end up with a spaghetti of dependencies across the tree.
>
> What I made just fit the purpose I had that day,
> which is no excuse for bad design.
>
> > I prefer this:
> >
> > quiet_cmd_header_test = HDRTEST $@
> > cmd_header_test = echo "\#include \"$*.h\"" > $@
> >
> > $(obj)/%.header_test.c:
> > $(call cmd,header_test)
>
> Even better - good.
>
> We call it HDRTEST - so why not just go for that name:
>
> hdrtest-y += headerfile.h
>
> ??
>
> The current proposal with "header-test-y" hurts the eye a little with
> two '-', and all other variables uses only one '-' as is today.
> (generic-y, obj-y etc).
>
> This is bikeshedding but is was itcing me a little.
I do not have a strong opinion.
I leave it to Jani. Either is fine with me.
There are variables that contain two '-'.
'no-clean-files', 'subdir-ccflags-y', etc.
--
Best Regards
Masahiro Yamada