Add NixOS support #1287

jaroeichler · 2024-08-28T03:56:56Z

Description

Use nix-ld to run Bazel.

Type of change

Please delete options that aren't relevant.

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

Used a pretty minimal system configuration and Bazel test ... passes 51 of 52 tests, integration_tests/running_actions_manager_test fails.

Checklist

Updated documentation if needed
Tests added/amended
bazel test //... passes locally
PR is contained in a single commit, using git amend see some docs

This change is

CLAassistant · 2024-08-28T03:57:02Z

All committers have signed the CLA.

jaroeichler · 2024-08-28T04:11:42Z

services.envfs.enable is only needed for some scripts that need /bin/bash. Is there a better way?

@SpamDoodler can you please check if this works on your system? I've made a few changes to what we've tried a few days ago. In particular, it works without NIX_CC_USE_RESPONSE_FILE=0 on a pretty minimal system configuration.

aaronmondal

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, and 3 discussions need to be resolved

flake.nix line 80 at r1 (raw file):

          "/run/current-system/sw/bin"
          "${binutils.bintools}/bin"
          "${coreutils}/bin"

nit: For kicks, consider using uutils-coreutils-noprefix as pure rust implementation of the coreutils 💃

flake.nix line 83 at r1 (raw file):

          "${customClang}/bin"
          "${git}/bin"
          "${python3Full}/bin"

Interesting. This looks like it exposes several nonhermeticities in the devShell. Most of this should probably be in the main devShell as well.

Regarding the python dependency, why do we need the Full variant? What's the closure size difference here?

flake.nix line 490 at r1 (raw file):

                ln -fs ${nixosBazelrc} nixos.bazelrc
                export CC=customClang
              fi

This should be a flake module similar to the config.pre-commit.installationScript and config.local-remote-execution.installationScript. This way users can use it as a generic "NixOS Bazel compatibility" module and are not bound to our choice of clang/binutils/python and may import it in arbitrary Bazel+Nix projects.

See:

nativelink/flake.nix

Line 44 in 1b38a64

./local-remote-execution/flake-module.nix
(unused in our flake, but makes it importable)

nativelink/flake.nix

Line 479 in 1b38a64

flakeModule = ./local-remote-execution/flake-module.nix;
https://github.com/TraceMachina/nativelink/blob/main/local-remote-execution/flake-module.nix
https://github.com/TraceMachina/nativelink/blob/main/local-remote-execution/modules/lre.nix

Also note that the symlink here is too naive and should be done similar to what we have in

nativelink/local-remote-execution/modules/lre.nix

Line 135 in 1b38a64

config = {

jaroeichler

Reviewable status: 0 of 2 LGTMs obtained, and 4 of 7 files reviewed, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, docker-compose-compiles-nativelink (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04 / stable, windows-2022 / stable, and 3 discussions need to be resolved

flake.nix line 80 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

nit: For kicks, consider using uutils-coreutils-noprefix as pure rust implementation of the coreutils 💃

Let's go with that.

flake.nix line 83 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Interesting. This looks like it exposes several nonhermeticities in the devShell. Most of this should probably be in the main devShell as well.

Regarding the python dependency, why do we need the Full variant? What's the closure size difference here?

There's around 20M closure size difference. The only reason I went for that is that it's used somewhere else in the flake for some image. I've replaced it with python3

flake.nix line 490 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

This should be a flake module similar to the config.pre-commit.installationScript and config.local-remote-execution.installationScript. This way users can use it as a generic "NixOS Bazel compatibility" module and are not bound to our choice of clang/binutils/python and may import it in arbitrary Bazel+Nix projects.

See:

nativelink/flake.nix

Line 44 in 1b38a64

./local-remote-execution/flake-module.nix

(unused in our flake, but makes it importable)

nativelink/flake.nix

Line 479 in 1b38a64

flakeModule = ./local-remote-execution/flake-module.nix;

https://github.com/TraceMachina/nativelink/blob/main/local-remote-execution/flake-module.nix

https://github.com/TraceMachina/nativelink/blob/main/local-remote-execution/modules/lre.nix

Also note that the symlink here is too naive and should be done similar to what we have in

nativelink/local-remote-execution/modules/lre.nix

Line 135 in 1b38a64

config = {

I've pretty much copied your setup. An empty default of paths in nixos.settings.path is somehow intentional since I don't want to pass customClang to the module and just replacing it with pkgs.clang leads to iostream not found. Side note, lib.mdDoc is deprecated.

aaronmondal

Nice work!

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed

aaronmondal

+@adam-singer

Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed (waiting on @adam-singer)

allada

-@adam-singer

Reviewed 4 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: complete! 2 of 2 LGTMs obtained, and all files reviewed

allada

Can we rebase and land this?

Reviewable status: complete! 2 of 2 LGTMs obtained, and all files reviewed

jaroeichler · 2024-10-23T01:15:14Z

Sure, adjusted the nixos flake module path to the darwin flake module path

allada

Reviewed 7 of 7 files at r3, all commit messages.
Reviewable status: 2 of 2 LGTMs obtained, and all files reviewed, and pending CI: Web Platform Deployment / macos-14

aaronmondal

@jaroeichler I believe there is some sort of rebase conflict going on here. The darwin logic in the flake no longer needs to be conditional and is already on main. For some reason darwin-related changes show up in this diff.

Reviewed 1 of 7 files at r3, all commit messages.
Reviewable status: 2 of 2 LGTMs obtained, and all files reviewed, and pending CI: Web Platform Deployment / macos-14

jaroeichler · 2024-10-25T20:27:38Z

@aaronmondal Thanks, didn't notice it when reviewing the diff. Should be fine now.

aaronmondal

Thanks ❤️

Reviewed 4 of 7 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: 2 of 2 LGTMs obtained, and all files reviewed, and pending CI: Bazel Dev / macos-13, Bazel Dev / macos-14, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Coverage, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, NativeLink.com Cloud / Remote Cache / macos-14, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, Web Platform Deployment / macos-14, Web Platform Deployment / ubuntu-24.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), integration-tests (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable

aaronmondal requested changes Aug 29, 2024

View reviewed changes

jaroeichler force-pushed the nixos branch from 7f23c94 to f863312 Compare August 30, 2024 03:49

jaroeichler temporarily deployed to production August 30, 2024 03:50 — with GitHub Actions Inactive

jaroeichler force-pushed the nixos branch from f863312 to 742d7b0 Compare August 30, 2024 04:02

jaroeichler temporarily deployed to production August 30, 2024 04:02 — with GitHub Actions Inactive

jaroeichler commented Aug 30, 2024

View reviewed changes

aaronmondal approved these changes Aug 30, 2024

View reviewed changes

aaronmondal assigned adam-singer Aug 30, 2024

aaronmondal approved these changes Aug 30, 2024

View reviewed changes

jaroeichler mentioned this pull request Oct 17, 2024

running_actions_manager_test fails on NixOS #1373

Closed

allada unassigned adam-singer Oct 22, 2024

allada approved these changes Oct 22, 2024

View reviewed changes

jaroeichler force-pushed the nixos branch from 742d7b0 to fd963c6 Compare October 23, 2024 01:01

jaroeichler temporarily deployed to production October 23, 2024 01:01 — with GitHub Actions Inactive

jaroeichler had a problem deploying to production October 23, 2024 01:01 — with GitHub Actions Failure

jaroeichler temporarily deployed to production October 23, 2024 01:01 — with GitHub Actions Inactive

jaroeichler temporarily deployed to production October 23, 2024 01:02 — with GitHub Actions Inactive

allada approved these changes Oct 23, 2024

View reviewed changes

jaroeichler had a problem deploying to production October 23, 2024 02:08 — with GitHub Actions Failure

jaroeichler had a problem deploying to production October 23, 2024 15:07 — with GitHub Actions Failure

jaroeichler had a problem deploying to production October 25, 2024 19:14 — with GitHub Actions Failure

aaronmondal requested changes Oct 25, 2024

View reviewed changes

Add NixOS support

b7fefbd

jaroeichler force-pushed the nixos branch from fd963c6 to b7fefbd Compare October 25, 2024 20:25

jaroeichler temporarily deployed to production October 25, 2024 20:25 — with GitHub Actions Inactive

jaroeichler temporarily deployed to production October 25, 2024 20:26 — with GitHub Actions Inactive

aaronmondal approved these changes Oct 25, 2024

View reviewed changes

aaronmondal merged commit b2386fd into TraceMachina:main Oct 25, 2024
35 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NixOS support #1287

Add NixOS support #1287

jaroeichler commented Aug 28, 2024 •

edited by allada

Loading

CLAassistant commented Aug 28, 2024 •

edited

Loading

jaroeichler commented Aug 28, 2024

aaronmondal left a comment

jaroeichler left a comment

aaronmondal left a comment

aaronmondal left a comment

allada left a comment

allada left a comment

jaroeichler commented Oct 23, 2024

allada left a comment

aaronmondal left a comment

jaroeichler commented Oct 25, 2024

aaronmondal left a comment

Add NixOS support #1287

Add NixOS support #1287

Conversation

jaroeichler commented Aug 28, 2024 • edited by allada Loading

Description

Type of change

How Has This Been Tested?

Checklist

CLAassistant commented Aug 28, 2024 • edited Loading

jaroeichler commented Aug 28, 2024

aaronmondal left a comment

Choose a reason for hiding this comment

jaroeichler left a comment

Choose a reason for hiding this comment

aaronmondal left a comment

Choose a reason for hiding this comment

aaronmondal left a comment

Choose a reason for hiding this comment

allada left a comment

Choose a reason for hiding this comment

allada left a comment

Choose a reason for hiding this comment

jaroeichler commented Oct 23, 2024

allada left a comment

Choose a reason for hiding this comment

aaronmondal left a comment

Choose a reason for hiding this comment

jaroeichler commented Oct 25, 2024

aaronmondal left a comment

Choose a reason for hiding this comment

jaroeichler commented Aug 28, 2024 •

edited by allada

Loading

CLAassistant commented Aug 28, 2024 •

edited

Loading