Re: [PATCH v2 0/7] include/hw/pci include/hw/cxl: Clean up includes

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Dec 23, 2022 at 06:27:07AM +0100, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Thu, Dec 22, 2022 at 11:48:25AM +0100, Markus Armbruster wrote:
>> >> Bernhard Beschow <shentey@gmail.com> writes:
>> >> 
>> >> > Am 22. Dezember 2022 10:03:23 UTC schrieb Markus Armbruster 
>> >> > <armbru@redhat.com>:
>> >> >>Back in 2016, we discussed[1] rules for headers, and these were
>> >> >>generally liked:
>> >> >>
>> >> >>1. Have a carefully curated header that's included everywhere first.  We
>> >> >>   got that already thanks to Peter: osdep.h.
>> >> >>
>> >> >>2. Headers should normally include everything they need beyond osdep.h.
>> >> >>   If exceptions are needed for some reason, they must be documented in
>> >> >>   the header.  If all that's needed from a header is typedefs, put
>> >> >>   those into qemu/typedefs.h instead of including the header.
>> >> >>
>> >> >>3. Cyclic inclusion is forbidden.
>> >> >
>> >> > Sounds like these -- useful and sane -- rules belong in QEMU's coding 
>> >> > style. What about putting them there for easy reference?
>> >> 
>> >> Makes sense.  I'll see what I can do.  Thanks!

Commit f07ceffdf50.

>> > It would be even better if there was e.g. a make target
>> > pulling in each header and making sure it's self consistent and
>> > no circularity. We could run it e.g. in CI.
>> 
>> Yes, that would be nice, but the problem I've been unable to crack is
>> deciding whether a header is supposed to compile target-independently or
>> not.  In my manual testing, I use trial and error: if it fails to
>> compile target-independently, compile for all targets.  This is s-l-o-w.

To spice things up, we also have headers that provide additional
contents in target-dependent context.  These need to be tested in both
contexts.

> Yes and it's annoying for developers too.
> Do we want to come up with a scheme for target-dependent files?
> Name them target-X or put in a target/ directory?

I'd be in favour.  Sadly, I'm not able to push such an effort myself.

> We could also write checkpatch rules to disallow
> including target specific headers in target independent files then.

Fortunately, that's pretty unlikely to compile :)

>> The other problem, of course, is coding it up in Meson.  I haven't even
>> tried.