From 1f710300c9de0a28b2c09cc95d201fa2ca7e9571 Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Costa Date: Tue, 14 Oct 2025 23:58:48 +0000 Subject: [PATCH] refactor(libstore): withCurlS3 -> withAWS Now that the legacy S3 implementation is gone, we can go back to calling things `NIX_WITH_S3_SUPPORT`. --- .github/workflows/ci.yml | 14 +++++++------- ci/gha/tests/default.nix | 8 +++----- ci/gha/tests/wrapper.nix | 4 ++-- src/libstore-tests/s3-binary-cache-store.cc | 2 +- src/libstore-tests/s3-url.cc | 2 +- src/libstore/aws-creds.cc | 2 +- src/libstore/builtins/fetchurl.cc | 2 +- src/libstore/filetransfer.cc | 10 +++++----- src/libstore/include/nix/store/aws-creds.hh | 2 +- src/libstore/include/nix/store/builtins.hh | 4 ++-- src/libstore/include/nix/store/filetransfer.hh | 6 +++--- .../include/nix/store/s3-binary-cache-store.hh | 2 +- src/libstore/include/nix/store/s3-url.hh | 2 +- src/libstore/meson.build | 2 +- src/libstore/package.nix | 6 +++--- src/libstore/s3-binary-cache-store.cc | 2 +- src/libstore/s3-url.cc | 2 +- src/libstore/unix/build/derivation-builder.cc | 12 ++++++------ .../unix/build/linux-derivation-builder.cc | 2 +- 19 files changed, 42 insertions(+), 44 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 00a7ef7a1..1259bbbb4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,7 +67,7 @@ jobs: instrumented: false primary: true stdenv: stdenv - withCurlS3: true + withAWS: true # TODO: remove once curl-based-s3 fully lands - scenario: on ubuntu (no s3) runs-on: ubuntu-24.04 @@ -75,21 +75,21 @@ jobs: instrumented: false primary: false stdenv: stdenv - withCurlS3: false + withAWS: false - scenario: on macos runs-on: macos-14 os: darwin instrumented: false primary: true stdenv: stdenv - withCurlS3: true + withAWS: true - scenario: on ubuntu (with sanitizers / coverage) runs-on: ubuntu-24.04 os: linux instrumented: true primary: false stdenv: clangStdenv - withCurlS3: true + withAWS: true name: tests ${{ matrix.scenario }} runs-on: ${{ matrix.runs-on }} timeout-minutes: 60 @@ -113,13 +113,13 @@ jobs: nix build --file ci/gha/tests/wrapper.nix componentTests -L \ --arg withInstrumentation ${{ matrix.instrumented }} \ --argstr stdenv "${{ matrix.stdenv }}" \ - ${{ format('--arg withCurlS3 {0}', matrix.withCurlS3) }} + ${{ format('--arg withAWS {0}', matrix.withAWS) }} - name: Run VM tests run: | nix build --file ci/gha/tests/wrapper.nix vmTests -L \ --arg withInstrumentation ${{ matrix.instrumented }} \ --argstr stdenv "${{ matrix.stdenv }}" \ - ${{ format('--arg withCurlS3 {0}', matrix.withCurlS3) }} + ${{ format('--arg withAWS {0}', matrix.withAWS) }} if: ${{ matrix.os == 'linux' }} - name: Run flake checks and prepare the installer tarball run: | @@ -131,7 +131,7 @@ jobs: nix build --file ci/gha/tests/wrapper.nix codeCoverage.coverageReports -L \ --arg withInstrumentation ${{ matrix.instrumented }} \ --argstr stdenv "${{ matrix.stdenv }}" \ - ${{ format('--arg withCurlS3 {0}', matrix.withCurlS3) }} \ + ${{ format('--arg withAWS {0}', matrix.withAWS) }} \ --out-link coverage-reports cat coverage-reports/index.txt >> $GITHUB_STEP_SUMMARY if: ${{ matrix.instrumented }} diff --git a/ci/gha/tests/default.nix b/ci/gha/tests/default.nix index be634e833..28e305a95 100644 --- a/ci/gha/tests/default.nix +++ b/ci/gha/tests/default.nix @@ -12,7 +12,7 @@ componentTestsPrefix ? "", withSanitizers ? false, withCoverage ? false, - withCurlS3 ? null, + withAWS ? null, ... }: @@ -58,9 +58,7 @@ rec { nix-expr = prev.nix-expr.override { enableGC = !withSanitizers; }; # Override AWS configuration if specified - nix-store = prev.nix-store.override ( - lib.optionalAttrs (withCurlS3 != null) { inherit withCurlS3; } - ); + nix-store = prev.nix-store.override (lib.optionalAttrs (withAWS != null) { inherit withAWS; }); mesonComponentOverrides = lib.composeManyExtensions componentOverrides; # Unclear how to make Perl bindings work with a dynamically linked ASAN. @@ -229,7 +227,7 @@ rec { vmTests = { } - // lib.optionalAttrs (withCurlS3 == true) { + // lib.optionalAttrs (withAWS == true) { # S3 binary cache store test using curl implementation inherit (nixosTests) curl-s3-binary-cache-store; } diff --git a/ci/gha/tests/wrapper.nix b/ci/gha/tests/wrapper.nix index 4b1656500..72b1ba7a3 100644 --- a/ci/gha/tests/wrapper.nix +++ b/ci/gha/tests/wrapper.nix @@ -5,7 +5,7 @@ stdenv ? "stdenv", componentTestsPrefix ? "", withInstrumentation ? false, - withCurlS3 ? null, + withAWS ? null, }@args: import ./. ( args @@ -13,6 +13,6 @@ import ./. ( getStdenv = p: p.${stdenv}; withSanitizers = withInstrumentation; withCoverage = withInstrumentation; - inherit withCurlS3; + inherit withAWS; } ) diff --git a/src/libstore-tests/s3-binary-cache-store.cc b/src/libstore-tests/s3-binary-cache-store.cc index 359c70148..670000520 100644 --- a/src/libstore-tests/s3-binary-cache-store.cc +++ b/src/libstore-tests/s3-binary-cache-store.cc @@ -1,6 +1,6 @@ #include "nix/store/s3-binary-cache-store.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/http-binary-cache-store.hh" # include "nix/store/filetransfer.hh" diff --git a/src/libstore-tests/s3-url.cc b/src/libstore-tests/s3-url.cc index 5f3f9702b..56ec4e40e 100644 --- a/src/libstore-tests/s3-url.cc +++ b/src/libstore-tests/s3-url.cc @@ -1,7 +1,7 @@ #include "nix/store/s3-url.hh" #include "nix/util/tests/gmock-matchers.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include # include diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index 05c11d24a..b0e1b7ed1 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -1,6 +1,6 @@ #include "nix/store/aws-creds.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include # include "nix/store/s3-url.hh" diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 2488e18af..d55915183 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -41,7 +41,7 @@ static void builtinFetchurl(const BuiltinBuilderContext & ctx) FileTransferRequest request(VerbatimURL{url}); request.decompress = false; -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT // Use pre-resolved credentials if available if (ctx.awsCredentials && request.uri.scheme() == "s3") { debug("[pid=%d] Using pre-resolved AWS credentials from parent process", getpid()); diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 2e12470a4..4f0f89b64 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -9,7 +9,7 @@ #include "store-config-private.hh" #include -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/aws-creds.hh" # include "nix/store/s3-url.hh" #endif @@ -435,7 +435,7 @@ struct curlFileTransfer : public FileTransfer } } -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT // Set up AWS SigV4 signing if this is an S3 request // Note: AWS SigV4 support guaranteed available (curl >= 7.75.0 checked at build time) // The username/password (access key ID and secret key) are set via the general @@ -820,7 +820,7 @@ struct curlFileTransfer : public FileTransfer void enqueueItem(std::shared_ptr item) { if (item->request.data && item->request.uri.scheme() != "http" && item->request.uri.scheme() != "https" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT && item->request.uri.scheme() != "s3" #endif ) @@ -841,7 +841,7 @@ struct curlFileTransfer : public FileTransfer { /* Ugly hack to support s3:// URIs. */ if (request.uri.scheme() == "s3") { -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT // New curl-based S3 implementation auto modifiedRequest = request; modifiedRequest.setupForS3(); @@ -876,7 +876,7 @@ ref makeFileTransfer() return makeCurlFileTransfer(); } -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT void FileTransferRequest::setupForS3() { auto parsedS3 = ParsedS3URL::parse(uri.parsed()); diff --git a/src/libstore/include/nix/store/aws-creds.hh b/src/libstore/include/nix/store/aws-creds.hh index 4930dc9d8..dcafa9c75 100644 --- a/src/libstore/include/nix/store/aws-creds.hh +++ b/src/libstore/include/nix/store/aws-creds.hh @@ -2,7 +2,7 @@ ///@file #include "nix/store/config.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/s3-url.hh" # include "nix/util/error.hh" diff --git a/src/libstore/include/nix/store/builtins.hh b/src/libstore/include/nix/store/builtins.hh index 5c15b2e9b..6b9431331 100644 --- a/src/libstore/include/nix/store/builtins.hh +++ b/src/libstore/include/nix/store/builtins.hh @@ -4,7 +4,7 @@ #include "nix/store/derivations.hh" #include "nix/store/config.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/aws-creds.hh" #endif @@ -18,7 +18,7 @@ struct BuiltinBuilderContext std::string caFileData; Path tmpDirInSandbox; -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT /** * Pre-resolved AWS credentials for S3 URLs in builtin:fetchurl. * When present, these should be used instead of creating new credential providers. diff --git a/src/libstore/include/nix/store/filetransfer.hh b/src/libstore/include/nix/store/filetransfer.hh index 78ce439ae..6ca6ffa16 100644 --- a/src/libstore/include/nix/store/filetransfer.hh +++ b/src/libstore/include/nix/store/filetransfer.hh @@ -12,7 +12,7 @@ #include "nix/util/url.hh" #include "nix/store/config.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/aws-creds.hh" #endif @@ -113,7 +113,7 @@ struct FileTransferRequest * When provided, these credentials will be used with curl's CURLOPT_USERNAME/PASSWORD option. */ std::optional usernameAuth; -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT /** * Pre-resolved AWS session token for S3 requests. * When provided along with usernameAuth, this will be used instead of fetching fresh credentials. @@ -132,7 +132,7 @@ struct FileTransferRequest return data ? "upload" : "download"; } -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT private: friend struct curlFileTransfer; void setupForS3(); diff --git a/src/libstore/include/nix/store/s3-binary-cache-store.hh b/src/libstore/include/nix/store/s3-binary-cache-store.hh index 61ff8cb6c..c071d0887 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -3,7 +3,7 @@ #include "nix/store/config.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/http-binary-cache-store.hh" diff --git a/src/libstore/include/nix/store/s3-url.hh b/src/libstore/include/nix/store/s3-url.hh index 49dadfbe8..4f0a7b0c2 100644 --- a/src/libstore/include/nix/store/s3-url.hh +++ b/src/libstore/include/nix/store/s3-url.hh @@ -2,7 +2,7 @@ ///@file #include "nix/store/config.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/util/url.hh" # include "nix/util/util.hh" diff --git a/src/libstore/meson.build b/src/libstore/meson.build index d691c10bf..c7d3f1600 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -160,7 +160,7 @@ if curl_s3_store_opt.enabled() deps_other += aws_crt_cpp endif -configdata_pub.set('NIX_WITH_CURL_S3', curl_s3_store_opt.enabled().to_int()) +configdata_pub.set('NIX_WITH_S3_SUPPORT', curl_s3_store_opt.enabled().to_int()) subdir('nix-meson-build-support/generate-header') diff --git a/src/libstore/package.nix b/src/libstore/package.nix index 846d0f15f..897662e11 100644 --- a/src/libstore/package.nix +++ b/src/libstore/package.nix @@ -23,7 +23,7 @@ embeddedSandboxShell ? stdenv.hostPlatform.isStatic, - withCurlS3 ? + withAWS ? # Default is this way because there have been issues building this dependency stdenv.hostPlatform == stdenv.buildPlatform && (stdenv.isLinux || stdenv.isDarwin), }: @@ -65,7 +65,7 @@ mkMesonLibrary (finalAttrs: { sqlite ] ++ lib.optional stdenv.hostPlatform.isLinux libseccomp - ++ lib.optional withCurlS3 aws-crt-cpp; + ++ lib.optional withAWS aws-crt-cpp; propagatedBuildInputs = [ nix-util @@ -75,7 +75,7 @@ mkMesonLibrary (finalAttrs: { mesonFlags = [ (lib.mesonEnable "seccomp-sandboxing" stdenv.hostPlatform.isLinux) (lib.mesonBool "embedded-sandbox-shell" embeddedSandboxShell) - (lib.mesonEnable "curl-s3-store" withCurlS3) + (lib.mesonEnable "curl-s3-store" withAWS) ] ++ lib.optionals stdenv.hostPlatform.isLinux [ (lib.mesonOption "sandbox-shell" "${busybox-sandbox-shell}/bin/busybox") diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 16228b9f1..fb62b9548 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -1,6 +1,6 @@ #include "nix/store/s3-binary-cache-store.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include diff --git a/src/libstore/s3-url.cc b/src/libstore/s3-url.cc index 478308270..947de60b0 100644 --- a/src/libstore/s3-url.cc +++ b/src/libstore/s3-url.cc @@ -1,6 +1,6 @@ #include "nix/store/s3-url.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/util/error.hh" # include "nix/util/split.hh" diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index f7bab7057..d0c8cce06 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -46,7 +46,7 @@ #include "store-config-private.hh" #include "build/derivation-check.hh" -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT # include "nix/store/aws-creds.hh" # include "nix/store/s3-url.hh" # include "nix/util/url.hh" @@ -296,7 +296,7 @@ protected: */ virtual void startChild(); -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT /** * Pre-resolve AWS credentials for S3 URLs in builtin:fetchurl. * This should be called before forking to ensure credentials are available in child. @@ -359,7 +359,7 @@ protected: */ struct RunChildArgs { -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT std::optional awsCredentials; #endif }; @@ -945,7 +945,7 @@ void DerivationBuilderImpl::openSlave() throw SysError("cannot pipe standard error into log file"); } -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT std::optional DerivationBuilderImpl::preResolveAwsCredentials() { if (drv.isBuiltin() && drv.builder == "builtin:fetchurl") { @@ -974,7 +974,7 @@ std::optional DerivationBuilderImpl::preResolveAwsCredentials() void DerivationBuilderImpl::startChild() { RunChildArgs args{ -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT .awsCredentials = preResolveAwsCredentials(), #endif }; @@ -1255,7 +1255,7 @@ void DerivationBuilderImpl::runChild(RunChildArgs args) BuiltinBuilderContext ctx{ .drv = drv, .tmpDirInSandbox = tmpDirInSandbox(), -#if NIX_WITH_CURL_S3 +#if NIX_WITH_S3_SUPPORT .awsCredentials = args.awsCredentials, #endif }; diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index be064566f..07e421bef 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -277,7 +277,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu void startChild() override { RunChildArgs args{ -# if NIX_WITH_CURL_S3 +# if NIX_WITH_S3_SUPPORT .awsCredentials = preResolveAwsCredentials(), # endif };