Skip to content

Separate headers from source files (backport #12764) #12820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 31, 2025

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Mar 31, 2025

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.


depends on #12831
depends on #12830
depends on #12833


Add 👍 to pull requests you find important.

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


This is an automatic backport of pull request #12764 done by [Mergify](https://mergify.com).

@mergify mergify bot added automatic backport This PR is a backport produced by automation (does not trigger backporting) conflicts merge-queue labels Mar 31, 2025

This comment was marked as resolved.

@github-actions github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority store Issues and pull requests concerning the Nix store repl The Read Eval Print Loop, "nix repl" command and debugger fetching Networking with the outside (non-Nix) world, input locking c api Nix as a C library with a stable interface labels Mar 31, 2025
@Ericson2314 Ericson2314 force-pushed the mergify/bp/2.28-maintenance/pr-12764 branch 2 times, most recently from ba54d9c to 4026128 Compare March 31, 2025 19:09
@Ericson2314
Copy link
Member

I fixed the conflicts, so this can be queued (but not "Enable auto-merged"). When the merge queue branch is made (from a rebase) the extra commits in here will disappear.

@Ericson2314
Copy link
Member

@mergify queue

Copy link
Contributor Author

mergify bot commented Mar 31, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b36f6ea

@Ericson2314 Ericson2314 changed the base branch from 2.28-maintenance to 2.8-maintenance March 31, 2025 20:24
@Ericson2314 Ericson2314 force-pushed the mergify/bp/2.28-maintenance/pr-12764 branch from 4026128 to 6479108 Compare March 31, 2025 20:27
@Ericson2314 Ericson2314 changed the base branch from 2.8-maintenance to 2.28-maintenance March 31, 2025 20:29
Copy link

dpulls bot commented Mar 31, 2025

🎉 All dependencies have been resolved !

- Since it's now private, give it a rename. Note that I want to switch the
  word order on the public ones too.

- Since it is only needed by two files, just include there rather than
  the nasty blanket-forced thing.

(cherry picked from commit 326548b)
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.

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 next commit.

(cherry picked from commit f3e1c47)
@Ericson2314 Ericson2314 force-pushed the mergify/bp/2.28-maintenance/pr-12764 branch from 6479108 to 15658b2 Compare March 31, 2025 22:04
@mergify mergify bot merged commit b36f6ea into 2.28-maintenance Mar 31, 2025
27 checks passed
@mergify mergify bot deleted the mergify/bp/2.28-maintenance/pr-12764 branch March 31, 2025 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automatic backport This PR is a backport produced by automation (does not trigger backporting) c api Nix as a C library with a stable interface conflicts documentation fetching Networking with the outside (non-Nix) world, input locking merge-queue new-cli Relating to the "nix" command repl The Read Eval Print Loop, "nix repl" command and debugger store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant