andygrove opened a new issue, #3292:
URL: https://github.com/apache/datafusion-comet/issues/3292

   ### Describe the bug
   
   ## Description
   
   The `build-release-comet.sh` script has a bug where it builds native 
binaries from the specified branch (via `-b` flag) but builds JVM components 
from whatever branch is currently checked out on the host machine. This can 
result in mismatched releases where native code and JVM code come from 
different branches.
   
   ## Steps to Reproduce
   
   1. Check out `main` branch on host machine
   2. Run the release build script with a different branch specified:
      ```bash
      ./build-release-comet.sh -b branch-0.13
      ```
   3. Observe the resulting artifacts
   
   ## Expected Behavior
   
   The entire build (both native binaries and JVM components) should be built 
from the branch specified via the `-b` flag (`branch-0.13` in this example).
   
   ## Actual Behavior
   
   - Native binaries are correctly built from `branch-0.13` (inside Docker 
containers)
   - JVM components and git hash are built/captured from `main` (on the host 
machine)
   
   This creates a mismatched artifact that combines code from two different 
branches.
   
   ## Root Cause
   
   The script has two separate build phases:
   
   1. **Native binaries** (lines 132-160): Built inside Docker containers that 
properly clone the repo and checkout the specified branch
      ```bash
      docker run ... $BUILDER_IMAGE_AMD64 "${REPO}" "${BRANCH}" amd64
      ```
   
   2. **JVM components** (lines 196-209): Built on the host machine from 
whatever is currently checked out
      ```bash
      pushd $COMET_HOME_DIR
      GIT_HASH=$(git rev-parse --short HEAD)  # Gets hash from current checkout!
      ./mvnw install  # Builds from current checkout!
      ```
   
   The script never switches branches on the host machine before building the 
JVM components.
   
   ## Proposed Solutions
   
   ### Option 1: Check out the branch on host (Recommended)
   Add a git checkout before building JVM components:
   ```bash
   pushd $COMET_HOME_DIR
   git checkout "${BRANCH}"  # Add this line
   GIT_HASH=$(git rev-parse --short HEAD)
   ./mvnw install
   popd
   ```
   
   ### Option 2: Document the requirement
   If the script is intended to build JVM components from the current branch, 
document this clearly:
   - Update the usage message to indicate the user must be on the correct branch
   - Add a validation check that fails if the current branch doesn't match the 
`-b` parameter
   
   ### Option 3: Build everything in Docker
   Build JVM components inside the Docker container alongside the native 
binaries to ensure consistency.
   
   ## Impact
   
   This bug can lead to:
   - Release artifacts that combine code from multiple branches
   - Version mismatches between native and JVM components
   - Difficult-to-debug issues when the release doesn't behave as expected
   - Incorrect git hashes in release metadata
   
   ## Environment
   
   - Script: `dev/release/build-release-comet.sh`
   - Affected versions: Current main branch
   
   
   ### Steps to reproduce
   
   _No response_
   
   ### Expected behavior
   
   _No response_
   
   ### Additional context
   
   _No response_


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to