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

darwin.linux-builder: Disable evaluation #268574

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

roberth
Copy link
Member

@roberth roberth commented Nov 19, 2023

Description of changes

A remote builder does not need to evaluate anything, so let's trim it down to (eventually) save some space, and make the purpose of the builder clear.

Users should evaluate on the host instead, if they aren't already.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

A remote builder does not need to evaluate anything, so let's trim
it down to (eventually) save some space, and make the purpose of
the builder clear.

Users should evaluate on the host instead.
These won't cause anything to appear in toplevel.
@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Nov 19, 2023
@Atemu Atemu merged commit 6fc4c1c into NixOS:master Nov 28, 2023
27 checks passed
Copy link
Contributor

Successfully created backport PR for release-23.11:

cpick pushed a commit to cpick/darwin-builder that referenced this pull request Aug 21, 2024
Now that the underlying darwin.linux-builder doesn't
[support evaluation](NixOS/nixpkgs#268574)
it doesn't make sense to do as much local configuration/testing that
requires root.
Comment on lines +112 to +114
environment.extraSetup = ''
rm --force $out/bin/{nix-instantiate,nix-build,nix-shell,nix-prefetch*,nix}
'';
Copy link
Member

Choose a reason for hiding this comment

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

My linux-builder has gotten into a bad state and removing these commands makes it very hard to debug the situation

@roberth I wonder if it's better to revert this section

Copy link
Member Author

Choose a reason for hiding this comment

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

Which commands do you need to run?

Would an opt-in option be sufficient for your purposes? Something like:

Suggested change
environment.extraSetup = ''
rm --force $out/bin/{nix-instantiate,nix-build,nix-shell,nix-prefetch*,nix}
'';
environment.extraSetup = mkIf (!cfg.nix.evaluator.enable) ''
rm --force $out/bin/{nix-instantiate,nix-build,nix-shell,nix-prefetch*,nix}
'';

Currently it's meant to simulate a store-only nix, so there's little benefit to this code as is, except manage expectations when we can reduce the linux builder's closure by only incorporating a store-only build of nix.

Copy link
Member

Choose a reason for hiding this comment

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

I want to run something like nix build --rebuild so ideally the nix command

If I'm understanding correctly, the default would be to include the nix command, which would be my preference as otherwise if your linux-builder gets in a bad state, it might be quite annoying/hard to bootstrap another one rather than be able to potentially fix it

Copy link
Member Author

Choose a reason for hiding this comment

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

it might be quite annoying/hard to bootstrap another

I thought it was basically fully automated, except for clearing it.

Anyway, not being able to debug it properly is annoying, so I'm ok to revert this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even the clearing is automated, too (we set min-free and max-free), but it might still be worth restoring the nix command-line tools for debugging purposes

Copy link
Member

@Enzime Enzime Oct 8, 2024

Choose a reason for hiding this comment

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

I was referring to bootstrapping my own Linux builder locally, but I realize now that if you just comment out any custom configuration (if you're using nix-darwin) and use a cached Linux builder from Nixpkgs that should work fine

I've made a PR here: #347205

Enzime added a commit to Enzime/nixpkgs that referenced this pull request Oct 8, 2024
wrbbz pushed a commit to wrbbz/nixpkgs that referenced this pull request Oct 9, 2024
Enzime added a commit to Enzime/nixpkgs that referenced this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants