Skip to content
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

Various packaging improvements #12033

Merged
merged 3 commits into from
Dec 11, 2024
Merged

Various packaging improvements #12033

merged 3 commits into from
Dec 11, 2024

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Dec 9, 2024

Motivation

Cherry picked from: #11566

Context


Add 👍 to pull requests you find important.

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

@Mic92 Mic92 requested a review from edolstra as a code owner December 9, 2024 20:16
we are not testing any nixos modules, so we don't need to generate
documentation. This will give us a bit of speed up.
@roberth
Copy link
Member

roberth commented Dec 9, 2024

Fails on the formatting check (pre-commit). Is that consistent with the failure you got in the upgrade?
If so, we can accept the small change in formatting.

@roberth
Copy link
Member

roberth commented Dec 9, 2024

(i.e. pre-commit run -a; git add --patch; git commit)

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 10, 2024
@Mic92
Copy link
Member Author

Mic92 commented Dec 10, 2024

Forgot to cherry-pick also the header update.

@@ -12,6 +12,8 @@
hooks = {
clang-format = {
enable = true;
# https://github.com/cachix/git-hooks.nix/pull/532
package = pkgs.llvmPackages_latest.clang-tools;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the impact of this on the devshell closure size? (It's already pretty big, depending on gcc, clang and a lot of other stuff.)

Copy link
Member Author

@Mic92 Mic92 Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the current clang-tools version. But we need both macOS and Linux to use the same version. Open for other suggestions, but it will increase the closure size on some platform for sure if it's not the same as default included one on one platform or the other. I sticked to _latest because it will never go out-of-date compared to pinning a version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having this will block our nixpkgs upgrade.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually on Linux it will likely not change anything, I assume. Because there shouldn't be clang included in the gcc build anyway. It will increase it for macOS.

@edolstra edolstra merged commit da2c254 into NixOS:master Dec 11, 2024
11 checks passed
@Mic92 Mic92 deleted the various-picks branch December 11, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants