Skip to content

Commit

Permalink
Merge remote-tracking branch 'apache/main' into fuzz-test
Browse files Browse the repository at this point in the history
  • Loading branch information
andygrove committed Feb 15, 2024
2 parents 64748e4 + 24c1c90 commit eda04f9
Show file tree
Hide file tree
Showing 21 changed files with 870 additions and 26 deletions.
44 changes: 44 additions & 0 deletions .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

name: Bug report
description: Create a bug report
labels: bug
body:
- type: textarea
attributes:
label: Describe the bug
description: Describe the bug.
placeholder: >
A clear and concise description of what the bug is.
validations:
required: true
- type: textarea
attributes:
label: Steps to reproduce
placeholder: >
Describe steps to reproduce the bug:
- type: textarea
attributes:
label: Expected behavior
placeholder: >
A clear and concise description of what you expected to happen.
- type: textarea
attributes:
label: Additional context
placeholder: >
Add any other context about the problem here.
38 changes: 38 additions & 0 deletions .github/ISSUE_TEMPLATE/feature_request.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

name: Feature request
description: Suggest an idea for this project
labels: enhancement
body:
- type: textarea
attributes:
label: What is the problem the feature request solves?
description: Please describe how feature request improves the Comet.
placeholder: >
A clear and concise description of what the improvement is. Ex. I'm always frustrated when [...]
(This section helps Comet developers understand the context and *why* for this feature, in addition to the *what*)
- type: textarea
attributes:
label: Describe the potential solution
placeholder: >
A clear and concise description of what you want to happen.
- type: textarea
attributes:
label: Additional context
placeholder: >
Add any other context or screenshots about the feature request here.
44 changes: 44 additions & 0 deletions .github/actions/setup-builder/action.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

name: Prepare Builder
description: 'Prepare Build Environment'
inputs:
rust-version:
description: 'version of rust to install (e.g. nightly)'
required: true
default: 'nightly'
jdk-version:
description: 'jdk version to install (e.g., 17)'
required: true
default: '17'
runs:
using: "composite"
steps:
- name: Install Build Dependencies
shell: bash
run: |
apt-get update
apt-get install -y openjdk-${{inputs.jdk-version}}-jdk protobuf-compiler
- name: Setup Rust toolchain
shell: bash
# rustfmt is needed for the substrait build script
run: |
echo "Installing ${{inputs.rust-version}}"
rustup toolchain install ${{inputs.rust-version}}
rustup default ${{inputs.rust-version}}
rustup component add rustfmt clippy
30 changes: 30 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->

Closes #.

## Rationale for this change

<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->

## What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->

## How are these changes tested?

<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
128 changes: 128 additions & 0 deletions .github/workflows/pr_build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

name: PR Build

concurrency:
group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}
cancel-in-progress: true

on:
push:
paths-ignore:
- "doc/**"
- "**.md"
pull_request:
paths-ignore:
- "doc/**"
- "**.md"
# manual trigger
# https://docs.github.com/en/actions/managing-workflow-runs/manually-running-a-workflow
workflow_dispatch:

env:
JAVA_VERSION: 17

jobs:
linux-rust-test:
name: Rust test (amd64)
runs-on: ubuntu-latest
container:
image: amd64/rust
steps:
- uses: actions/checkout@v4
- name: Setup Rust & Java toolchain
uses: ./.github/actions/setup-builder
with:
rust-version: nightly
jdk-version: ${{env.JAVA_VERSION}}

- name: Check cargo fmt
run: |
cd core
cargo fmt --all -- --check --color=never
- name: Check cargo clippy
run: |
cd core
cargo clippy --color=never -- -D warnings
- name: Check compilation
run: |
cd core
cargo check --benches
- name: Setup JAVA_HOME
run: |
echo "JAVA_HOME=/usr/lib/jvm/java-${{env.JAVA_VERSION}}-openjdk-amd64" >> $GITHUB_ENV
- name: Cache Maven dependencies
uses: actions/cache@v4
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-maven-
- name: Build common module (pre-requisite for Rust tests)
run: |
cd common
../mvnw clean compile -DskipTests
- name: Run cargo test
run: |
cd core
# This is required to run some JNI related tests on the Rust side
RUST_BACKTRACE=1 LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$JAVA_HOME/lib:$JAVA_HOME/lib/server cargo test
linux-java-test:
name: Java test (amd64)
runs-on: ubuntu-latest
container:
image: amd64/rust
steps:
- uses: actions/checkout@v4
- name: Setup Rust & Java toolchain
uses: ./.github/actions/setup-builder
with:
rust-version: nightly
jdk-version: ${{env.JAVA_VERSION}}

- name: Run cargo build
run: |
cd core
cargo build
- name: Setup JAVA_HOME
run: |
echo "JAVA_HOME=/usr/lib/jvm/java-${{env.JAVA_VERSION}}-openjdk-amd64" >> $GITHUB_ENV
- name: Cache Maven dependencies
uses: actions/cache@v4
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-maven-
- name: Run Maven compile
run: |
./mvnw compile test-compile scalafix:scalafix -Psemanticdb
- name: Run tests
run: |
SPARK_HOME=`pwd` ./mvnw clean install
Binary file added .mvn/wrapper/maven-wrapper.jar
Binary file not shown.
19 changes: 19 additions & 0 deletions .mvn/wrapper/maven-wrapper.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.2.0/maven-wrapper-3.2.0.jar
20 changes: 10 additions & 10 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,27 @@ all: core jvm
core:
cd core && cargo build
jvm:
mvn clean package -DskipTests $(PROFILES)
./mvnw clean package -DskipTests $(PROFILES)
test:
mvn clean
./mvnw clean
# We need to compile CometException so that the cargo test can pass
mvn compile -pl common -DskipTests $(PROFILES)
./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
SPARK_HOME=`pwd` COMET_CONF_DIR=$(shell pwd)/conf RUST_BACKTRACE=1 mvn verify $(PROFILES)
SPARK_HOME=`pwd` COMET_CONF_DIR=$(shell pwd)/conf RUST_BACKTRACE=1 ./mvnw verify $(PROFILES)
clean:
cd core && cargo clean
mvn clean
./mvnw clean
rm -rf .dist
bench:
cd core && 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 \
RUSTFLAGS="-Ctarget-cpu=native" cargo bench $(filter-out $@,$(MAKECMDGOALS))
format:
mvn compile test-compile scalafix:scalafix -Psemanticdb $(PROFILES)
mvn spotless:apply $(PROFILES)
./mvnw compile test-compile scalafix:scalafix -Psemanticdb $(PROFILES)
./mvnw spotless:apply $(PROFILES)

core-amd64:
rustup target add x86_64-apple-darwin
Expand Down Expand Up @@ -75,11 +75,11 @@ release-linux: clean
cd core && 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 --features nightly --release
cd core && RUSTFLAGS="-Ctarget-cpu=skylake -Ctarget-feature=-prefer-256-bit" CC=o64-clang CXX=o64-clang++ cargo build --target x86_64-apple-darwin --features nightly --release
cd core && RUSTFLAGS="-Ctarget-cpu=native -Ctarget-feature=-prefer-256-bit" cargo build --features nightly --release
mvn install -Prelease -DskipTests $(PROFILES)
./mvnw install -Prelease -DskipTests $(PROFILES)
release:
cd core && RUSTFLAGS="-Ctarget-cpu=native" cargo build --features nightly --release
mvn install -Prelease -DskipTests $(PROFILES)
./mvnw install -Prelease -DskipTests $(PROFILES)
benchmark-%: clean release
cd spark && COMET_CONF_DIR=$(shell pwd)/conf MAVEN_OPTS='-Xmx20g' .mvn exec:java -Dexec.mainClass="$*" -Dexec.classpathScope="test" -Dexec.cleanupDaemonThreads="false" -Dexec.args="$(filter-out $@,$(MAKECMDGOALS))" $(PROFILES)
cd spark && COMET_CONF_DIR=$(shell pwd)/conf MAVEN_OPTS='-Xmx20g' ../mvnw exec:java -Dexec.mainClass="$*" -Dexec.classpathScope="test" -Dexec.cleanupDaemonThreads="false" -Dexec.args="$(filter-out $@,$(MAKECMDGOALS))" $(PROFILES)
.DEFAULT:
@: # ignore arguments provided to benchmarks e.g. "make benchmark-foo -- --bar", we do not want to treat "--bar" as target
1 change: 1 addition & 0 deletions core/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ mod tests {
/// See [`object_panic_exception`] for a test which involves generating a panic and verifying
/// that the resulting stack trace includes the offending call.
#[test]
#[ignore]
pub fn stacktrace_string() {
// Setup: Start with a backtrace that includes all of the expected scenarios, including
// cases where the file and location are not provided as part of the backtrace capture
Expand Down
9 changes: 5 additions & 4 deletions dev/scalastyle-config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ This file is divided into 3 sections:
<parameter name="groups">java,scala,org,apache,3rdParty,comet</parameter>
<parameter name="group.java">javax?\..*</parameter>
<parameter name="group.scala">scala\..*</parameter>
<parameter name="group.org">org\..*</parameter>
<parameter name="group.apache">org\.apache\..*</parameter>
<parameter name="group.3rdParty">(?!org\.apache\.comet\.).*</parameter>
<parameter name="group.org">org\.(?!apache\.comet).*</parameter>
<parameter name="group.apache">org\.apache\.(?!comet).*</parameter>
<parameter name="group.3rdParty">(?!(javax?\.|scala\.|org\.apache\.comet\.)).*</parameter>
<parameter name="group.comet">org\.apache\.comet\..*</parameter>
</parameters>
</check>
Expand Down Expand Up @@ -301,7 +301,8 @@ This file is divided into 3 sections:
</check>

<check level="error" class="org.scalastyle.scalariform.NoWhitespaceBeforeLeftBracketChecker" enabled="true"></check>
<check level="error" class="org.scalastyle.scalariform.NoWhitespaceAfterLeftBracketChecker" enabled="true"></check>
<!-- This rule conflicts with spotless, so disabling it for now -->
<check level="error" class="org.scalastyle.scalariform.NoWhitespaceAfterLeftBracketChecker" enabled="false"></check>

<!-- This breaks symbolic method names so we don't turn it on. -->
<!-- Maybe we should update it to allow basic symbolic names, and then we are good to go. -->
Expand Down
Loading

0 comments on commit eda04f9

Please sign in to comment.