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

nixos/corerad: init #77252

Merged
merged 1 commit into from Jan 16, 2020
Merged

nixos/corerad: init #77252

merged 1 commit into from Jan 16, 2020

Conversation

@mdlayher
Copy link
Member

@mdlayher mdlayher commented Jan 7, 2020

Motivation for this change

Now that CoreRAD is packaged (#76825), I'd like to add a proper NixOS module for it. At the moment the project is in alpha and I haven't stabilized the config, so this module expects a raw CoreRAD TOML configuration. In the future I would be interested in making the config more idiomatic, but it seems that various other projects do this with varying levels of support as well.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Here's some tidied up test output showing that things seem to work: https://gist.github.com/mdlayher/183eaf50f9167e1061074c43508f9132.

Notify maintainers

I'm the sole maintainer for this, so I'll tag folks who helped out on my previous packaging PR: #76825.

cc @danderson @jonringer @filalex77

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 7, 2020

@GrahamcOfBorg build nixosTests.corerad

@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch from a869233 to 27e0467 Jan 7, 2020
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 7, 2020

@GrahamcOfBorg build nixosTests.corerad

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 7, 2020

@GrahamcOfBorg test corerad

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 7, 2020

Looks like aarch64 is happy but x86_64 is still blocking on something. Any ideas?

@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch from 27e0467 to f82eef6 Jan 7, 2020
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 7, 2020

Found a small mistake. Will retrigger tests when the initial checks pass.

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 7, 2020

@GrahamcOfBorg test corerad

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 7, 2020

@jonringer looks like things are all set now! By the way, what's the difference between "test corerad" and "build nixosTests.corerad"? I saw both on a different PR I was using as a reference.

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 7, 2020

@GrahamcOfBorg test corerad

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 7, 2020

the build command will try to build it for all available runners (x86, arm, and darwin). the test command I think is just a convenience method, which is smart about only targeting linux.

https://github.com/NixOS/ofborg#test-added-2017-11-24

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 7, 2020

cc @Infinisil for module experience

Copy link
Contributor

@aanderse aanderse left a comment

I've added a few minor comments I hope you find useful.

nixos/modules/services/networking/corerad.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/corerad.nix Outdated Show resolved Hide resolved
@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch from f82eef6 to 342e38f Jan 8, 2020
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 8, 2020

@GrahamcOfBorg test corerad

@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch from 342e38f to 280abe3 Jan 12, 2020
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 12, 2020

@GrahamcOfBorg test corerad

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 14, 2020

Any additional thoughts? Would love to get this merged.

Copy link
Contributor

@jonringer jonringer left a comment

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 14, 2020

it LGTM, I'll merge if no one has an issue in a day

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 14, 2020

Thanks!

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 14, 2020

just ping this thread if I forget :) wish there was a remindme! type bot

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 15, 2020

@jonringer looks like it's been about a day with no word from @Infinisil. Merge when you're ready, please and thanks!

@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch from 280abe3 to ce93677 Jan 16, 2020
nixos/modules/services/networking/corerad.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/corerad.nix Outdated Show resolved Hide resolved
@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch 2 times, most recently from 071d571 to ce4bb00 Jan 16, 2020
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 16, 2020

@GrahamcOfBorg test corerad

@mdlayher mdlayher force-pushed the mdlayher:mdl-module-corerad branch from ce4bb00 to 1393e8b Jan 16, 2020
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 16, 2020

Made a minor mistake while syncing configs between a local git repo and the nixpkgs repo. I'll let all the CI run and then ping when it's done.

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 16, 2020

@GrahamcOfBorg test corerad

1 similar comment
@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 16, 2020

@GrahamcOfBorg test corerad

@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 16, 2020

Seems like the x86_64-linux runner is stuck?

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 16, 2020

if it passes locally, I'll say this is good

@jonringer
Copy link
Contributor

@jonringer jonringer commented Jan 16, 2020

passed locally, assuming this is issue with CI

@jonringer jonringer merged commit 5089214 into NixOS:master Jan 16, 2020
13 of 14 checks passed
13 of 14 checks passed
tests.corerad on x86_64-linux
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
tests.corerad on aarch64-linux Success
Details
@mdlayher
Copy link
Member Author

@mdlayher mdlayher commented Jan 16, 2020

Thank you all for your time and patience with the reviews! This has been very educational.

@mdlayher mdlayher deleted the mdlayher:mdl-module-corerad branch Jan 16, 2020
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Feb 5, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/january-2020-in-nixos/5771/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.