Skip to content

The Nix pipelines #1573

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

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .cargo/audit.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't work in the sandbox

What happens in the sandbox? Does it fail to send HTTP requests to crates.io?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did not checked (this is crane generated defaults) but I suppose yes it fails

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it. Makes sense if this file was generated by some tool.

But I don't think we'd want to disable audits for yanked crates just because a dev tool doesn't work well with it.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Doesn't work in the sandbox
[yanked]
enabled = false # Warn for yanked crates in Cargo.lock (default: true)
update_index = false # Auto-update the crates.io index (default: true)
1 change: 1 addition & 0 deletions .envrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
use flake
24 changes: 24 additions & 0 deletions .github/flake.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: update-flake-lock

on:
workflow_dispatch: # allows manual triggering
schedule:
- cron: '0 0 * * 0' # runs weekly on Sunday at 00:00

jobs:
lockfile:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Install Determinate Nix
uses: DeterminateSystems/nix-installer-action@main
with:
determinate: true
- name: Update flake.lock
uses: DeterminateSystems/update-flake-lock@main
with:
pr-title: "Update flake.lock" # Title of PR to be created
pr-labels: | # Labels to be set on the PR
dependencies
automated
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/stage
/parts
/prime
.direnv
.gitignore.swp
.DS_Store
result
Expand Down
2 changes: 2 additions & 0 deletions deny.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[licenses]
allow = ["MIT"]
94 changes: 94 additions & 0 deletions flake.lock
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll have to research flake later, but could you discuss the pros of tracking the lock file in the repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same pros of tracking cargo.lock in repository - to be sure the shell will be SAME, and to be sure it is going to behave same way anywhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose the question is: do we need that amount of consistency? Or can we trust that the requirements and constraints in flake.nix are most likely good enough?

Continuing with the comparison to Cargo.lock, there are many times where you don't commit that lock file, because you don't need to use exact versions. This is common when authoring libraries, for example.

Choose a reason for hiding this comment

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

Not including it would null most of nix's benefits. I have yet to find a single nix flake without its respective lockfile attached.

Just my 2cts here. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, that makes sense. But I believe having to regularly update the lock file was part of the reason that the initial attempt was reverted (#1549 (comment)).

I'm guessing that these benefits are being able to create a reproducible environment? IMO that's not always the highest priority -- our devcontainer config isn't that strict, for example (actually I think it's a bit too strict right now). Sometimes ease of setup, with the assumption that a well-functioning environment will be built, is enough.

Copy link
Contributor Author

@Sk7Str1p3 Sk7Str1p3 Jun 5, 2025

Choose a reason for hiding this comment

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

That's why I added workflow updating flake.lock regularly (exactly, once in a week).

Although now I realized I didn't check it yet and don't really know how do I do that 😅.

I'm doing some research right now.

Also, (I didn't checked yet, but) I believe nix would refuse to work with flake.lock in .gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

189 changes: 189 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
{
description = ''
Git repository summary on your terminal
'';

inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";
crane.url = "github:ipetkov/crane";
flake-utils.url = "github:numtide/flake-utils";

advisory-db = {
url = "github:rustsec/advisory-db";
flake = false;
};
};

outputs =
{
self,
nixpkgs,
crane,
flake-utils,
advisory-db,
...
}:
flake-utils.lib.eachDefaultSystem (
system:
let
pkgs = nixpkgs.legacyPackages.${system};

inherit (pkgs) lib;
craneLib = crane.mkLib pkgs;

# This filter prevent project from being rebuilded then changing
# unrelated files ,e.g. README
Comment on lines +36 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# This filter prevent project from being rebuilded then changing
# unrelated files ,e.g. README
# This filter prevents the project from being rebuilt when changing
# unrelated files, e.g. README

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I'm bad with English, sorry. I'll fix soon

Comment on lines +36 to +37
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

Grammar error: 'prevent project from being rebuilded then changing' should be 'prevents the project from being rebuilt when changing'

Suggested change
# This filter prevent project from being rebuilded then changing
# unrelated files ,e.g. README
# This filter prevents the project from being rebuilt when changing
# unrelated files, e.g., README

Copilot uses AI. Check for mistakes.

Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

Formatting issue: there should be no space before the comma in ',e.g.'

Suggested change
# unrelated files ,e.g. README
# unrelated files, e.g. README

Copilot uses AI. Check for mistakes.

filter' =
path: _type:
builtins.match (lib.concatStringsSep "|" [
".*tera"
".*yaml"
".*zstd"
".*snap"
".*sh"
".+LICENSE.md"
]) path != null;
filter = path: type: (filter' path type) || (craneLib.filterCargoSources path type);
src = lib.cleanSourceWith {
src = ./.;
inherit filter;
name = "source";
};

# Common arguments can be set here to avoid repeating them later
common = {
inherit src;
strictDeps = true;

# Bunch of libraries required for package proper work
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Bunch of libraries required for package proper work
# Bunch of libraries required for package to properly work

buildInputs =
with pkgs;
[
# package dependencies
zstd
]
++ lib.optionals pkgs.stdenv.isDarwin (
with pkgs;
[
# additional dependencies on Darwin systems
CoreFoundation
libresolv
Security
]
);
# Software required for project build
nativeBuildInputs = with pkgs; [
cmake
pkg-config
];
# Tools required for checks
nativeCheckInputs = with pkgs; [ git ];

# Additional environment variables
# This one overrides build profile (default is 'release')
CARGO_PROFILE = "dev";

};

# Build dependencies only, so we will be able to reuse them further
cargoArtifacts = craneLib.buildDepsOnly common;

# Build the actual crate itself, reusing the dependency
# artifacts from above.
build = craneLib.buildPackage (common // { inherit cargoArtifacts; });
in
{
checks = {
# Build the crate as part of `nix flake check` for convenience
inherit build;

# Run clippy (and deny all warnings) on the crate source,
# again, reusing the dependency artifacts from above.
clippy = craneLib.cargoClippy (
common
// {
inherit cargoArtifacts;
cargoClippyExtraArgs = "--all-targets -- --deny warnings";
}
);

doc = craneLib.cargoDoc (common // { inherit cargoArtifacts; });

# Check formatting
fmt = craneLib.cargoFmt { inherit src; };

tomlFmt = craneLib.taploFmt {
src = pkgs.lib.sources.sourceFilesBySuffices src [ ".toml" ];
# taplo arguments can be further customized below as needed
# taploExtraArgs = "--config ./taplo.toml";
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The commented taplo configuration line references './taplo.toml' but the actual taplo.toml file exists at the project root. This should be uncommented and the path corrected to just 'taplo.toml' or removed if not needed.

Suggested change
# taploExtraArgs = "--config ./taplo.toml";
taploExtraArgs = "--config taplo.toml";

Copilot uses AI. Check for mistakes.

};

# Audit dependencies
audit = craneLib.cargoAudit { inherit src advisory-db; };

# Audit licenses
deny = craneLib.cargoDeny { inherit src; };

# Run tests with cargo-nextest
# Consider setting `doCheck = false` on `my-crate` if you do not want
# the tests to run twice
nextest = craneLib.cargoNextest (
common
// {
inherit cargoArtifacts;
partitions = 1;
partitionType = "count";
cargoNextestPartitionsExtraArgs = "--no-tests=pass";
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The cargo-nextest argument '--no-tests=pass' appears incorrect. The correct nextest argument should be '--fail-fast' or similar. '--no-tests' is not a valid nextest option.

Suggested change
cargoNextestPartitionsExtraArgs = "--no-tests=pass";
cargoNextestPartitionsExtraArgs = "--fail-fast";

Copilot uses AI. Check for mistakes.

}
);
};

packages = rec {
onefetch-debug = craneLib.buildPackage (
common
// {
inherit cargoArtifacts;
doCheck = false;
}
);
onefetch = craneLib.buildPackage (
common
// {
CARGO_PROFILE = "release";
inherit cargoArtifacts;
doCheck = false;
}
);
default = onefetch-debug;
};

apps.default = flake-utils.lib.mkApp { drv = (build // { CARGO_PROFILE = "release"; }); };
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The attribute set merge syntax '(build // { CARGO_PROFILE = "release"; })' is incorrect here. The 'build' derivation cannot be merged with environment variables this way. This should reference the 'onefetch' package from packages instead.

Suggested change
apps.default = flake-utils.lib.mkApp { drv = (build // { CARGO_PROFILE = "release"; }); };
apps.default = flake-utils.lib.mkApp { drv = self.packages.${system}.onefetch; };

Copilot uses AI. Check for mistakes.


devShells.default = craneLib.devShell {
# Inherit inputs from checks.
checks = self.checks.${system};

# Additional dev-shell environment variables can be set directly
# MY_CUSTOM_DEVELOPMENT_VAR = "something else";

# Extra inputs can be added here; cargo and rustc are provided by default.
packages = with pkgs; [
# pkgs.ripgrep
nixd
nixfmt-rfc-style
];
};
}
);
# Sets substituters to avoid locally building something already built
nixConfig = {
extra-substituters = [
"https://crane.cachix.org"
"https://cache.garnix.io"
];
extra-trusted-public-keys = [
"crane.cachix.org-1:8Scfpmn9w+hGdXH/Q9tTLiYAE/2dnJYRJP7kl80GuRk="
"cache.garnix.io:CTFPyKSLcx5RMJKfLo5EEPUObbA78b0YQ2DTCJXqr9g="
];
};
}
22 changes: 22 additions & 0 deletions taplo.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Setting up TOML formatting sounds good to me, but TBH I think this can go in a separate PR. That way we can get TOML formatting merged in faster while we still discuss flake.

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Sorts `Cargo.toml` dependencies. All other `.toml` files are formatted with the default config.
#
# https://taplo.tamasfe.dev/configuration/file.html#configuration-file

[formatting]
align_comments = true
align_entries = true
#
array_auto_collapse = false
array_auto_expand = true
array_trailing_comma = true
#
compact_arrays = false
compact_entries = false
compact_inline_tables = false
#
indent_entries = true
indent_tables = true
#
reorder_arrays = true
reorder_inline_tables = true
reorder_keys = true