Skip to content

Place generated files in CMAKE_BINARY_DIR.#3459

Closed
teo-tsirpanis wants to merge 1 commit into
aws:mainfrom
teo-tsirpanis:binary-dir-autogen
Closed

Place generated files in CMAKE_BINARY_DIR.#3459
teo-tsirpanis wants to merge 1 commit into
aws:mainfrom
teo-tsirpanis:binary-dir-autogen

Conversation

@teo-tsirpanis

@teo-tsirpanis teo-tsirpanis commented Jun 20, 2025

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

This PR updates the CMake project to place generated files in the binary directory, instead of the source directory. This follows best practices, and unblocks the vcpkg port to support configuring aws-sdk-cpp in parallel.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kai-ion

kai-ion commented Oct 29, 2025

Copy link
Copy Markdown
Collaborator

pr merged

@kai-ion kai-ion closed this Oct 29, 2025
@teo-tsirpanis teo-tsirpanis deleted the binary-dir-autogen branch October 29, 2025 18:58
@teo-tsirpanis

Copy link
Copy Markdown
Contributor Author

@kai-ion what do you mean? The PR wasn't merged (example).

@teo-tsirpanis teo-tsirpanis restored the binary-dir-autogen branch March 12, 2026 20:25
poodlewars pushed a commit to man-group/ArcticDB that referenced this pull request Jun 5, 2026
Bump the AWS SDK and its CRT dependency stack to the latest versions
resolvable from vcpkg. This advances the vcpkg submodule and
builtin-baseline together (bb14c5ab24 -> cea592f477) because vcpkg
resolves dependency versions against the commit named by
builtin-baseline, not the submodule working tree; the override versions
for aws-crt-cpp and the aws-c-* libraries only exist in the newer
version database.

Overrides updated to the CRT set that aws-sdk-cpp 1.11.820 bundles as
submodules (crt-cpp 0.39.1, aws-c-common 0.13.1, aws-c-io 0.26.3,
aws-c-s3 0.12.4, aws-c-http 0.11.0, aws-c-auth 0.10.3, aws-c-cal
0.9.14, aws-c-mqtt 0.15.2, aws-c-event-stream 0.7.1, aws-c-sdkutils
0.2.4, aws-checksums 0.2.10), keeping our pins aligned with the
versions AWS tested together.

The overlay vcpkg.json was regenerated from the 1.11.820 source
(435 service features, up from 414) and the portfile SHA512 updated to
the 1.11.820 tarball (verified against the upstream vcpkg port hash and
a direct download).

Patch changes
-------------

Removed fix-refresh.patch. It changed RefreshIfExpired() in
STSProfileCredentialsProvider.cpp from "||" to "&&" so a non-expired
credential would not be refreshed merely because the reload interval had
elapsed. Upstream has since adopted exactly this logic and gone further:
1.11.820 reads "!IsTimeToRefresh(...) && !m_credentials.IsEmpty() &&
!m_credentials.ExpiresSoon(...)". The fix is upstream, and the patch's
context lines no longer exist, so it would fail to apply.

Removed fix-win-https-conn.patch (Windows only). It dropped the
"&& m_verifySSL" guard in WinHttpSyncHttpClient::OpenRequest so HTTPS
requests always set WINHTTP_FLAG_SECURE regardless of verifySSL.
1.11.820 already sets the flag unconditionally for HTTPS
("if (scheme == HTTPS) requestFlags |= WINHTTP_FLAG_SECURE;"), so the
patch is redundant and, again, no longer applies to the refactored code.

Added configure-binary-dir.patch (symlinked from the upstream vcpkg
port, aws/aws-sdk-cpp#3459). 1.11.820 generates VersionConfig.h and
SDKConfig.h into the source tree during configure; the patch redirects
them to the binary dir and reads the version from the VERSION file. This
is part of upstream's own patch list for 1.11.820 and is required to
build against a read-only source tree.

The >=1.11.486 default data-integrity (checksum) behaviour change that
previously blocked this upgrade is already neutralised in
s3_storage.hpp::configure_s3_checksum_validation, which forces
when_required unless the user opts in; the override note is updated to
record this.

Other dependencies without explicit overrides move to the new baseline
(e.g. boost 1.91.0, curl 8.20.0). vcpkg dry-run confirms the full plan
resolves and selects aws-sdk-cpp 1.11.820 with [s3, identity-management,
sts, cognito-identity, core].
poodlewars pushed a commit to man-group/ArcticDB that referenced this pull request Jun 16, 2026
Bump the AWS SDK and its CRT dependency stack to the latest versions
resolvable from vcpkg. This advances the vcpkg submodule and
builtin-baseline together (bb14c5ab24 -> cea592f477) because vcpkg
resolves dependency versions against the commit named by
builtin-baseline, not the submodule working tree; the override versions
for aws-crt-cpp and the aws-c-* libraries only exist in the newer
version database.

Overrides updated to the CRT set that aws-sdk-cpp 1.11.820 bundles as
submodules (crt-cpp 0.39.1, aws-c-common 0.13.1, aws-c-io 0.26.3,
aws-c-s3 0.12.4, aws-c-http 0.11.0, aws-c-auth 0.10.3, aws-c-cal
0.9.14, aws-c-mqtt 0.15.2, aws-c-event-stream 0.7.1, aws-c-sdkutils
0.2.4, aws-checksums 0.2.10), keeping our pins aligned with the
versions AWS tested together.

The overlay vcpkg.json was regenerated from the 1.11.820 source
(435 service features, up from 414) and the portfile SHA512 updated to
the 1.11.820 tarball (verified against the upstream vcpkg port hash and
a direct download).

Patch changes
-------------

Removed fix-refresh.patch. It changed RefreshIfExpired() in
STSProfileCredentialsProvider.cpp from "||" to "&&" so a non-expired
credential would not be refreshed merely because the reload interval had
elapsed. Upstream has since adopted exactly this logic and gone further:
1.11.820 reads "!IsTimeToRefresh(...) && !m_credentials.IsEmpty() &&
!m_credentials.ExpiresSoon(...)". The fix is upstream, and the patch's
context lines no longer exist, so it would fail to apply.

Removed fix-win-https-conn.patch (Windows only). It dropped the
"&& m_verifySSL" guard in WinHttpSyncHttpClient::OpenRequest so HTTPS
requests always set WINHTTP_FLAG_SECURE regardless of verifySSL.
1.11.820 already sets the flag unconditionally for HTTPS
("if (scheme == HTTPS) requestFlags |= WINHTTP_FLAG_SECURE;"), so the
patch is redundant and, again, no longer applies to the refactored code.

Added configure-binary-dir.patch (symlinked from the upstream vcpkg
port, aws/aws-sdk-cpp#3459). 1.11.820 generates VersionConfig.h and
SDKConfig.h into the source tree during configure; the patch redirects
them to the binary dir and reads the version from the VERSION file. This
is part of upstream's own patch list for 1.11.820 and is required to
build against a read-only source tree.

The >=1.11.486 default data-integrity (checksum) behaviour change that
previously blocked this upgrade is already neutralised in
s3_storage.hpp::configure_s3_checksum_validation, which forces
when_required unless the user opts in; the override note is updated to
record this.

Other dependencies without explicit overrides move to the new baseline
(e.g. boost 1.91.0, curl 8.20.0). vcpkg dry-run confirms the full plan
resolves and selects aws-sdk-cpp 1.11.820 with [s3, identity-management,
sts, cognito-identity, core].
poodlewars pushed a commit to man-group/ArcticDB that referenced this pull request Jun 16, 2026
Bump the AWS SDK and its CRT dependency stack to the latest versions
resolvable from vcpkg. This advances the vcpkg submodule and
builtin-baseline together (bb14c5ab24 -> cea592f477) because vcpkg
resolves dependency versions against the commit named by
builtin-baseline, not the submodule working tree; the override versions
for aws-crt-cpp and the aws-c-* libraries only exist in the newer
version database.

Overrides updated to the CRT set that aws-sdk-cpp 1.11.820 bundles as
submodules (crt-cpp 0.39.1, aws-c-common 0.13.1, aws-c-io 0.26.3,
aws-c-s3 0.12.4, aws-c-http 0.11.0, aws-c-auth 0.10.3, aws-c-cal
0.9.14, aws-c-mqtt 0.15.2, aws-c-event-stream 0.7.1, aws-c-sdkutils
0.2.4, aws-checksums 0.2.10), keeping our pins aligned with the
versions AWS tested together.

The overlay vcpkg.json was regenerated from the 1.11.820 source
(435 service features, up from 414) and the portfile SHA512 updated to
the 1.11.820 tarball (verified against the upstream vcpkg port hash and
a direct download).

Patch changes
-------------

Removed fix-refresh.patch. It changed RefreshIfExpired() in
STSProfileCredentialsProvider.cpp from "||" to "&&" so a non-expired
credential would not be refreshed merely because the reload interval had
elapsed. Upstream has since adopted exactly this logic and gone further:
1.11.820 reads "!IsTimeToRefresh(...) && !m_credentials.IsEmpty() &&
!m_credentials.ExpiresSoon(...)". The fix is upstream, and the patch's
context lines no longer exist, so it would fail to apply.

Removed fix-win-https-conn.patch (Windows only). It dropped the
"&& m_verifySSL" guard in WinHttpSyncHttpClient::OpenRequest so HTTPS
requests always set WINHTTP_FLAG_SECURE regardless of verifySSL.
1.11.820 already sets the flag unconditionally for HTTPS
("if (scheme == HTTPS) requestFlags |= WINHTTP_FLAG_SECURE;"), so the
patch is redundant and, again, no longer applies to the refactored code.

Added configure-binary-dir.patch (symlinked from the upstream vcpkg
port, aws/aws-sdk-cpp#3459). 1.11.820 generates VersionConfig.h and
SDKConfig.h into the source tree during configure; the patch redirects
them to the binary dir and reads the version from the VERSION file. This
is part of upstream's own patch list for 1.11.820 and is required to
build against a read-only source tree.

The >=1.11.486 default data-integrity (checksum) behaviour change that
previously blocked this upgrade is already neutralised in
s3_storage.hpp::configure_s3_checksum_validation, which forces
when_required unless the user opts in; the override note is updated to
record this.

Other dependencies without explicit overrides move to the new baseline
(e.g. boost 1.91.0, curl 8.20.0). vcpkg dry-run confirms the full plan
resolves and selects aws-sdk-cpp 1.11.820 with [s3, identity-management,
sts, cognito-identity, core].
poodlewars added a commit to man-group/ArcticDB that referenced this pull request Jun 17, 2026
Build and test with AWS_S3 tests:
https://github.com/man-group/ArcticDB/actions/runs/26978150121
Benchmarks with AWS_S3:
https://github.com/man-group/ArcticDB/actions/runs/26978179363

I've tested against VAST and PURE. I looked at the checksums on VAST
(haven't looked at PURE) and unfortunately looks like they aren't
providing checksums on GET requests - I will raise with them.
https://code.maninvestments.com/aseaton/scratch/src/branch/main/support/checksums

The motivation for this is so that we can opt in (on adhoc basis, to
investigate particular issues) to the checksum behaviour using,

```
inline void configure_s3_checksum_validation() {
    const char* response_checksum = std::getenv("AWS_RESPONSE_CHECKSUM_VALIDATION");
    const char* request_checksum = std::getenv("AWS_REQUEST_CHECKSUM_CALCULATION");

    if ((response_checksum && std::string(response_checksum) == "when_supported") ||
        (request_checksum && std::string(request_checksum) == "when_supported")) {
        log::storage().warn("S3 Checksum validation has been specifically enabled by user. "
                            "If endpoint doesn't support it, 1. incorrect objects could be silently written "
                            "2. Endpoint response will be rejected by SDK and lead to storage exception in arcticdb");
    } else {
#ifdef _WIN32
        _putenv_s("AWS_RESPONSE_CHECKSUM_VALIDATION", "when_required");
        _putenv_s("AWS_REQUEST_CHECKSUM_CALCULATION", "when_required");
#else
        setenv("AWS_RESPONSE_CHECKSUM_VALIDATION", "when_required", 1);
        setenv("AWS_REQUEST_CHECKSUM_CALCULATION", "when_required", 1);
#endif
    }
}
```

Documentation for these settings are here
https://docs.aws.amazon.com/sdkref/latest/guide/feature-dataintegrity.html
.

These settings don't do anything with our current SDK version so we need
to upgrade.

### Detailed Notes on the Changes

Bump the AWS SDK and its CRT dependency stack to the latest versions
resolvable from vcpkg. This advances the vcpkg submodule and
builtin-baseline together (bb14c5ab24 -> cea592f477).

Overrides updated to the CRT set that aws-sdk-cpp 1.11.820 bundles as
submodules (crt-cpp 0.39.1, aws-c-common 0.13.1, aws-c-io 0.26.3,
aws-c-s3 0.12.4, aws-c-http 0.11.0, aws-c-auth 0.10.3, aws-c-cal 0.9.14,
aws-c-mqtt 0.15.2, aws-c-event-stream 0.7.1, aws-c-sdkutils 0.2.4,
aws-checksums 0.2.10).

Removed fix-refresh.patch. It changed RefreshIfExpired() in
STSProfileCredentialsProvider.cpp from "||" to "&&" so a non-expired
credential would not be refreshed merely because the reload interval had
elapsed. Upstream has since adopted exactly this logic and gone further:
1.11.820 reads "!IsTimeToRefresh(...) && !m_credentials.IsEmpty() &&
!m_credentials.ExpiresSoon(...)". The fix is upstream, and the patch's
context lines no longer exist, so it would fail to apply.

Removed fix-win-https-conn.patch (Windows only). It dropped the "&&
m_verifySSL" guard in WinHttpSyncHttpClient::OpenRequest so HTTPS
requests always set WINHTTP_FLAG_SECURE regardless of verifySSL.
1.11.820 already sets the flag unconditionally for HTTPS ("if (scheme ==
HTTPS) requestFlags |= WINHTTP_FLAG_SECURE;"), so the patch is redundant
and, again, no longer applies to the refactored code.

Added configure-binary-dir.patch (symlinked from the upstream vcpkg
port, aws/aws-sdk-cpp#3459). 1.11.820 generates VersionConfig.h and
SDKConfig.h into the source tree during configure; the patch redirects
them to the binary dir and reads the version from the VERSION file. This
is part of upstream's own patch list for 1.11.820 and is required to
build against a read-only source tree.

The >=1.11.486 default data-integrity (checksum) behaviour change that
previously blocked this upgrade is already handled in
s3_storage.hpp::configure_s3_checksum_validation, which forces
when_required unless the user opts in; the override note is updated to
record this. Conda has already been running past this version.

We also needed to upgrade s2n. It must track aws-c-io; 0.26.3 needs s2n
>=1.5.10 (s2n/unstable/cleanup.h). 1.7.3 is the version bundled with
aws-crt-cpp 0.39.1.

Other upgrades pulled in by the new baseline:

| Package | Old | New | Notes |
|---|---|---|---|
| boost-\* (~75 components) | 1.90.0#1 | 1.91.0 | Biggest surface. Used
heavily (interprocess, multiprecision, dynamic-bitset, etc.).
`boost-compat` is newly pulled as a 1.91 component. |
| azure-storage-blobs-cpp | 12.16.0 | 12.17.0#1 | Azure backend. Now
pairs with the pinned `azure-core-cpp 1.12.0` / `azure-identity-cpp
1.6.0`. |
| azure-storage-common-cpp | 12.12.0 | 12.13.0#1 | Same version-skew
note as above. |
| curl | 8.19.0 | 8.20.0#2 | Constrained by `version>=8.18.0`, not
overridden. |
| fast-float | 8.2.4 | 8.2.6 | Arrow dep. |
| xsimd | 14.1.0 | 14.2.0 | Arrow dep. |
| liblzma | 5.8.2#1 | 5.8.3 | |
| libiconv | 1.18#3 | 1.18#4 | Port rebuild. |
| abseil | 20260107.1#2 | 20260107.1#3 | Same upstream, port-version
rebuild. |

---------

Co-authored-by: Alex Seaton <alex.seaton@man.com>
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.

2 participants