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

Reply via email to