rich7420 commented on code in PR #6377: URL: https://github.com/apache/datafusion-comet/pull/6377#discussion_r4129951370
########## docs/source/contributor-guide/release_process.md: ########## @@ -323,14 +324,15 @@ Options are: Example: ```shell -cd dev/release && ./build-release-comet.sh && cd ../.. +cd dev/release && ./build-release-comet.sh -b branch-0.13 && cd ../.. ``` #### Build output The build output is installed to a temporary local maven repository. The build script will print the name of the repository location at the end. This location will be required at the time of deploying the artifacts to a staging -repository +repository. The script also prints the commit that the artifacts were built from, which should be the commit that +you tag in the next step. Review Comment: Could we update the tagging commands below to tag the printed commit directly? They still fetch the release branch and reset to its latest tip. If the branch advances from A to B during the build, this script correctly builds everything from A, but following those commands tags B. I reproduced that mismatch with a local Git fixture. Using `git tag <rc-tag> <built-commit>` would preserve the pinned revision through this last step. ########## dev/release/build-release-comet.sh: ########## @@ -87,6 +96,17 @@ if [ "$JAVA_VERSION" -lt 17 ]; then fi echo "Java version check passed: $JAVA_VERSION" +# Resolve the branch to a single commit, and build both the native binaries (in the docker +# containers) and the jars (in a fresh clone) from that commit, so that they match even if +# the branch moves during the build. Nothing is built from the local checkout that this +# script is run from, so its current branch, local changes and stale build output (see +# https://github.com/apache/datafusion-comet/issues/2232) cannot leak into the release. Review Comment: Would it be simpler to derive the Docker build context and `cargo.config` from `BUILD_DIR` as well? Both image builds still use `$SCRIPT_DIR/comet-rm`, so the Dockerfile and entrypoint come from the caller's checkout. An uncommitted entrypoint edit was still executed in both builders in my fixture. Using the pinned clone for those paths removed that dependency without another clone. If caller-side build tooling is intentional, could we narrow the new documentation's guarantee about local changes to make that distinction clear? -- 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]
