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.

Sign up for GitHub

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

Jump to bottom

Add overlay output, allow overriding NixOS module package, respect Nixpkgs config #26

Merged
merged 6 commits into from
Mar 20, 2024
Merged

Add overlay output, allow overriding NixOS module package, respect Nixpkgs config #26

merged 6 commits into from
Mar 20, 2024

Conversation

evanrelf
Copy link
Contributor

This adds an overlay output to the flake, and allows overriding the package used by the NixOS module. The motivation for this was (quoting the first commit):

Allows re-packaging in your own flake using your Nixpkgs pin's
configuration, notably allowUnfree = true.

Using the packages.x86_64-linux.kolide-launcher derivation directly
doesn't respect your Nixpkgs config because it relies on vanilla Nixpkgs
from the flake input.

Commits are atomic; I recommend reviewing them individually.

Thanks for supporting Nix/NixOS!

@evanrelf
Add overlay
33ba369 
Allows re-packaging in your own flake using your Nixpkgs pin's
configuration, notably `allowUnfree = true`.

Using the `packages.x86_64-linux.kolide-launcher` derivation directly
doesn't respect your Nixpkgs config because it relies on vanilla Nixpkgs
from the flake input.
@evanrelf
Allow configuring package in NixOS module
7a902a6
@evanrelf
Don't require using overlay with NixOS module
e5f99e5
@CLAassistant
Copy link

CLAassistant commented Mar 15, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@evanrelf
Copy link
Contributor Author

evanrelf commented Mar 15, 2024 •
edited
Loading

Before this change, my coworkers had to run their nixos-rebuild commands with the --impure flag to allow reading from environment variables, and had to specify NIXPKGS_ALLOW_UNFREE=1 to allow the kolide-launcher package to be used.

After this change, their Nixpkgs configs (with allowUnfree = true set) are respected; the package is built using their stdenv.mkDerivation functions, and no --impure or NIXPKGS_ALLOW_UNFREE is needed.

@RebeccaMahany
Copy link
Contributor

@evanrelf thank you for contributing this! Took a quick look through and it looks good to me -- I will approve/merge on Tuesday.

@RebeccaMahany
Copy link
Contributor

It looks like this fails nix flake check with overlay does not take an argument named 'final' -- I'm happy to merge and fix that up after, or happy to reapprove if you'd like to push an update.

@evanrelf
Copy link
Contributor Author

Hmm, I don't understand why it's failing with that error... seems to contradict my code, since I am using the name final?

I don't have access to an x86_64-linux machine at the moment (believe it or not), so if you know how to fix this I welcome you to! Otherwise, I'm happy to provide a fix eventually. Sorry about that.

@evanrelf
Remove system from flake overlays
651b03e 
Overlays are system-agnostic. The error in CI is bad. Running `nix flake
check` locally (works far enough on macOS to check this) with Nix
2.20.5, I get this more helpful error:

```
error:
       … while checking flake output 'overlays'
         at /nix/store/43a7kmhwjk6hvzvfnbybrwbdkxxggp8s-source/flake.nix:16:5:
           15|
           16|     overlays.x86_64-linux.default = final: prev: {
             |     ^
           17|       kolide-launcher = final.callPackage ./kolide-launcher.nix { };

       … while checking the overlay 'overlays.x86_64-linux'
         at /nix/store/43a7kmhwjk6hvzvfnbybrwbdkxxggp8s-source/flake.nix:16:5:
           15|
           16|     overlays.x86_64-linux.default = final: prev: {
             |     ^
           17|       kolide-launcher = final.callPackage ./kolide-launcher.nix { };

       error: overlay is not a function, but a set instead
```
@evanrelf
Copy link
Contributor Author

Actually I think I might've found the issue.

@evanrelf
Copy link
Contributor Author

Oof silly typo. Sorry for the churn. If this doesn't work, I'll wait to come back with a fix I know works 😅

@RebeccaMahany
Copy link
Contributor

I think I fixed the setting where it's requiring a manual approval from me to run the checks in CI, hopefully that makes it a little easier 🤞

RebeccaMahany
RebeccaMahany approved these changes Mar 20, 2024
View reviewed changes
Copy link
Contributor

@RebeccaMahany RebeccaMahany left a comment

Choose a reason for hiding this comment

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

I re-tested on an x86 linux VM and it still works -- I think the final CI failure yesterday was just CI flakiness. I re-ran and it passed. Thanks again for this contribution!

@RebeccaMahany RebeccaMahany merged commit 90b7761 into kolide:main Mar 20, 2024
2 checks passed
@RebeccaMahany RebeccaMahany mentioned this pull request May 28, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RebeccaMahany RebeccaMahany RebeccaMahany approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@evanrelf @CLAassistant @RebeccaMahany