Skip to content

Commit

Permalink
build: Make the build system work out of box
Browse files Browse the repository at this point in the history
This commit also updates the DEVELOPMENT.md to help
new developers set up the project in IDEs.
  • Loading branch information
advancedxy committed Feb 29, 2024
1 parent e2a6aca commit 32be12d
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 35 deletions.
13 changes: 2 additions & 11 deletions .github/actions/java-test/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,9 @@ runs:
- name: Run Maven compile
shell: bash
run: |
echo "JAVA_VERSION=${JAVA_VERSION}"
if [ $JAVA_VERSION == "1.8" ]; then
./mvnw -B compile test-compile scalafix:scalafix -Psemanticdb -Djava.version=${JAVA_VERSION} -Dspotless.version=2.29.0
else
./mvnw -B compile test-compile scalafix:scalafix -Psemanticdb -Djava.version=${JAVA_VERSION}
fi
./mvnw -B compile test-compile scalafix:scalafix -Psemanticdb
- name: Run tests
shell: bash
run: |
if [ $JAVA_VERSION == "1.8" ]; then
SPARK_HOME=`pwd` ./mvnw -B clean install -Djava.version=${JAVA_VERSION} -Dspotless.version=2.29.0
else
SPARK_HOME=`pwd` ./mvnw -B clean install -Djava.version=${JAVA_VERSION}
fi
SPARK_HOME=`pwd` ./mvnw -B clean install
9 changes: 1 addition & 8 deletions .github/actions/rust-test/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ runs:
shell: bash
run: |
cd common
if [ $JAVA_VERSION == "1.8" ]; then
../mvnw -B clean compile -DskipTests -Djava.version=${JAVA_VERSION} -Dspotless.version=2.29.0
else
../mvnw -B clean compile -DskipTests -Djava.version=${JAVA_VERSION}
fi
../mvnw -B clean compile -DskipTests
- name: Run Cargo test
shell: bash
Expand All @@ -64,9 +60,6 @@ runs:
# This is required to run some JNI related tests on the Rust side
JAVA_LD_LIBRARY_PATH=$JAVA_HOME/lib/server:$JAVA_HOME/lib:$JAVA_HOME/lib/jli
# special handing for java 1.8 for both linux and mac distributions
if [ $JAVA_VERSION == "8" ]; then
JAVA_LD_LIBRARY_PATH=$JAVA_HOME/jre/lib/amd64/server:$JAVA_HOME/jre/lib/amd64:$JAVA_HOME/jre/lib/amd64/jli:$JAVA_HOME/jre/lib/server:$JAVA_HOME/jre/lib:$JAVA_HOME/jre/lib/jli
fi
RUST_BACKTRACE=1 \
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$JAVA_LD_LIBRARY_PATH \
DYLD_LIBRARY_PATH=$DYLD_LIBRARY_PATH:$JAVA_LD_LIBRARY_PATH \
Expand Down
6 changes: 0 additions & 6 deletions .github/workflows/pr_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ jobs:
runs-on: ${{ matrix.os }}
container:
image: amd64/rust
env:
JAVA_VERSION: ${{ matrix.java_version == 8 && '1.8' || format('{0}', matrix.java_version) }}
steps:
- uses: actions/checkout@v4
- name: Setup Rust & Java toolchain
Expand All @@ -82,8 +80,6 @@ jobs:
if: github.event_name == 'push'
name: ${{ matrix.test-target }} test on ${{ matrix.os }} with java ${{ matrix.java_version }}
runs-on: ${{ matrix.os }}
env:
JAVA_VERSION: ${{ matrix.java_version == 8 && '1.8' || format('{0}', matrix.java_version) }}
steps:
- uses: actions/checkout@v4
- name: Setup Rust & Java toolchain
Expand Down Expand Up @@ -113,8 +109,6 @@ jobs:
fail-fast: false
name: ${{ matrix.test-target }} test on macos-aarch64 with java ${{ matrix.java_version }}
runs-on: macos-14
env:
JAVA_VERSION: ${{ matrix.java_version == 8 && '1.8' || format('{0}', matrix.java_version) }}
steps:
- uses: actions/checkout@v4
- name: Setup Rust & Java toolchain
Expand Down
23 changes: 22 additions & 1 deletion DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ under the License.

## Development Setup

1. Make sure `JAVA_HOME` is set and point to JDK 11 installation.
1. Make sure `JAVA_HOME` is set and point to JDK 8/11/17 installation.
2. Install Rust toolchain. The easiest way is to use
[rustup](https://rustup.rs).

Expand All @@ -39,6 +39,8 @@ under the License.
A few common commands are specified in project's `Makefile`:

- `make`: compile the entire project, but don't run tests
- `make test-rust`: compile the project and run tests in Rust side
- `make test-java`: compile the project and run tests in Java side
- `make test`: compile the project and run tests in both Rust and Java
side.
- `make release`: compile the project and creates a release build. This
Expand All @@ -47,6 +49,25 @@ A few common commands are specified in project's `Makefile`:
- `make clean`: clean up the workspace
- `bin/comet-spark-shell -d . -o spark/target/` run Comet spark shell for V1 datasources
- `bin/comet-spark-shell -d . -o spark/target/ --conf spark.sql.sources.useV1SourceList=""` run Comet spark shell for V2 datasources

## Opening Project in IDEs
Comet is a multi-language project with native code written in Rust and JVM code written in Java and Scala.
For Rust code, the CLion IDE is recommended. For JVM code, IntelliJ IDEA is recommended.

Before opening the project in an IDE, make sure to run `make` first to generate the necessary files for the IDEs. Currently, it's mostly about
generating protobuf message classes for the JVM side. It's only required to run `make` once after cloning the repo.

### IntelliJ IDEA
First make sure to install the Scala plugin in IntelliJ IDEA.
After that, you can open the project in IntelliJ IDEA. The IDE should automatically detect the project structure and import as a Maven project.

### CLion
First make sure to install the Rust plugin in CLion or you can use the dedicated Rust IDE: RustRover.
After that you can open the project in CLion. The IDE should automatically detect the project structure and import as a Cargo project.

### Running Tests in IDEA
Like other Maven projects, you can run tests in IntelliJ IDEA by right-clicking on the test class or test method and selecting "Run" or "Debug".
However if the tests is related to the native side. Please make sure to run `make core` or `cd core && cargo build` before running the tests in IDEA.

## Benchmark

Expand Down
9 changes: 9 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,17 @@ all: core jvm

core:
cd core && cargo build
test-rust:
# We need to compile CometException so that the cargo test can pass
./mvnw compile -pl common -DskipTests $(PROFILES)
cd core && cargo build && \
LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+${LD_LIBRARY_PATH}:}${JAVA_HOME}/lib:${JAVA_HOME}/lib/server:${JAVA_HOME}/lib/jli && \
DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH:+${DYLD_LIBRARY_PATH}:}${JAVA_HOME}/lib:${JAVA_HOME}/lib/server:${JAVA_HOME}/lib/jli \
RUST_BACKTRACE=1 cargo test
jvm:
./mvnw clean package -DskipTests $(PROFILES)
test-jvm: core
SPARK_HOME=`pwd` COMET_CONF_DIR=$(shell pwd)/conf RUST_BACKTRACE=1 ./mvnw verify $(PROFILES)
test:
./mvnw clean
# We need to compile CometException so that the cargo test can pass
Expand Down
46 changes: 37 additions & 9 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -492,9 +492,6 @@ under the License.
<spark.version>3.2.2</spark.version>
<spark.version.short>3.2</spark.version.short>
<parquet.version>1.12.0</parquet.version>
<java.version>1.8</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
</properties>
</profile>

Expand All @@ -505,9 +502,6 @@ under the License.
<spark.version>3.3.2</spark.version>
<spark.version.short>3.3</spark.version.short>
<parquet.version>1.12.0</parquet.version>
<java.version>11</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
</properties>
</profile>

Expand All @@ -517,9 +511,6 @@ under the License.
<scala.version>2.12.17</scala.version>
<spark.version.short>3.4</spark.version.short>
<parquet.version>1.13.1</parquet.version>
<java.version>11</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
</properties>
</profile>

Expand All @@ -531,6 +522,43 @@ under the License.
</properties>
</profile>

<profile>
<id>jdk1.8</id>
<activation>
<jdk>1.8</jdk>
</activation>
<properties>
<java.version>1.8</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
<spotless.version>2.29.0</spotless.version>
</properties>
</profile>

<profile>
<id>jdk11</id>
<activation>
<jdk>11</jdk>
</activation>
<properties>
<java.version>11</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
</properties>
</profile>

<profile>
<id>jdk17</id>
<activation>
<jdk>17</jdk>
</activation>
<properties>
<java.version>17</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
</properties>
</profile>

<profile>
<id>semanticdb</id>
<properties>
Expand Down

0 comments on commit 32be12d

Please sign in to comment.