On Wed, 02 Apr 2025, Masahiro Yamada <masahi...@kernel.org> wrote: > On Wed, Apr 2, 2025 at 1:12 AM Linus Torvalds > <torva...@linux-foundation.org> wrote: >> >> On Tue, 1 Apr 2025 at 05:21, Jani Nikula <jani.nik...@linux.intel.com> wrote: >> > >> > The header checks have existed for uapi headers before, including the, >> > uh, turds, but apparently adding them in drm broke the camel's back. >> >> The uapi header test things never caused any problems for me [*], >> because they don't actually pollute the source tree. >> >> Why? Because they all end up in that generated 'usr/include/' directory. >> >> So when I look at the source files, filename completion is entirely >> unaffected, and it all works fine. >> >> Look, I can complete something like >> >> include/uapi/asm-generic/poll.h >> >> perfectly fine, because there is *not* some generated turd that affects it >> all. >> >> Because for the uapi files those hdrtest files end up being in >> >> ./usr/include/asm-generic/poll.hdrtest >> >> and I never have any reason to really look at that subdirectory at >> all, since it's all generated. >> >> Or put another way - if I _were_ to look at it, it would be exactly >> because I want to see some generated file, in which case the 'hdrtest' >> turd would be part of it. >> >> (Although I cannot recall that ever having actually happened, to be >> honest - but looking at various header files is common, and I hit the >> drm case immediately) >> >> Would you mind taking more of that uapi approach than creating that >> hidden directory specific to the drm tree? Maybe this could *all* be >> generalized? >> >> Linus >> >> [*] I say "never caused any problems for me", but maybe it did way in >> the past and it was fixed and I just don't recall. I have definitely >> complained about pathname completion issues to people before. > > I know you dislike this, and I NACKed this before (but too late): > https://lore.kernel.org/dri-devel/CAK7LNATHXwEkjJHP7b-ZmhzLfyyuOdsyimna-=r-sjk+dxi...@mail.gmail.com/ > > Compile-testing UAPI headers is useful > (and I believe this is the only case where it is useful) > because userspace is independent of any CONFIG option, > and we have no control over the include order in > userspace programs. > > In contrast, kernel-space headers are conditionally parsed, > depending on CONFIG options. > > You dislike this feature for the reason of TAB-completion, > but I am negative about this feature from another perspective. > > We cannot avoid a false-positive: > https://lore.kernel.org/all/20190718130835.ga28...@lst.de/ > > It is not <drm/*.h> but <linux/*.h> > However, it is annoying to make every header self-contained > "just because we are checking this".
As I explained earlier in the thread, it's not *just* about the headers being self-contained. It's *also* about checking header guards and kernel-doc, maybe other things in the future. If it's possible to do opt-in build checks for this, and catch all of these pre-merge, why shouldn't we? We can catch a whole class of build issues and bypass the subsequent fixes with this, and make life easier for us. > Actually, it is not a real problem when the relevant > CONFIG option is disabled. > I did not interrupt him because he was doing this > checkunder drivers/gpu/drm/i915/. > () > But, I am opposed to expanding it to more-global include/drm/, > or any other subsystem. > > In my opinion, the right fix is > "git revert 62ae45687e43" > > > If we continue this, maybe leave a caution > in capital letters? Or maybe let the subsystem and driver maintainers decide? I don't think there's a *single* header under include/drm where them being self-contained depends on a config option. I'm sure there are plenty in include/linux, but I wouldn't say they *all* do. Maybe help and support us in generalizing this in scripts/Makefile.build to avoid the boilerplate, and make Linus happy, instead of trying to block us from having nice things? Please? BR, Jani. > > > diff --git a/include/Kbuild b/include/Kbuild > index 5e76a599e2dd..4dff23ae51a4 100644 > --- a/include/Kbuild > +++ b/include/Kbuild > @@ -1 +1,3 @@ > +# The drm subsystem is strict about the self-containedness of header files. > +# OTHER SUBSYSTEMS SHOULD NOT FOLLOW THIS PRACTICE. > obj-$(CONFIG_DRM_HEADER_TEST) += drm/ > > > > -- > Best Regards > > Masahiro Yamada -- Jani Nikula, Intel
