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

k3s: remove extra buildInputs #84384

Merged
merged 2 commits into from Apr 7, 2020
Merged

k3s: remove extra buildInputs #84384

merged 2 commits into from Apr 7, 2020

Conversation

euank
Copy link
Member

@euank euank commented Apr 5, 2020

In the intial version of this package, I missed a few inputs that
weren't needed including a debug hook.

Motivation for this change

Review on #84325 after it was merged pointed out I had an extra debug buildInput still.

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.

@euank
Copy link
Member Author

euank commented Apr 5, 2020

I can try to fix the aarch64 build on this pr or separately. k3s is supposed to compile and work on arm, so it should be pretty easy to fix.

@euank
Copy link
Member Author

euank commented Apr 5, 2020

cc @flokli since these changes are in response to your review on #84325

flokli
flokli requested changes Apr 6, 2020
View reviewed changes
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

Also, please do the other fixes in separate commits (can be part of the same PR)

pkgs/applications/networking/cluster/k3s/default.nix Outdated Show resolved Hide resolved
@euank
k3s: remove extra buildInputs
83f827c 
In the intial version of this package, I missed a few inputs that
weren't needed including a debug hook.
@euank
k3s: update maintainers, per pr comment
617fa43
@euank
Copy link
Member Author

euank commented Apr 7, 2020

Well, I'm unable to get nix-build --arg crossSystem '{ config = "aarch64-linux"; }' to actually work on my system so far, and I haven't figured out the failure yet.

It fails on building git well before it gets to k3s. I'll give getting it working on aarch64 a try as a separate PR when I have more time to figure out this issue.

@euank euank requested a review from flokli April 7, 2020 05:15
@flokli flokli merged commit 7183053 into NixOS:master Apr 7, 2020
@euank euank deleted the pkg/k3s-update branch April 7, 2020 16:46
@misuzu
Copy link
Contributor

misuzu commented Apr 9, 2020

This broke k3s build: https://hydra.nixos.org/build/116153430

@euank
Copy link
Member Author

euank commented Apr 9, 2020

Thanks for pointing that out @misuzu, appreciated! I'll look into it soon

@flokli
Copy link
Contributor

flokli commented Apr 9, 2020

@misuzu that hydra build was still from a version with breakpointHook installed.

I was able to compile k3s just fine. A (still scheduled) build on hydra can be found here: https://hydra.nixos.org/build/116387472

@euank euank mentioned this pull request Apr 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli Awaiting requested review from flokli

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@euank @misuzu @flokli @grahamc