Re: [PATCH 00/32] lib: Finish rearrangement of includes and license headers

On Tue, 2026-05-12 at 12:20 +0200, Tobias Schaffner wrote:
> Hi Florian,
> 
> On 5/8/26 16:23, Florian Bezdeka wrote:
> > On Fri, 2026-05-08 at 16:15 +0200, Jan Kiszka wrote:
> > > On 08.05.26 08:22, Florian Bezdeka wrote:
> > > > Hi all,
> > > > 
> > > > This is the missing cleanup of demo/, lib/, testuite/ and utils/ that
> > > > was partially handled already in the past as preparation commits for
> > > > bugfixes or build optimizations. I finally found some time to finish
> > > > that work for the missing folders.
> > > > 
> > > > There might be even more potential for cleanups inside the include/
> > > > directory - and as a result once again inside the already cleaned
> > > > directories. I skipped that for now, as I consider include/ as an API.
> > > > Changes might force users to adapt/recompile.
> > > > 
> > > > Tested against Dovetail 7.0 already.
> > > > 
> > > 
> > > How did you retrieve the header needs and coverage again, manually,
> > > script-based or AI? And what do you suggest to maintain consistency in
> > > the future?
> > > 
> > 
> > Sorry, forgot to mention that.
> > 
> > Formatting and sorting of includes: clang-format (with the option
> > enabled in the last patch)
> > 
> > Indexer: The indexer part of JetBrains Clion. Used to find and eliminate
> > unused includes. Followed by a manual compile to verify the result after
> > each compile unit.
> > 
> > No AI.
> > 
> > I haven't checked yet if there is any command line tool for identifying
> > unused includes yet. If so, we might be able to integrate something into
> > CI - on the long run.
> > 
> > For the formatting part clang-format is able to report differences - or
> > formatting violations. At the moment the code base is not ready for that
> > yet. Too much differences. Step by step.
> 
> this must have been a lot of work and I really like that you started 
> this. I had a closer look at the patches and decided to give summary 
> instead of single comments on the patches as the comments apply to all 
> patches.
> 
> 
> A general questions first and maybe this has already been discussed 
> behind the scenes: Are we sure that we want to we prefer minimization of 
> includes over self containment?

.h files: self-containment, include only what you need 
.c files: include what you need, from private to public headers, no
          duplications.

> Simple example from the first patch: lib/alchemy/init.c removing stdio 
> but using fprinf() which is now included deep behind a pretty long 
> include chain which is a bit hard to follow and changes for the 
> different build configurations.

lib/alchemy/init.c needed a revisit after cleaning up the private
headers. Should be fixed now.

> I personally think that only removing the unused includes and may be 
> easier to maintain.
> 

I'm cleaning up unused + unnecessary includes here. Everything included
by private headers already is unnecessary.

> 
> I tried some more exotic build configurations and e.g. Mercury fails 
> when --disable-pshared or --enable-debug is used.

Hm. Can not reproduce here - nor in CI - nor with my usual compiler/os
variations. Which ./configure line in combination with which compiler?

I might have noticed that upfront. Testing with:
  - gcc 14 on Debian 13
  - gcc 12 on Debian 13
  - gcc 12 on Debian 12
  - gcc 16 on Fedora (needs additional fixes on top, see wip/flo/gcc-16)

> 
> 
> Can you somehow make sure, that we do not have any ifdefs silently 
> taking the undef path after the changes?
> Example: clocktest.c included xeno_config.h which is now removed. Is 
> this still included indirectly and taking the if path when checking if 
> HAVE_PTHREAD_SPIN_LOCK is set or is it always taking the else path now?

Whenever I removed xeno_config.h I tested that it is really already
included upfront. In this case:

In file included from ../../include/boilerplate/libc.h:30,
                 from include/cobalt/pthread.h:24,
                 from ../../include/boilerplate/ancillaries.h:23,
                 from clocktest.c:21:

But: There is no tooling behind. I should probably go and check that
again. I might have missed one.

> 
> 
> Some small nitpicks:
> 
> s/Harmonzie/Harmonize/ in the commit messages.
> 
> 
> There are some commented includes left in the patches:
> git grep "//#include"
> demo/posix/cyclictest/error.h://#include <stdio.h>
> lib/alchemy/event.h://#include <alchemy/event.h>
> lib/trank/native.c://#include <copperplate/heapobj.h>
> lib/trank/native.c://#include <copperplate/threadobj.h>

Those have been addressed meanwhile. The current state is published as
wip/flo/refactor-includes.