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

nix: minimum viable dev shell for make build and make test #448

Merged
merged 3 commits into from
Mar 19, 2023
Merged

nix: minimum viable dev shell for make build and make test #448

merged 3 commits into from
Mar 19, 2023

Conversation

jhillyerd
Copy link
Contributor

@jhillyerd jhillyerd commented Mar 9, 2023

Background: The current cargo2nix based nix flake is broken, and cannot be used to provide a development environment to nix users, it errors out.

This PR introduces a minimal viable devShell that provides a rustup environment for building auraed and auraescript. It uses a shellHook to set a default rust toolchain, as the existing Makefile makes changes to the rustup environment. We may want to switch the project to use a rust-toolchain.toml file in the future, but I don't know enough about aurae to properly test that.

This is a first step towards removing cargo2nix as agreed in #398. I'll send a followup PR to remove the cargo2nix portions of the flake.

Update: Added libseccomp dependency, and confirmed all of make build and make test succeed.

@cla-bot
Copy link

cla-bot bot commented Mar 9, 2023

In order to contribute to a Nivenly Foundation project you must sign and agree to the CLA. Reply with @cla-bot check to check again.

@jhillyerd
Copy link
Contributor Author

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Mar 9, 2023

The cla-bot has been summoned, and re-checked this pull request!

@jhillyerd jhillyerd marked this pull request as ready for review March 9, 2023 23:00
@ph
Copy link

ph commented Mar 12, 2023

I've been looking at this situation too, I am kinda new to nix. I wonder if we should really clean the file up as you mentioned and also use the rust-toolchain.yml directly using the overlay helpers fromRustupToolchainFile something like this in the flake.nix:

{
  description = "Aurae development";

  inputs = {
    nixpkgs.url      = "github:NixOS/nixpkgs/nixos-unstable";
    rust-overlay.url = "github:oxalica/rust-overlay";
    flake-utils.url  = "github:numtide/flake-utils";
  };

  outputs = { self, nixpkgs, rust-overlay, flake-utils }:
    flake-utils.lib.eachDefaultSystem (system:
      let
        overlays = [ (import rust-overlay) ];
        pkgs = import nixpkgs {
          inherit system overlays;
        };

        rustVersion = pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml;
      in {
        devShell = pkgs.mkShell {
          buildInputs =
            [
              (rustVersion.override { extensions = [ "rust-src" ]; })
              pkgs.rust-analyzer
              pkgs.protobuf
              pkgs.buf
              pkgs.libseccomp
            ];
        };
      });
}

file: rust-toolchain.toml

[toolchain]
channel = "1.68.0"
components = [ "rustfmt", "clippy", "rust-analyzer"]
targets = [ "x86_64-unknown-linux-musl", "aarch64-unknown-linux-musl" ]

We would also need to patch the Makefile to short circuit the rustup calls. I used the following, Note I don't know what are the good practice in nix for theses situation. But it seems to make everything in local environment work.

.PHONY: musl
musl: ## Add target for musl
ifeq ($(origin IN_NIX_SHELL), undefined)
	rustup target list | grep -qc '$(uname_m)-unknown-linux-musl (installed)' || \
	rustup target add $(uname_m)-unknown-linux-musl
endif

@jhillyerd
Copy link
Contributor Author

jhillyerd commented Mar 12, 2023

@ph yes, your example is very similar to what I was thinking. Instead of hacking the Makefile to check for nix shell, it may make sense for the entire project to switch over to rust-toolchain.toml, as that's part of rustup and not specific to nix. I suspect the Makefile is going about things the wrong way.

All that seems a bit much for my first PR to the project, I just want to make an incremental improvement (really a small break-fix) before I start deleting the broken aspects of the nix build.

@jhillyerd jhillyerd changed the title nix: minimum viable dev shell to make aurae(d|script)-build nix: minimum viable dev shell for make build and make test Mar 12, 2023
@krisnova krisnova merged commit 1290ec8 into aurae-runtime:main Mar 19, 2023
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.

3 participants