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

Introduce Nix development flake #330

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

aaronmondal
Copy link
Contributor

@aaronmondal aaronmondal commented Oct 20, 2023

Provide a mostly self-contained development environment automatically sets up tooling like Cargo and Bazel via nix.

This makes it easy for nix users to contribute to our project and enables future work on reproducible release binaries and containers.


This change is Reviewable

@aaronmondal
Copy link
Contributor Author

cc @blakehatch I can't add you as reviewer for some reason

@aaronmondal aaronmondal force-pushed the development-flake branch 2 times, most recently from fd5e0d3 to cdc5878 Compare October 20, 2023 18:43
Copy link
Collaborator

@allada allada left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 10 files at r1.
Reviewable status: 8 of 10 files reviewed, 4 unresolved discussions (waiting on @aaronmondal and @MarcusSorealheis)


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

  outputs = inputs@{ nixpkgs, flake-parts, ... }:
    flake-parts.lib.mkFlake { inherit inputs; } {
      systems = [ "x86_64-linux" ];

What about mac, windows, ARM?


.github/workflows/nix.yaml line 14 at r1 (raw file):

jobs:
  nix-bazel:
    runs-on: ubuntu-latest

nit: Lets pin specific versions.


.github/workflows/nix.yaml line 39 at r1 (raw file):

          bash -c "bazel test ..."
  nix-cargo:
    runs-on: ubuntu-latest

nit: ditto.


tools/wrapped-bazel.nix line 18 at r1 (raw file):

]]; then
    ${bazel}/bin/bazel $1 \
        --action_env=OPENSSL_INCLUDE_DIR="${openssl.dev}/include" \

nit: just put this into the .bazelrc something like:

build:nix --action_env=OPENSSL_INCLUDE_DIR
build:nix --action_env=OPENSSL_LIB_DIR

Then just inject those enviorns to bazel.

Then run bazel as "$@" --config nix

Provide a mostly self-contained development environment automatically
sets up tooling like Cargo and Bazel via nix.

This makes it easy for nix users to contribute to our project and
enables future work on reproducible release binaries and containers.
@aaronmondal
Copy link
Contributor Author

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

Previously, allada (Nathan (Blaise) Bruer) wrote…

What about mac, windows, ARM?

Native Windows doesn't work with Nix. WSL2 usually defaults to Ubuntu. As long as we don't depend on CUDA the current workflows should cover that.

I can't test Mac at the moment. I propose to postpone this until we have a way to debug those builds on Mac x86_64 and ARM.

Copy link
Collaborator

@allada allada left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: 8 of 10 files reviewed, 1 unresolved discussion (waiting on @aaronmondal and @MarcusSorealheis)


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

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Native Windows doesn't work with Nix. WSL2 usually defaults to Ubuntu. As long as we don't depend on CUDA the current workflows should cover that.

I can't test Mac at the moment. I propose to postpone this until we have a way to debug those builds on Mac x86_64 and ARM.

Yeah sounds good :-)

@aaronmondal
Copy link
Contributor Author

tools/wrapped-bazel.nix line 18 at r1 (raw file):

Previously, allada (Nathan (Blaise) Bruer) wrote…

nit: just put this into the .bazelrc something like:

build:nix --action_env=OPENSSL_INCLUDE_DIR
build:nix --action_env=OPENSSL_LIB_DIR

Then just inject those enviorns to bazel.

Then run bazel as "$@" --config nix

That doesn't work in this case because these paths are dynamically generated and the .bazelrc can't resolve $SOMEVARIABLE. We'd have to manually update these environment variables every time someone updates the flake. I had something like that at some point but it was too tedious to keep it in sync.

@aaronmondal aaronmondal merged commit a0792fd into TraceMachina:main Oct 20, 2023
14 of 15 checks passed
Copy link
Collaborator

@allada allada left a comment

Choose a reason for hiding this comment

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

Reviewable status: 8 of 10 files reviewed, all discussions resolved


tools/wrapped-bazel.nix line 18 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

That doesn't work in this case because these paths are dynamically generated and the .bazelrc can't resolve $SOMEVARIABLE. We'd have to manually update these environment variables every time someone updates the flake. I had something like that at some point but it was too tedious to keep it in sync.

On the line above this, cant you just use:
export OPENSSL_INCLUDE_DIR="${openssl.dev}/include"

The --action_env will auto-pickup those environs.

?

blakehatch pushed a commit to blakehatch/nativelink that referenced this pull request Nov 14, 2023
Provide a mostly self-contained development environment automatically
sets up tooling like Cargo and Bazel via nix.

This makes it easy for nix users to contribute to our project and
enables future work on reproducible release binaries and containers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants