viirya commented on code in PR #7524:
URL: https://github.com/apache/datafusion-comet/pull/7524#discussion_r4485393361
##########
Makefile:
##########
@@ -67,52 +67,31 @@ format:
./mvnw compile test-compile scalafix:scalafix -Psemanticdb $(PROFILES)
./mvnw spotless:apply $(PROFILES)
-# build native libs for amd64 architecture Linux/MacOS on a Linux/amd64
machine/container
+# build native libs for amd64 architecture Linux on a Linux/amd64
machine/container
core-amd64-libs:
cd native && RUSTFLAGS="-Ctarget-cpu=x86-64-v3" cargo build --release
$(FEATURES_ARG)
-ifdef HAS_OSXCROSS
- rustup target add x86_64-apple-darwin
- cd native && cargo build --target x86_64-apple-darwin --release
$(FEATURES_ARG)
-endif
-# build native libs for arm64 architecture Linux/MacOS on a Linux/arm64
machine/container
+# build native libs for arm64 architecture Linux on a Linux/arm64
machine/container
core-arm64-libs:
cd native && RUSTFLAGS="-Ctarget-cpu=neoverse-n1" cargo build --release
$(FEATURES_ARG)
-ifdef HAS_OSXCROSS
- rustup target add aarch64-apple-darwin
- cd native && cargo build --target aarch64-apple-darwin --release
$(FEATURES_ARG)
-endif
core-amd64:
Review Comment:
`core-amd64` and `core-arm64` both end with `./dev/deploy-file`, which has
never existed in this repo, and nothing calls either target. After this change
they still fail at their last step. Delete both targets instead?
##########
Makefile:
##########
@@ -67,52 +67,31 @@ format:
./mvnw compile test-compile scalafix:scalafix -Psemanticdb $(PROFILES)
./mvnw spotless:apply $(PROFILES)
-# build native libs for amd64 architecture Linux/MacOS on a Linux/amd64
machine/container
+# build native libs for amd64 architecture Linux on a Linux/amd64
machine/container
core-amd64-libs:
cd native && RUSTFLAGS="-Ctarget-cpu=x86-64-v3" cargo build --release
$(FEATURES_ARG)
-ifdef HAS_OSXCROSS
- rustup target add x86_64-apple-darwin
- cd native && cargo build --target x86_64-apple-darwin --release
$(FEATURES_ARG)
-endif
-# build native libs for arm64 architecture Linux/MacOS on a Linux/arm64
machine/container
+# build native libs for arm64 architecture Linux on a Linux/arm64
machine/container
core-arm64-libs:
cd native && RUSTFLAGS="-Ctarget-cpu=neoverse-n1" cargo build --release
$(FEATURES_ARG)
-ifdef HAS_OSXCROSS
- rustup target add aarch64-apple-darwin
- cd native && cargo build --target aarch64-apple-darwin --release
$(FEATURES_ARG)
-endif
core-amd64:
- rustup target add x86_64-apple-darwin
- cd native && RUSTFLAGS="-Ctarget-cpu=skylake" CC=o64-clang
CXX=o64-clang++ cargo build --target x86_64-apple-darwin --release
$(FEATURES_ARG)
- mkdir -p spark/target/classes/org/apache/comet/darwin/x86_64
- cp native/target/x86_64-apple-darwin/release/libcomet.dylib
spark/target/classes/org/apache/comet/darwin/x86_64
cd native && RUSTFLAGS="-Ctarget-cpu=x86-64-v3" cargo build --release
$(FEATURES_ARG)
mkdir -p spark/target/classes/org/apache/comet/linux/amd64
cp native/target/release/libcomet.so
spark/target/classes/org/apache/comet/linux/amd64
jar -cf spark/target/comet-native-x86_64.jar \
- -C spark/target/classes/org/apache/comet darwin \
-C spark/target/classes/org/apache/comet linux
./dev/deploy-file spark/target/comet-native-x86_64.jar
comet-native-x86_64${COMET_CLASSIFIER} jar
core-arm64:
- rustup target add aarch64-apple-darwin
- cd native && RUSTFLAGS="-Ctarget-cpu=apple-m1"
CC=arm64-apple-darwin21.4-clang CXX=arm64-apple-darwin21.4-clang++
CARGO_FEATURE_NEON=1 cargo build --target aarch64-apple-darwin --release
$(FEATURES_ARG)
- mkdir -p spark/target/classes/org/apache/comet/darwin/aarch64
- cp native/target/aarch64-apple-darwin/release/libcomet.dylib
spark/target/classes/org/apache/comet/darwin/aarch64
cd native && RUSTFLAGS="-Ctarget-cpu=neoverse-n1" cargo build --release
$(FEATURES_ARG)
mkdir -p spark/target/classes/org/apache/comet/linux/aarch64
cp native/target/release/libcomet.so
spark/target/classes/org/apache/comet/linux/aarch64
jar -cf spark/target/comet-native-aarch64.jar \
- -C spark/target/classes/org/apache/comet darwin \
-C spark/target/classes/org/apache/comet linux
./dev/deploy-file spark/target/comet-native-aarch64.jar
comet-native-aarch64${COMET_CLASSIFIER} jar
release-linux: clean
Review Comment:
Without the darwin builds, this is just `clean` plus `release`, and nothing
references it. Drop it, along with its `.PHONY` entry on line 18?
##########
dev/release/comet-rm/Dockerfile:
##########
@@ -63,36 +61,7 @@ RUN curl --proto '=https' --tlsv1.2 -sSf
https://sh.rustup.rs | sh -s -- -y
ENV PATH="/root/.cargo/bin:${PATH}"
RUN cargo install cargo2junit
-# Stage to add OSXCross if MacOSSDK is provided
-FROM base AS with-macos-sdk-true
-ARG MACOS_SDK
-
-COPY workdir/$MACOS_SDK /opt/xcode/
-
-RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
- rustup target add aarch64-apple-darwin; \
-elif [ "$TARGETPLATFORM" = "linux/amd64" ]; then \
- rustup target add x86_64-apple-darwin; \
-fi
-
-# Build OSXCross
-RUN cd /opt && git clone --depth 1
https://github.com/tpoechtrager/osxcross.git \
- && cd /opt/osxcross \
- && ./tools/gen_sdk_package_pbzx.sh /opt/xcode/${MACOS_SDK} \
- && cd .. \
- && cp /opt/osxcross/*.tar.xz tarballs \
- && UNATTENDED=1 ./build.sh
-ENV PATH="/opt/osxcross/target/bin:${PATH}"
-# Use osxcross toolchain for cargo
-COPY workdir/cargo.config /root/.cargo/config
-ENV HAS_OSXCROSS="true"
-
-# Placeholder Stage if MacOSSDK is not provided
-FROM base AS with-macos-sdk-false
-RUN echo "Building without MacOS"
-
-
-FROM with-macos-sdk-${HAS_MACOS_SDK} AS final
+FROM base AS final
Review Comment:
Nit: only one stage is left, so this could be folded into the `FROM
ubuntu:20.04` line.
--
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]
