Skip to content

Sync with upstream 2.33.3#351

Merged
edolstra merged 12 commits intomainfrom
sync-2.33.3
Feb 13, 2026
Merged

Sync with upstream 2.33.3#351
edolstra merged 12 commits intomainfrom
sync-2.33.3

Conversation

@edolstra
Copy link
Collaborator

@edolstra edolstra commented Feb 13, 2026

Motivation

Context

Summary by CodeRabbit

  • New Features

    • S3 binary caches now default to virtual-hosted-style addressing; added configurable addressing-style option (auto, path, virtual) for explicit control.
    • Enabled TCP keep-alive for HTTP connections to improve connection stability.
  • Bug Fixes

    • Enhanced error handling for invalid S3 addressing styles and alignment computation overflow protection.

edolstra and others added 12 commits February 2, 2026 18:43
(cherry picked from commit b9c77ec)
…tenance

[Backport 2.33-maintenance] Fix: `QueryPathInfo` throws on invalid path error in daemon
Old code with size + (size % 8 ? 8 - (size % 8) : 0) also suffered from this.

(cherry picked from commit d77c131)
…tenance

[Backport 2.33-maintenance] libutil: Add overflow check to alignUp
Fixes "attribute 'x' is a thunk".

(cherry picked from commit 2989a23)
…tenance

[Backport 2.33-maintenance] builtins.flakeRefToString: Evaluate attributes
Idle connections in libcurl's connection pool can be silently dropped by
the OS or intermediate firewalls/NATs before they can be reused, forcing
new TCP connections to be created. This is especially problematic for
HTTP/1.1 endpoints where multiplexing is unavailable.

Enable TCP keep-alive with a 60-second idle/interval on all curl easy
handles to prevent idle connection drops and improve connection reuse.

(cherry picked from commit 736abd5)
…tyle option

S3 binary caches now use virtual-hosted-style URLs by default for
standard AWS endpoints. Path-style endpoints (s3.region.amazonaws.com)
only serve HTTP/1.1, preventing HTTP/2 multiplexing and causing TCP
TIME_WAIT socket exhaustion under high concurrency. Virtual-hosted-style
endpoints (bucket.s3.region.amazonaws.com) support HTTP/2, enabling
multiplexing with the existing CURLPIPE_MULTIPLEX configuration.

Add a new `addressing-style` store option (auto/path/virtual) to control
this behavior. `auto` (default) uses virtual-hosted-style for standard
AWS endpoints and path-style for custom endpoints. `path` forces
path-style for backwards compatibility. `virtual` forces virtual-hosted-
style for all endpoints including custom ones.

Fixes: NixOS#15208
(cherry picked from commit 759f6c8)
Tagging release 2.33.3
@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

This PR introduces S3 addressing-style configuration options (virtual-hosted, path-style, auto), updates error handling for invalid paths in QueryPathInfo, enables TCP keep-alive for HTTP connections, adds overflow detection to alignment functions, expands S3 URL test coverage, and bumps the version to 2.33.3 with corresponding release notes.

Changes

Cohort / File(s) Summary
Version & Release Notes
.version, doc/manual/source/release-notes/rl-2.33.md
Version bump to 2.33.3 and release notes documenting new S3 addressing-style configuration, virtual-hosted-style defaults, and TCP keep-alive improvements.
S3 Addressing-Style Core
src/libstore/include/nix/store/s3-url.hh, src/libstore/s3-url.cc
Introduces S3AddressingStyle enum (Auto, Path, Virtual) with parsing/serialization functions, JSON marshalling, and BaseSetting specializations; extends ParsedS3URL with optional addressingStyle member and updates toHttpsUrl() logic for addressing mode computation and endpoint selection.
S3 Configuration Integration
src/libstore/include/nix/store/s3-binary-cache-store.hh
Adds addressingStyle configuration setting to S3BinaryCacheStoreConfig with descriptive documentation and includes it in s3UriSettings.
S3 Test Coverage
src/libstore-tests/s3-url.cc
Expands test suite with parameterized error tests for invalid addressing-style values, validates virtual-style with missing authority throws exceptions, and updates expected HTTPS URL outputs to reflect new default behaviors and endpoint configurations.
QueryPathInfo Error Handling
src/libstore/daemon.cc
Wraps store->queryPathInfo() call with exception handling to catch InvalidPath exceptions and return null instead of propagating errors.
HTTP Connection Improvements
src/libstore/filetransfer.cc
Enables TCP keep-alive probes for curl transfers by setting CURLOPT_TCP_KEEPALIVE and KEEPIDLE/KEEPINTVL to 60 seconds.
Alignment Function Safety
src/libutil/include/nix/util/alignment.hh, src/libutil-tests/alignment.cc
Adds overflow detection to alignUp() function with error throwing for values exceeding safe alignment bounds; includes typed test suite validating overflow scenarios and edge cases.
S3 Integration Tests
tests/nixos/s3-binary-cache-store.nix
Adds virtual-hosted-style MinIO test configuration with domain setup and introduces test_virtual_hosted_copy() and test_explicit_path_style() functions to validate addressing-style behavior in realistic scenarios.

Sequence Diagram

sequenceDiagram
    participant App as Application<br/>(e.g., nix copy)
    participant Config as S3BinaryCacheStoreConfig
    participant Parser as S3 URL Parser
    participant Logic as toHttpsUrl() Logic
    participant HTTP as HTTP Layer

    App->>Config: Configure S3 store<br/>with addressing-style
    Config->>Parser: Parse S3 URL with<br/>addressing-style param
    Parser->>Parser: parseS3AddressingStyle()<br/>validate enum value
    alt Invalid addressing-style
        Parser-->>App: Throw InvalidS3AddressingStyle
    end
    Parser->>Logic: Provide ParsedS3URL with<br/>addressing-style
    Logic->>Logic: Compute addressing mode<br/>(default Auto, consider<br/>endpoint type)
    Logic->>Logic: Select endpoint format<br/>(virtual-hosted vs path-style)
    Logic->>Logic: Handle dotted bucket<br/>names (path-style only)
    Logic->>HTTP: Generate final HTTPS URL<br/>with correct format
    HTTP->>HTTP: Execute request with<br/>TCP keep-alive enabled
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

flake-regression-test

Suggested reviewers

  • cole-h
  • grahamc

Poem

🐰 Hop, hop! New addressing styles,
Virtual paths now shine for miles,
TCP keeps the connection alive,
Alignments safe—no overflow to survive!
S3 buckets dance in the cloud,
This rabbit's proud!

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Sync with upstream 2.33.3' accurately describes the main objective—syncing with the upstream 2.33.3 release. The changeset comprehensively reflects this with a version bump, release notes, and feature/fix implementations from the upstream release.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync-2.33.3

Comment @coderabbitai help to get the list of available commands and usage tips.

- Henry [**(@cootshk)**](https://github.com/cootshk)
- Martin Joerg [**(@mjoerg)**](https://github.com/mjoerg)
- Farid Zakaria [**(@fzakaria)**](https://github.com/fzakaria)
# Release 2.33.3 (2026-02-13)
Copy link
Member

Choose a reason for hiding this comment

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

This looks odd... But if upstream has it then I guess let's keep it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's correct (though maybe the formatting won't look great).

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@doc/manual/source/release-notes/rl-2.33.md`:
- Around line 304-308: The fenced code block containing the example URL
"s3://my-bucket/key?region=us-east-1&addressing-style=path" needs a language tag
to satisfy markdownlint (MD040); update the triple-backtick fence to include a
tag such as ```text (or ```bash) before the URL so the block reads like ```text
followed by the URL and then closing backticks.

Comment on lines +304 to +308
Example using path-style for backwards compatibility:

```
s3://my-bucket/key?region=us-east-1&addressing-style=path
```
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a language tag to the fenced code block.
This avoids MD040 warnings and keeps markdownlint happy.

📝 Suggested fix
-  ```
+  ```text
   s3://my-bucket/key?region=us-east-1&addressing-style=path
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion
Example using path-style for backwards compatibility:
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)

[warning] 306-306: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In `@doc/manual/source/release-notes/rl-2.33.md` around lines 304 - 308, The
fenced code block containing the example URL
"s3://my-bucket/key?region=us-east-1&addressing-style=path" needs a language tag
to satisfy markdownlint (MD040); update the triple-backtick fence to include a
tag such as ```text (or ```bash) before the URL so the block reads like ```text
followed by the URL and then closing backticks.

@github-actions
Copy link

@github-actions github-actions bot temporarily deployed to pull request February 13, 2026 17:54 Inactive
@edolstra edolstra enabled auto-merge February 13, 2026 17:58
@edolstra edolstra added this pull request to the merge queue Feb 13, 2026
Merged via the queue into main with commit 8a1e3d8 Feb 13, 2026
28 checks passed
@edolstra edolstra deleted the sync-2.33.3 branch February 13, 2026 18:58
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.

5 participants