Skip to content

libstore: make FileTransfer injectable into HttpBinaryCacheStore#15206

Merged
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:injectable-filetransfer
Feb 12, 2026
Merged

libstore: make FileTransfer injectable into HttpBinaryCacheStore#15206
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:injectable-filetransfer

Conversation

@amaanq
Copy link
Member

@amaanq amaanq commented Feb 11, 2026

Motivation

This commit makes FileTransfer self-contained by giving it a reference to FileTransferSettings instead of reading from the global. It also adds an optional FileTransfer parameter to HttpBinaryCacheStore so callers can inject their own instance.

The main motivation is test isolation. The HTTPS store tests now create custom FileTransferSettings with the test CA certificate and pass it through makeFileTransfer(), avoiding global state mutation entirely.


Add 👍 to pull requests you find important.

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

@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Feb 11, 2026
@amaanq amaanq force-pushed the injectable-filetransfer branch 2 times, most recently from f1562fa to 39259b6 Compare February 11, 2026 22:28

namespace nix {

const std::filesystem::path & nixConfDir();
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this needed?

Copy link
Member

Choose a reason for hiding this comment

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

In a setting's default value. Once the setting info is in the .cc not header, this will go away.

Copy link
Member

Choose a reason for hiding this comment

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

Somewhere in #5638 this is covered as a desiderata for new settings system.

@amaanq amaanq force-pushed the injectable-filetransfer branch from 39259b6 to bfb5713 Compare February 11, 2026 22:36
@Ericson2314 Ericson2314 marked this pull request as draft February 11, 2026 22:45
@amaanq amaanq force-pushed the injectable-filetransfer branch from bfb5713 to 747d1d0 Compare February 11, 2026 23:06
@amaanq amaanq force-pushed the injectable-filetransfer branch from 747d1d0 to 9bea619 Compare February 11, 2026 23:09
@amaanq amaanq force-pushed the injectable-filetransfer branch 2 times, most recently from 39dc405 to 8172af5 Compare February 11, 2026 23:34
@Ericson2314 Ericson2314 marked this pull request as ready for review February 11, 2026 23:41
@amaanq amaanq force-pushed the injectable-filetransfer branch from 8172af5 to 00135bf Compare February 11, 2026 23:59
This commit makes `FileTransfer` self-contained by giving it a reference
to `FileTransferSettings` instead of reading from the global. It also
adds an optional `FileTransfer` parameter to `HttpBinaryCacheStore` so
callers can inject their own instance.

The main motivation is test isolation. The HTTPS store tests now create
custom `FileTransferSettings` with the test CA certificate and pass it
through `makeFileTransfer()`, avoiding global state mutation entirely.
@amaanq amaanq force-pushed the injectable-filetransfer branch from 00135bf to 403e30f Compare February 12, 2026 00:00
@Ericson2314 Ericson2314 added this pull request to the merge queue Feb 12, 2026
Merged via the queue into NixOS:master with commit c756d02 Feb 12, 2026
14 checks passed
@Ericson2314 Ericson2314 deleted the injectable-filetransfer branch February 12, 2026 15:42
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.

3 participants