On Mon, 2023-08-07 at 22:52 +0300, Peter Pentchev wrote: > On Mon, Aug 07, 2023 at 06:43:36PM +0000, Benjamin Drung wrote: > > Hi, > > > > while working a whole week on fixing failing C/C++ header compilations > > for armhf time_t [1], I noticed a common pattern: The library -dev > > packages was missing one or more dependencies on another -dev package. > > Over 200 -dev packages are affected. > > > > I propose to add an autodep8 test for C/C++ header files that tries to > > compile the header file. This will catch issues like missing > > dependencies and other issues. > > > > Here is a draft for the autopkgtest check script that takes a binary > > -dev package as parameter: > > https://github.com/bdrung/bdrung-scripts/blob/compile-header-check/compile-header-check > > > > What do you think? Should I proceed developing and packaging this check > > script and submit support for it in autodep8? > > > > [1] https://salsa.debian.org/vorlon/armhf-time_t/-/merge_requests/103 > > Whlie it does seem an interesting tool at first glance (and thanks for > doing the work and presenting a proof of concept), I can think of > several cases when compilation of the concatenated header files would > fail:
Yes, I expect that several packages would fail to compile and the check script would need several knobs to tweak (skip certain headers, add or exclude certain include directories, define a specific C/C++ standand version, use only C and not C++, etc). A set of common approaches to solve such failures should be added to the documentation of the check script (probably including skipping the whole check). > 1) The library has a "main" header file that must be included before > any of the others, and it does not come first in lexicographical > order. It may define typedefs and structure definitions that > the other header files can use, it may define preprocessor symbols > that reflect the availability of this or that system header file or > type; there are also other ways in which another header file > distributed by the -dev package may depend on the main one. In this case the non-"main" header could just include the "main" header as first step. Alternatively, an option to specify headers that should be included first could be added to the check script. > 2) As a variation of the above, the -dev package may distribute > the full set of header files included in the library, and some of > them may only be included if certain preprocessor symbols are > defined. Since their use is guarded by conditionals, they are > allowed to unconditionally include system-specific header files > that are only available on e.g. Windows or NetBSD or Darwin, etc. > Unconditionally compiling the contents of those files would fail. The -dev packages could drop shipping unneeded headers (e.g. Windows or BSD specific ones), some headers could be excluded by a parameter to the check script, and/or the check script could allow defining some names (pass -D name=definition to gcc). > 3) The -dev package may distribute the full set of header files > included in the library, and some of them may be mutually exclusive > and impossible to combine. For example, a header file may include > either this or that other header file based on preprocessor > defintions (OS type, features enabled, etc), and the included > files may both define a function with the same name, but different > contents. That one is trickier to reflect in the check script. > 4) Some of the library's header files may not be supposed to be > included in all cases; the library's -dev package may suggest > (but not depend on or recommend) another -dev package as > an optional dependency, and a library's header file may > unconditionally include some header file from the latter package. Either exclude the header from the check script or not ship this header file in the -dev package. > All of these cases are things I've seen in complex libraries with > many header files; maybe not all of them can be found in Debian > right now. When you look at check-armhf-time_t [2] you will find all those mentioned cases and some more. > So... yeah. Thanks for your work, I know you mean well and you are > trying to make the life of Debian developers better, but this > particular approach will likely fail on a non-trivial set of > Debian -dev packages. I expect this approach to work for most packages, fail for several packages due to bugs in the package (missing dependencies and a bunch of other issues like missing double include protection) and some will fail with the reasons above. According to [3] there are 5360 C/C++ -dev package and only 1900 -dev packages (35%) did not compile out of the box [4]. The majority of those 1900 -dev packages do not compile due to bugs in the package (judging from the workaround that we apply in [2]). IMO having this check for 90% of the packages to catch bugs and disabling it for 10% is better than not having it. If there were such check script in place before the armhf time_t analysis started, we would have had significantly fewer packages to analyze. [2] https://salsa.debian.org/vorlon/armhf-time_t/-/blob/main/check-armhf-time_t [3] https://wiki.debian.org/ReleaseGoals/64bit-time [4] https://lists.debian.org/debian-devel/2023/05/msg00168.html -- Benjamin Drung Debian & Ubuntu Developer