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

Bare minimum ssh-ng:// extensions for Hydra #12479

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ericson2314
Copy link
Member

Motivation

This is not as comprehensive as #10748, but I am also interested in figuring out whether all those additions are in fact necessary.

This is bare minimum needed for
NixOS/hydra#1445, which has notable gaps but nevertheless reimplements enough with ssh-ng:// to past all tests.

I would like to merge this change as definitely necessary, and unclear whether sufficient. Then I would iterate on the corresponding Hydra PR until it seems potentially correct, seeing what, if any, further Nix API changes are necessary.

Context

NixOS/hydra#1445, NixOS/hydra#688 and #4665


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This is not as comprehensive as #10748, but I am also interested in
figuring out whether all those additions are in fact necessary.

This is bare minimum needed for
NixOS/hydra#1445, which has notable gaps but
nevertheless reimplements enough with `ssh-ng://` to past all tests.

I would like to merge this change as definitely necessary, and unclear
whether sufficient. Then I would iterate on the corresponding Hydra PR
until it seems potentially correct, seeing what, if any, further Nix API
changes are necessary.
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Feb 17, 2025
@@ -122,6 +122,16 @@ public:
BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
BuildMode buildMode) override;

/**
* Note, the returned function must only be called once, or we'll
* try to read from the connection twice.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* try to read from the connection twice.
* try to read from the connection twice.
* This function is used by Hydra.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
store Issues and pull requests concerning the Nix store
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants