Separate headers from source files

[Ericson2314]

Motivation

The short answer for why we need to do this is so we can consistently do #include "nix/...". Without this change, there are ways to still make that work, but they are hacky, and they have downsides such as making it harder to make sure headers from the wrong Nix library (e..g.
libnixexpr headers in libnixutil) aren't being used.

The C API alraedy used nix_api_*, so its headers are not put in subdirectories accordingly.

See comments on the script; this is supposed to avoid breaking muscle memory without complicating the build system (which proved harder than I thought too) or not doing the header hygiene change at all.

Context

Progress on #7876

We resisted doing this for a while because it would be annoying to not have the header source file pairs close by / easy to change file path/name from one to the other. But I am ameliorating that with symlinks in the final commit.

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[roberth]

Glad that the symlinks aren't committed.

my earlier misread

I don't think the symlinks are a good idea, because it opens up more opportunities for various tools and editors, as well as users to get confused.

Furthermore I believe in practice this isn't as useful as it seems.
For example, consider the following

1. user navigates to a certain header
2. editor follows the symlink
3. editor buffer has include/... as its file name
4. user opens accompanying tree view / file open UI / default basedir, expecting to navigate to a sibling .cc file
5. user is launched into the includes dir, not the one with the .cc files

Also I've seen instances of symlink contents appearing as "distinct" files while they are not, e.g. allowing both the canonical and non-canonical paths to have an editor buffer simultaneously, leading to conflicts if the user isn't paying attention to that. Obviously that's an editor bug, but this is the kind of problem we're opening ourselves up to by inviting in more symlinks.

And that's assuming the bug is in the tooling. Humans also have to make sense of it.
I'd much prefer a simple solution to the include namespace problem, without symlinks.

[roberth]

       > ../nix_api_util.cc:10:10: fatal error: config-util.h: No such file or directory
       >    10 | #include "config-util.h"

Looks like an interaction with #12773. config-util.h is a private header here.

nix build .#nix-util-tests.tests.run

[edolstra]

Note: we really shouldn't have backport labels on a PR that changes 666 files. Code restructurings like this are inappropriate for the maintenance branches.

[edolstra]

What's the impact of this on IDEs, i.e. can editors still follow a line like

#include "nix/machines.hh"

to the corresponding file in the Git repo?

Also, shouldn't the subdirectory of the component be included, e.g.

#include "nix/libstore/machines.hh"

?

[Ericson2314]

What's the impact of this on IDEs, i.e. can editors still follow a line like

#include "nix/machines.hh"

I don't know, but see the symlink script that is included. With that, your workflow should be exactly the same as before,

[Ericson2314]

Also, shouldn't the subdirectory of the component be included, e.g.

#include "nix/libstore/machines.hh"

I was starting more minimally, but if you prefer that, I'm happy to jump all the way to that.

Just one thing, I would do

#include "nix/store/machines.hh"

I don't think the lib prefix is adding anything of value.

[Ericson2314]

Actually, because the per-project dirs is a less mechanical change, I would rather do it in a follow-up please. Happy to make it right-away, however.

[Mic92]

@Ericson2314 btw. I refactored the python script a bit but you seemed to have pushed over this change again: b6cb4be

[Ericson2314]

OK this no longer includes the symlink script --- the plan is that @Mic92 will make that in a separate PR (he is working on it right now).

[Ericson2314]

@mergify queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 5a8dedc

[Ericson2314]

As to

#include "nix/store/machines.hh"

I think we should do that, but that should also be in a separate PR, since it is a bit less mechanical than this, as I said above.