Skip to content

fix(native): Incorrect release build and make errors#26997

Merged
czentgr merged 1 commit intoprestodb:masterfrom
czentgr:cz_fix_runtime_bld
Jan 21, 2026
Merged

fix(native): Incorrect release build and make errors#26997
czentgr merged 1 commit intoprestodb:masterfrom
czentgr:cz_fix_runtime_bld

Conversation

@czentgr
Copy link
Copy Markdown
Contributor

@czentgr czentgr commented Jan 20, 2026

The recent runtime images published did not build
with the correct CMake options as intended.

This PR fixes two issues:

  1. Ensure the cmake flags are passed correctly in the publish step
  2. Ensure that make fails if an error is encountered in one of the sub-commands.

Fixes: #26996

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
  • If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

The recent runtime images published did not build
with the correct CMake options as intended.

This PR fixes two issues:
1. Ensure the cmake flags are passed correctly in the publish step
2. Ensure that make fails if an error is encountered in one of the
   sub-commands.
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Jan 20, 2026
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai bot commented Jan 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts native execution build and release pipeline so that CMake flags are correctly propagated during Docker image builds and make targets fail fast when sub-commands fail.

Sequence diagram for release publish job with corrected CMake flags

sequenceDiagram
    participant GitHubActions
    participant DockerCompose
    participant CMake
    participant Make
    participant RuntimeImage

    GitHubActions->>DockerCompose: docker compose build --build-arg EXTRA_CMAKE_FLAGS
    DockerCompose->>CMake: Invoke cmake with EXTRA_CMAKE_FLAGS
    CMake->>Make: Configure build system with flags
    Make->>Make: Build native binaries with configured flags
    Make-->>DockerCompose: Build result (native binaries)
    DockerCompose-->>RuntimeImage: Embed binaries into runtime image
    RuntimeImage-->>GitHubActions: Publishable native runtime image
Loading

Flow diagram for updated native Make targets and failure propagation

flowchart TD
    A[Start make target] --> B{Target}

    B -->|velox-submodule| C[git submodule sync --recursive]
    C -->|on success| D[git submodule update --init --recursive]
    C -->|on failure| Z[Make fails]

    B -->|debug| E[Invoke make cmake BUILD_DIR=debug BUILD_TYPE=Debug]
    E -->|on success| F[Invoke make build BUILD_DIR=debug]
    E -->|on failure| Z

    B -->|release| G[Invoke make cmake BUILD_DIR=release BUILD_TYPE=Release]
    G --> H[Invoke make build BUILD_DIR=release]

    B -->|cmake-and-build| I[cmake -B build_dir with CMAKE_FLAGS and EXTRA_CMAKE_FLAGS]
    I -->|on success| J[cmake --build build_dir -j NUM_THREADS]
    I -->|on failure| Z

    F --> K[Debug binaries built]
    H --> L[Release binaries built]
    J --> M[Configured and built without submodule update]

    K --> Y[End]
    L --> Y
    M --> Y
    Z --> Y[End with failure]
Loading

File-Level Changes

Change Details Files
Ensure git submodule sync failures break the build and avoid running subsequent commands on failure.
  • Chain git submodule sync with git submodule update using && so that submodule update only runs if sync succeeds.
presto-native-execution/Makefile
Make debug build target fail when the cmake configuration step fails.
  • Chain the debug target’s cmake invocation with the subsequent build step using && so that build only runs when configuration succeeds.
presto-native-execution/Makefile
Make combined cmake-and-build target fail when the cmake configuration step fails.
  • Chain the cmake configuration command and the build invocation using && so that build does not run on failed configuration and make correctly propagates the error.
presto-native-execution/Makefile
Fix passing of EXTRA_CMAKE_FLAGS into Docker build so that multiple flags are correctly interpreted.
  • Move the closing quote for EXTRA_CMAKE_FLAGS so that the multiline -D flags are included in the build arg instead of being treated as separate shell arguments.
.github/workflows/presto-release-publish.yml

Assessment against linked issues

Issue Objective Addressed Explanation
#26996 Ensure the native runtime release build is invoked with the intended CMake flags (including S3 and other features) in the publish workflow so that the runtime image includes those features.
#26996 Ensure that native builds fail when a sub-command (e.g., cmake, git submodule, or other steps in the Makefile targets) encounters an error, instead of silently succeeding with default features.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@majetideepak majetideepak marked this pull request as ready for review January 21, 2026 16:18
@majetideepak majetideepak requested review from a team and unidevel as code owners January 21, 2026 16:18
@prestodb-ci prestodb-ci requested review from a team, Dilli-Babu-Godari and Joe-Abraham and removed request for a team January 21, 2026 16:18
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • For cmake-and-build, consider reusing the existing cmake and build targets (e.g., via $(MAKE) cmake ... and $(MAKE) build ...) to avoid duplicating the configure/build logic and reduce the risk of these paths drifting apart.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- For `cmake-and-build`, consider reusing the existing `cmake` and `build` targets (e.g., via `$(MAKE) cmake ...` and `$(MAKE) build ...`) to avoid duplicating the configure/build logic and reduce the risk of these paths drifting apart.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@unidevel unidevel left a comment

Choose a reason for hiding this comment

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

My fault, thanks.

@czentgr czentgr merged commit 688e0a2 into prestodb:master Jan 21, 2026
117 of 120 checks passed
@czentgr czentgr deleted the cz_fix_runtime_bld branch January 21, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:IBM PR from IBM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

native: The runtime image does not contain S3 and other features

4 participants