feat(libstore): curl-based s3 #13752

lovesegfault · 2025-08-14T00:14:56Z

Motivation

#13084

Context

Early days, just making a draft for visibility and CI

Add 👍 to pull requests you find important.

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

src/libstore/filetransfer.cc

src/libstore/meson.build

xokdvium · 2025-08-14T17:47:06Z

Would it be conceivable to just outright replace aws-sdk-cpp with this? I don't think we'd want to support it if we land this eventually.

lovesegfault · 2025-08-14T18:15:41Z

I was unsure about just removing the existing implementation outright, but I can do that if folks are OK with it?

xokdvium · 2025-08-14T18:23:08Z

but I can do that if folks are OK with it?

Can't speak for everyone, but I feel ripping out aws-sdk-cpp would be very welcome if the new thing is more lightweight and reuses the curl infrastructure with feature parity. That would definitely be better than supporting both implementations.

lovesegfault · 2025-08-14T18:34:27Z

@xokdvium I'm going to get this in a fully working state, and after I'm happy with it I'll tack on a final commit removing the old impl, so if there's tension we can discuss that point separately, or I can make it a separate PR, etc.

I'm with you that if this works well, we're better off just tossing the old impl, though that would mean folks building with old curl would lose the ability to talk to s3, but idk if we care to support people building non-standard configurations like that.

Mic92 · 2025-08-15T06:04:14Z

I want to definitely test this with our hydra staging instance to ensure it still can upload stuff to s3 buckets before merging.

lovesegfault · 2025-08-15T06:40:10Z

AFAICT this is ready for (very) preliminary testing; at least I was able to pull and push to an S3 store and auth just worked, like magic :)

lovesegfault · 2025-08-15T07:47:47Z

Nevermind, I messed something up, buckets outside of us-east-1 don't work :P

src/libstore/http-binary-cache-store.cc

xokdvium

Some preliminary comments

src/libstore-tests/filetransfer-s3.cc

src/libstore-tests/s3-edge-cases.cc

src/libstore-tests/s3-http-integration.cc

src/libstore/aws-auth.cc

src/libstore/include/nix/store/http-binary-cache-store.hh

This allows us to replace some very hacky and not correct string concatentation in `HttpBinaryCacheStore`. It will especially be useful with NixOS#13752, when today's hacks started to cause problems in practice, not just theory. Also make `fixGitURL` returned a `ParsedURL`.

This is extracted from the work in NixOS#13752

This allows us to replace some very hacky and not correct string concatentation in `HttpBinaryCacheStore`. It will especially be useful with NixOS#13752, when today's hacks started to cause problems in practice, not just theory. Also make `fixGitURL` returned a `ParsedURL`.

src/libstore-tests/s3-http-integration.cc

Ericson2314 · 2025-08-25T21:29:34Z

src/libstore/filetransfer.cc

+{
+    // Access the curlFileTransfer singleton and clear its cache
+    try {
+        auto ft = getFileTransfer();


This will not work as expected in builtin derivaiton fetching, because it doesn't use getFileTransfer in that case because it is in a fork.

Yeah, those would each end up with their own provider. I couldn't see a clean way to improve that though, and AFAICT the status-quo also does that.

We should add a test though.

#13791 (comment) This comment was supposed to go with this PR. It might be a pain, but it looks like if we keep on following the definitions of these global AWS functions, eventually we can avoid global state and fork problems, while still using public things.

This allows us to replace some very hacky and not correct string concatentation in `HttpBinaryCacheStore`. It will especially be useful with NixOS#13752, when today's hacks started to cause problems in practice, not just theory. Also make `fixGitURL` returned a `ParsedURL`.

This commit replaces the AWS C++ SDK with a lighter curl-based approach for S3 binary cache operations. - Removed dependency on the heavy aws-cpp-sdk-s3 and aws-cpp-sdk-transfer - Added lightweight aws-crt-cpp for credential resolution only - Leverages curl's native AWS SigV4 authentication (requires curl >= 7.75.0) - S3BinaryCacheStore now delegates to HttpBinaryCacheStore - Function s3ToHttpsUrl converts ParsedS3URL to ParsedURL - Multipart uploads are no longer supported (may be reimplemented later) - Build now requires curl >= 7.75.0 for AWS SigV4 support Fixes: NixOS#13084, NixOS#12671, NixOS#11748, NixOS#12403, NixOS#5947

Ericson2314 · 2025-08-27T14:46:50Z

src/libstore/include/nix/store/http-binary-cache-store.hh

@@ -1,3 +1,5 @@
+#pragma once


Suggested change

#pragma once

#pragma once

///@file

And we can land this one write away, just to chip things off to shrink the diff

Ericson2314 · 2025-08-27T14:48:09Z

src/libstore/meson.build

+if not aws_crt_cpp.found()
+  # Fallback: try pkg-config with different name
+  aws_crt_cpp = dependency('libaws-crt-cpp', required : false)
+endif
+if not aws_crt_cpp.found()
+  # Fallback: try to find library manually
+  aws_crt_cpp = cxx.find_library('aws-crt-cpp', required : false)
+endif


is it in practice fucked up like this? I would hope not! :)

xokdvium reviewed Aug 14, 2025

View reviewed changes

src/libstore/filetransfer.cc Outdated Show resolved Hide resolved

Mic92 reviewed Aug 14, 2025

View reviewed changes

src/libstore/meson.build Show resolved Hide resolved

lovesegfault force-pushed the curl-based-s3-v2 branch from cd41165 to 1046367 Compare August 14, 2025 22:50

github-actions bot added the store Issues and pull requests concerning the Nix store label Aug 14, 2025

lovesegfault force-pushed the curl-based-s3-v2 branch 2 times, most recently from 2030275 to 86873ed Compare August 14, 2025 23:40

github-actions bot added the documentation label Aug 14, 2025

lovesegfault force-pushed the curl-based-s3-v2 branch 3 times, most recently from aefcffd to 3433232 Compare August 15, 2025 20:33

xokdvium reviewed Aug 15, 2025

View reviewed changes

src/libstore/http-binary-cache-store.cc Outdated Show resolved Hide resolved

lovesegfault force-pushed the curl-based-s3-v2 branch 3 times, most recently from 2dd60ad to 80e3327 Compare August 17, 2025 04:17

lovesegfault marked this pull request as ready for review August 17, 2025 04:37

lovesegfault requested review from Ericson2314 and edolstra as code owners August 17, 2025 04:37

lovesegfault requested review from Mic92 and xokdvium August 17, 2025 04:37

xokdvium reviewed Aug 17, 2025

View reviewed changes

lovesegfault force-pushed the curl-based-s3-v2 branch from 80e3327 to 3333622 Compare August 18, 2025 03:49

This comment was marked as spam.

Sign in to view

lovesegfault added a commit to lovesegfault/nix that referenced this pull request Aug 25, 2025

feat(libstore/s3): add toHttpsUrl

94edec9

This is extracted from the work in NixOS#13752

lovesegfault added a commit to lovesegfault/nix that referenced this pull request Aug 25, 2025

feat(libstore/s3): add toHttpsUrl

26914fb

This is extracted from the work in NixOS#13752

lovesegfault added a commit to lovesegfault/nix that referenced this pull request Aug 25, 2025

feat(libstore/s3): add toHttpsUrl

67f5354

This is extracted from the work in NixOS#13752

lovesegfault added a commit to lovesegfault/nix that referenced this pull request Aug 25, 2025

feat(libstore/s3): add toHttpsUrl

135840b

This is extracted from the work in NixOS#13752

lovesegfault added a commit to lovesegfault/nix that referenced this pull request Aug 25, 2025

feat(libstore/s3): add toHttpsUrl

663c0d5

This is extracted from the work in NixOS#13752

lovesegfault added a commit to lovesegfault/nix that referenced this pull request Aug 25, 2025

feat(libstore/s3): add toHttpsUrl

5985d67

This is extracted from the work in NixOS#13752

lovesegfault force-pushed the curl-based-s3-v2 branch from 97f78c0 to 1c2d53b Compare August 25, 2025 20:46

Ericson2314 reviewed Aug 25, 2025

View reviewed changes

src/libstore-tests/s3-http-integration.cc Outdated Show resolved Hide resolved

lovesegfault force-pushed the curl-based-s3-v2 branch from 1c2d53b to 55f8b1a Compare August 25, 2025 21:12

Ericson2314 reviewed Aug 25, 2025

View reviewed changes

lovesegfault force-pushed the curl-based-s3-v2 branch from 55f8b1a to 9e2e093 Compare August 26, 2025 00:25

lovesegfault force-pushed the curl-based-s3-v2 branch from 9e2e093 to 6cc049a Compare August 27, 2025 14:31

lovesegfault added 3 commits August 27, 2025 14:36

drop! comment out functional tests that fail locally

c2af43e

feat(libstore): AWS credential provider caching

e81af42

lovesegfault force-pushed the curl-based-s3-v2 branch from 6cc049a to e81af42 Compare August 27, 2025 14:36

Ericson2314 reviewed Aug 27, 2025

View reviewed changes

Uh oh!

feat(libstore): curl-based s3 #13752

Are you sure you want to change the base?

feat(libstore): curl-based s3 #13752

Conversation

lovesegfault commented Aug 14, 2025

Motivation

Context

Uh oh!

Uh oh!

Uh oh!

xokdvium commented Aug 14, 2025

Uh oh!

lovesegfault commented Aug 14, 2025

Uh oh!

xokdvium commented Aug 14, 2025

Uh oh!

lovesegfault commented Aug 14, 2025

Uh oh!

Mic92 commented Aug 15, 2025

Uh oh!

lovesegfault commented Aug 15, 2025

Uh oh!

lovesegfault commented Aug 15, 2025

Uh oh!

Uh oh!

xokdvium left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Ericson2314 Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

lovesegfault Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

lovesegfault Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!