drm/i915: add Makefile magic for testing headers are self-contained

Submitted by Jani Nikula on April 3, 2019, 10:06 a.m.

Details

Message ID 20190403100636.28966-1-jani.nikula@intel.com
State New
Headers show
Series "drm/i915: add Makefile magic for testing headers are self-contained" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Jani Nikula April 3, 2019, 10:06 a.m.
The below commits added dummy files to test that certain headers are
self-contained, i.e. compilable as standalone units:

39e2f501c1b4 ("drm/i915: Split struct intel_context definition to its own header")
3a891a626794 ("drm/i915: Move intel_engine_mask_t around for use by i915_request_types.h")
8b74594aa455 ("drm/i915: Split out i915_priolist_types into its own header")

The idea is fine, but the implementation is a bit tedious and
inflexible, and does not really scale well.

Implement the same in make using autogenerated dummy sources to include
the headers.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 | 16 +++++---------
 drivers/gpu/drm/i915/Makefile.header-test     | 21 +++++++++++++++++++
 .../i915/test_i915_active_types_standalone.c  |  7 -------
 .../test_i915_gem_context_types_standalone.c  |  7 -------
 .../test_i915_priolist_types_standalone.c     |  7 -------
 .../test_i915_scheduler_types_standalone.c    |  7 -------
 .../test_i915_timeline_types_standalone.c     |  7 -------
 .../test_intel_context_types_standalone.c     |  7 -------
 .../i915/test_intel_engine_types_standalone.c |  7 -------
 .../test_intel_workarounds_types_standalone.c |  7 -------
 10 files changed, 26 insertions(+), 67 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/Makefile.header-test
 delete mode 100644 drivers/gpu/drm/i915/test_i915_active_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_intel_context_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_intel_engine_types_standalone.c
 delete mode 100644 drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 30bf33..dba77b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -57,17 +57,6 @@  i915-$(CONFIG_COMPAT)   += i915_ioc32.o
 i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
 i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
 
-# Test the headers are compilable as standalone units
-i915-$(CONFIG_DRM_I915_WERROR) += \
-	test_i915_active_types_standalone.o \
-	test_i915_gem_context_types_standalone.o \
-	test_i915_priolist_types_standalone.o \
-	test_i915_scheduler_types_standalone.o \
-	test_i915_timeline_types_standalone.o \
-	test_intel_context_types_standalone.o \
-	test_intel_engine_types_standalone.o \
-	test_intel_workarounds_types_standalone.o
-
 # GEM code
 i915-y += \
 	  i915_active.o \
@@ -218,3 +207,8 @@  i915-y += intel_lpe_audio.o
 
 obj-$(CONFIG_DRM_I915) += i915.o
 obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
+
+# Extra header tests
+ifeq ($(CONFIG_DRM_I915_WERROR),y)
+include $(src)/Makefile.header-test
+endif
diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test
new file mode 100644
index 000000..0cae6e3
--- /dev/null
+++ b/drivers/gpu/drm/i915/Makefile.header-test
@@ -0,0 +1,21 @@ 
+# SPDX-License-Identifier: MIT
+# Copyright © 2019 Intel Corporation
+
+# Test the headers are compilable as standalone units
+
+quiet_cmd_header_test = HDRTEST	$@
+      cmd_header_test = echo "\#include \"$(<F)\"" > $@
+
+header_test_%.c: %.h
+	$(call cmd,header_test)
+
+# To test <base>.h add header_test_<base>.o here
+extra-y += \
+	header_test_i915_active_types.o \
+	header_test_i915_gem_context_types.o \
+	header_test_i915_priolist_types.o \
+	header_test_i915_scheduler_types.o \
+	header_test_i915_timeline_types.o \
+	header_test_intel_context_types.o \
+	header_test_intel_engine_types.o \
+	header_test_intel_workarounds_types.o
diff --git a/drivers/gpu/drm/i915/test_i915_active_types_standalone.c b/drivers/gpu/drm/i915/test_i915_active_types_standalone.c
deleted file mode 100644
index 144ebd..000000
--- a/drivers/gpu/drm/i915/test_i915_active_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "i915_active_types.h"
diff --git a/drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c b/drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c
deleted file mode 100644
index 4e4da486..000000
--- a/drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "i915_gem_context_types.h"
diff --git a/drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c b/drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c
deleted file mode 100644
index f465eb..000000
--- a/drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "i915_priolist_types.h"
diff --git a/drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c b/drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c
deleted file mode 100644
index 8afa2c..000000
--- a/drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "i915_scheduler_types.h"
diff --git a/drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c b/drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c
deleted file mode 100644
index f58e14..000000
--- a/drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "i915_timeline_types.h"
diff --git a/drivers/gpu/drm/i915/test_intel_context_types_standalone.c b/drivers/gpu/drm/i915/test_intel_context_types_standalone.c
deleted file mode 100644
index b39e3c4..000000
--- a/drivers/gpu/drm/i915/test_intel_context_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "intel_context_types.h"
diff --git a/drivers/gpu/drm/i915/test_intel_engine_types_standalone.c b/drivers/gpu/drm/i915/test_intel_engine_types_standalone.c
deleted file mode 100644
index d05e4cd..000000
--- a/drivers/gpu/drm/i915/test_intel_engine_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "intel_engine_types.h"
diff --git a/drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c b/drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c
deleted file mode 100644
index 4f658b..000000
--- a/drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c
+++ /dev/null
@@ -1,7 +0,0 @@ 
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2019 Intel Corporation
- */
-
-#include "intel_workarounds_types.h"

Comments

Quoting Jani Nikula (2019-04-03 11:06:36)
> The below commits added dummy files to test that certain headers are
> self-contained, i.e. compilable as standalone units:
> 
> 39e2f501c1b4 ("drm/i915: Split struct intel_context definition to its own header")
> 3a891a626794 ("drm/i915: Move intel_engine_mask_t around for use by i915_request_types.h")
> 8b74594aa455 ("drm/i915: Split out i915_priolist_types into its own header")
> 
> The idea is fine, but the implementation is a bit tedious and
> inflexible, and does not really scale well.
> 
> Implement the same in make using autogenerated dummy sources to include
> the headers.

I knew it would work :-p

> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 | 16 +++++---------
>  drivers/gpu/drm/i915/Makefile.header-test     | 21 +++++++++++++++++++
>  .../i915/test_i915_active_types_standalone.c  |  7 -------
>  .../test_i915_gem_context_types_standalone.c  |  7 -------
>  .../test_i915_priolist_types_standalone.c     |  7 -------
>  .../test_i915_scheduler_types_standalone.c    |  7 -------
>  .../test_i915_timeline_types_standalone.c     |  7 -------
>  .../test_intel_context_types_standalone.c     |  7 -------
>  .../i915/test_intel_engine_types_standalone.c |  7 -------
>  .../test_intel_workarounds_types_standalone.c |  7 -------
>  10 files changed, 26 insertions(+), 67 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/Makefile.header-test
>  delete mode 100644 drivers/gpu/drm/i915/test_i915_active_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_intel_context_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_intel_engine_types_standalone.c
>  delete mode 100644 drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 30bf33..dba77b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -57,17 +57,6 @@ i915-$(CONFIG_COMPAT)   += i915_ioc32.o
>  i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
>  i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
>  
> -# Test the headers are compilable as standalone units
> -i915-$(CONFIG_DRM_I915_WERROR) += \
> -       test_i915_active_types_standalone.o \
> -       test_i915_gem_context_types_standalone.o \
> -       test_i915_priolist_types_standalone.o \
> -       test_i915_scheduler_types_standalone.o \
> -       test_i915_timeline_types_standalone.o \
> -       test_intel_context_types_standalone.o \
> -       test_intel_engine_types_standalone.o \
> -       test_intel_workarounds_types_standalone.o
> -
>  # GEM code
>  i915-y += \
>           i915_active.o \
> @@ -218,3 +207,8 @@ i915-y += intel_lpe_audio.o
>  
>  obj-$(CONFIG_DRM_I915) += i915.o
>  obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
> +
> +# Extra header tests
> +ifeq ($(CONFIG_DRM_I915_WERROR),y)
> +include $(src)/Makefile.header-test
> +endif

Can we get the headers tested first?

Fixing the standalone tests is a lot easier than traversing the
dependency chain of includes in a regular compile file.

> diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test
> new file mode 100644
> index 000000..0cae6e3
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/Makefile.header-test
> @@ -0,0 +1,21 @@
> +# SPDX-License-Identifier: MIT
> +# Copyright © 2019 Intel Corporation
> +
> +# Test the headers are compilable as standalone units
> +
> +quiet_cmd_header_test = HDRTEST        $@
> +      cmd_header_test = echo "\#include \"$(<F)\"" > $@
> +
> +header_test_%.c: %.h
> +       $(call cmd,header_test)
> +
> +# To test <base>.h add header_test_<base>.o here
> +extra-y += \
> +       header_test_i915_active_types.o \
> +       header_test_i915_gem_context_types.o \
> +       header_test_i915_priolist_types.o \
> +       header_test_i915_scheduler_types.o \
> +       header_test_i915_timeline_types.o \
> +       header_test_intel_context_types.o \
> +       header_test_intel_engine_types.o \
> +       header_test_intel_workarounds_types.o

Should there be a way to add the autogenerated files to make clean?
-Chris
Quoting Chris Wilson (2019-04-03 11:10:31)
> Quoting Jani Nikula (2019-04-03 11:06:36)
> > The below commits added dummy files to test that certain headers are
> > self-contained, i.e. compilable as standalone units:
> > 
> > 39e2f501c1b4 ("drm/i915: Split struct intel_context definition to its own header")
> > 3a891a626794 ("drm/i915: Move intel_engine_mask_t around for use by i915_request_types.h")
> > 8b74594aa455 ("drm/i915: Split out i915_priolist_types into its own header")
> > 
> > The idea is fine, but the implementation is a bit tedious and
> > inflexible, and does not really scale well.
> > 
> > Implement the same in make using autogenerated dummy sources to include
> > the headers.
> 
> I knew it would work :-p
> 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile                 | 16 +++++---------
> >  drivers/gpu/drm/i915/Makefile.header-test     | 21 +++++++++++++++++++
> >  .../i915/test_i915_active_types_standalone.c  |  7 -------
> >  .../test_i915_gem_context_types_standalone.c  |  7 -------
> >  .../test_i915_priolist_types_standalone.c     |  7 -------
> >  .../test_i915_scheduler_types_standalone.c    |  7 -------
> >  .../test_i915_timeline_types_standalone.c     |  7 -------
> >  .../test_intel_context_types_standalone.c     |  7 -------
> >  .../i915/test_intel_engine_types_standalone.c |  7 -------
> >  .../test_intel_workarounds_types_standalone.c |  7 -------
> >  10 files changed, 26 insertions(+), 67 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/Makefile.header-test
> >  delete mode 100644 drivers/gpu/drm/i915/test_i915_active_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_intel_context_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_intel_engine_types_standalone.c
> >  delete mode 100644 drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 30bf33..dba77b 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -57,17 +57,6 @@ i915-$(CONFIG_COMPAT)   += i915_ioc32.o
> >  i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
> >  i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
> >  
> > -# Test the headers are compilable as standalone units
> > -i915-$(CONFIG_DRM_I915_WERROR) += \
> > -       test_i915_active_types_standalone.o \
> > -       test_i915_gem_context_types_standalone.o \
> > -       test_i915_priolist_types_standalone.o \
> > -       test_i915_scheduler_types_standalone.o \
> > -       test_i915_timeline_types_standalone.o \
> > -       test_intel_context_types_standalone.o \
> > -       test_intel_engine_types_standalone.o \
> > -       test_intel_workarounds_types_standalone.o
> > -
> >  # GEM code
> >  i915-y += \
> >           i915_active.o \
> > @@ -218,3 +207,8 @@ i915-y += intel_lpe_audio.o
> >  
> >  obj-$(CONFIG_DRM_I915) += i915.o
> >  obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
> > +
> > +# Extra header tests
> > +ifeq ($(CONFIG_DRM_I915_WERROR),y)
> > +include $(src)/Makefile.header-test
> > +endif
> 
> Can we get the headers tested first?
> 
> Fixing the standalone tests is a lot easier than traversing the
> dependency chain of includes in a regular compile file.
> 
> > diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test
> > new file mode 100644
> > index 000000..0cae6e3
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/Makefile.header-test
> > @@ -0,0 +1,21 @@
> > +# SPDX-License-Identifier: MIT
> > +# Copyright © 2019 Intel Corporation
> > +
> > +# Test the headers are compilable as standalone units
> > +
> > +quiet_cmd_header_test = HDRTEST        $@
> > +      cmd_header_test = echo "\#include \"$(<F)\"" > $@
> > +
> > +header_test_%.c: %.h
> > +       $(call cmd,header_test)
> > +
> > +# To test <base>.h add header_test_<base>.o here
> > +extra-y += \
> > +       header_test_i915_active_types.o \
> > +       header_test_i915_gem_context_types.o \
> > +       header_test_i915_priolist_types.o \
> > +       header_test_i915_scheduler_types.o \
> > +       header_test_i915_timeline_types.o \
> > +       header_test_intel_context_types.o \
> > +       header_test_intel_engine_types.o \
> > +       header_test_intel_workarounds_types.o
> 
> Should there be a way to add the autogenerated files to make clean?

clean-files += header_test_*.c

No idea if that works.
-Chris
Quoting Jani Nikula (2019-04-03 11:06:36)
> The below commits added dummy files to test that certain headers are
> self-contained, i.e. compilable as standalone units:
> 
> 39e2f501c1b4 ("drm/i915: Split struct intel_context definition to its own header")
> 3a891a626794 ("drm/i915: Move intel_engine_mask_t around for use by i915_request_types.h")
> 8b74594aa455 ("drm/i915: Split out i915_priolist_types into its own header")
> 
> The idea is fine, but the implementation is a bit tedious and
> inflexible, and does not really scale well.
> 
> Implement the same in make using autogenerated dummy sources to include
> the headers.

Also wants
i915/.gitignore: header_test_*.c
-Chris
Quoting Jani Nikula (2019-04-03 11:06:36)
> The below commits added dummy files to test that certain headers are
> self-contained, i.e. compilable as standalone units:
> 
> 39e2f501c1b4 ("drm/i915: Split struct intel_context definition to its own header")
> 3a891a626794 ("drm/i915: Move intel_engine_mask_t around for use by i915_request_types.h")
> 8b74594aa455 ("drm/i915: Split out i915_priolist_types into its own header")
> 
> The idea is fine, but the implementation is a bit tedious and
> inflexible, and does not really scale well.
> 
> Implement the same in make using autogenerated dummy sources to include
> the headers.

The next question is to how to handle subdirs?
-Chris