Skip to content

Conversation

@weriomat
Copy link
Contributor

@weriomat weriomat commented Jun 9, 2025

this should fix #318 by deduplicating the flake paths with a Hashset

this should fix [serokell#318](serokell#318)
by deduplicating the flake paths with a Hashset
Copy link

@Sntx626 Sntx626 left a comment

Choose a reason for hiding this comment

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

This does not de-duplicate all flake checks, just flake checks from having the same node provided multiple times...

What is the use-case for this?

@weriomat
Copy link
Contributor Author

weriomat commented Oct 1, 2025

Currently, if I deploy multiple nodes from a single repo (i.e. deploy --targets .#a .#b) it should run nix flake check . for each node/ path for the node. Thus, it should be run two times, my attempt is to deduplicate by the path such as it will only get run once regardless of the amount of nodes deployed from that repo.
That is my understanding of the issue described in #318.
Have I missed something?

Copy link

@Sntx626 Sntx626 left a comment

Choose a reason for hiding this comment

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

I see, in this case I partially misunderstood the purpose of this PR in my previous comment.

I tested this after applying #341 to be able to pass multiple deployment --targets.
It works as described, though it still evaluates the same flake for multiple hosts.

Fixes the problem described.
LGTM

@weriomat
Copy link
Contributor Author

weriomat commented Oct 2, 2025

Hey what exactly do you mean by evaluating? Evaluating the configuration of the particular host, i.e. the system config or doing the nix flake check?

@Sntx626
Copy link

Sntx626 commented Oct 2, 2025

I'm getting multiple evaluations of the same flake (. in this case):

$ deploy --targets .#aux .#backup .#mail .#mon .#web
🚀 ℹ️ [deploy] [INFO] Running checks for flake in .
warning: unknown flake output 'deploy'
warning: The check omitted these incompatible systems: aarch64-darwin, aarch64-linux, x86_64-darwin
Use '--all-systems' to check all.
🚀 ℹ️ [deploy] [INFO] Evaluating flake in .
🚀 ℹ️ [deploy] [INFO] Evaluating flake in .
🚀 ℹ️ [deploy] [INFO] Evaluating flake in .
🚀 ℹ️ [deploy] [INFO] Evaluating flake in .
🚀 ℹ️ [deploy] [INFO] Evaluating flake in .
...

I'm referring to the multiple evaluations that should also be able to be de-duplicated, since one evaluation per flake should result in all nodes.

@weriomat
Copy link
Contributor Author

weriomat commented Oct 2, 2025

That is a misconception on your part. For each node/ profile the nixosConfiguration (via. outputs.nixosConfiguration.config.system.build.toplevel`) is evaluated in that step. As deploy-rs does not know anything about this configuration it cannot assume that the configuration is the same across the nodes/ profiles.
In essence, it could be evaluated in a singe step (for all requested host) instead of for each once, but that complicates the design of the program, the speed-up is not huge and is therefore not used.

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.

Checks are repeated several times

2 participants