Repository navigation
Conversation
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review of all three changed files at befb34a69a56fe7f117bb821d8950628aa4441b9: no new actionable findings and no existing blocking inline findings.
Reviewed flow: archive registration → checksum/extraction → package dispatch → CMake configure/build/install → source cleanup. The downloaded 0.2.0 archive matches the declared MD5 and extraction directory. Default builds order OpenSSL and curl before the SDK; named and resume paths use the same recipe.
Critical checkpoint conclusions:
- Goal: the patch prepares the third-party SDK dependency for the separate runtime integration. Static inspection supports the intended headers/archive installation; executable build success was not established here.
- Scope and reuse: the three-file change is focused and uses existing downloader, dispatch, installation-prefix and cleanup mechanisms.
- Concurrency: package installation/cleanup remains sequential; only normal compiler jobs run in parallel. No new application locks or shared state.
- Lifecycle: sources are checked, configured, built and installed before cleanup; shell failure propagation prevents cleanup after a failed command. No new static-initialization dependency.
- Configuration: build options select installed curl/OpenSSL and enable RTTI; no dynamic runtime configuration is added.
- Compatibility: upstream defaults provide static PIC C++17 code with tests/samples disabled. Inspected curl APIs fit Doris 8.2.1 and hash APIs fit OpenSSL 1.1.1s. No existing runtime ABI, storage or rolling-upgrade protocol changes. Upstream build configuration.
- Thrift/protobuf: no generated definition changes.
- Parallel paths: full, named-package and resume flows, plus platform/compiler setup, were inspected; no omitted package-registration path found.
- Conditions: no new special conditional branch; upstream dependency lookups use REQUIRED package discovery.
- Test coverage: no tests are added. Existing downloader tests and CI build configuration were inspected. Author-reported macOS package/smoke testing was read, not reproduced; Linux/full-build success is not claimed.
- Expected results: no test-result files changed.
- Observability: existing downloader/CMake/build diagnostics cover this build-only addition; no runtime metrics are needed in this patch.
- Persistence/transactions: unaffected.
- Database writes and crash atomicity: unaffected; no runtime SDK consumer is introduced.
- FE/BE variable transport: unaffected.
- Performance: extra SDK compilation is intentional; no new runtime hot path or redundant dependency rebuild was found.
- Other concerns: explicit Doris dependency paths, CMake root restrictions, archive identity, install layout and successful-build cache cleanup were checked without substantiating a defect.
No additional user focus was supplied. Review converged after three rounds, including the final frozen-summary challenge by both complete reviewers and the separate risk reviewer. No unresolved code-level question or uncovered changed-file scope remains. This was static inspection only: no builds, tests, compiler probes or runtime validation were executed, and no source files were modified.
Review converged after 3 round(s).
### Background The native OSS client proposed in apache#70423 needs the Alibaba Cloud OSS C++ SDK V2 in the Doris third-party installation. ### What problem does this PR solve? Issue Number: apache#65793 Related PR: apache#70423 (consumes the SDK in BE and Recycler) PR Type: Build/CI/Chore Extract the SDK build from apache#70423 so it can be included in the third-party build image ahead of the runtime integration. ### Details - **Affected:** Register the upstream 0.2.0 source archive and checksum. Add `oss_sdk` to the default build after curl and OpenSSL, and register its source directory for cleanup. - **Developer and release impact:** The build installs the SDK headers and static library using the Doris curl and OpenSSL dependencies. To update an existing third-party installation, run `thirdparty/build-thirdparty.sh oss_sdk`. - **License and third-party:** Alibaba Cloud OSS C++ SDK V2 0.2.0 is licensed under Apache-2.0. apache#70423 carries the distribution license metadata along with the runtime integration. - **Rollback:** Revert this change to remove the SDK from future third-party builds. ### Testing - Ran the package's `build_oss_sdk` function on macOS arm64 against a fresh extraction of the checksum-verified archive, using Doris curl 8.2.1 and OpenSSL 1.1.1s. Verified the installed static archive with `ar` and checked for the public `OSSClient.h` header. Compiled, linked and ran a C++17 smoke test that constructs an `OSSClient` against the installed libraries. - Ran `bash -n` on both modified scripts, the third-party downloader checksum test, and `git diff --check`. Confirmed the three-file patch matches apache#70423 byte for byte. ### Release note None ### Check List (For Author) - Behavior changed: Yes. The default third-party build includes the OSS SDK. - Does this need documentation: No. The third-party changelog includes the incremental build command. - Pre-submission status: Partially verified. The SDK package build passed locally. The full third-party build and Linux build were not run locally. ### AI Disclosure - Tool: OpenAI Codex (GPT-6) - Generated: Extracted the existing changes from apache#70423 and drafted this PR description. - Human review: Pending.
befb34a to
b0a5a04
Compare
|
run buildall |
Background
The native OSS client proposed in #70423 needs the Alibaba Cloud OSS C++ SDK V2 in the Doris third-party installation.
What problem does this PR solve?
Issue Number: #65793
Related PR: #70423 (consumes the SDK in BE and Recycler)
PR Type: Build/CI/Chore
Extract the SDK build from #70423 so it can be included in the third-party build image ahead of the runtime integration.
Details
oss_sdkto the default build after curl and OpenSSL, and register its source directory for cleanup.thirdparty/build-thirdparty.sh oss_sdk.Testing
build_oss_sdkfunction on macOS arm64 against a fresh extraction of the checksum-verified archive, using Doris curl 8.2.1 and OpenSSL 1.1.1s. Verified the installed static archive witharand checked for the publicOSSClient.hheader. Compiled, linked and ran a C++17 smoke test that constructs anOSSClientagainst the installed libraries.bash -non both modified scripts, the third-party downloader checksum test, andgit diff --check. Confirmed the three-file patch matches [feature](cloud) Support native OSS V2 object storage requests in BE and Recycler #70423 byte for byte.Release note
None
Check List (For Author)
AI Disclosure