Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All changes together without offline #20504

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions .github/actions/compile-commit/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ runs:
key: compile-commit-success-${{ github.event.pull_request.head.repo.full_name }}-${{ steps.repo-hash.outputs.tree-hash }}
- uses: ./.github/actions/setup
if: steps.check-compile-commit-success.outputs.cache-hit != 'true'
with:
# Dependencies are downloaded explicitly below, after MAVEN has been set here. This allows modifications to the MAVEN setup with a PR.
download_dependencies: false
- name: Check if a specified commit compiles
shell: bash
if: steps.check-compile-commit-success.outputs.cache-hit != 'true'
Expand All @@ -35,17 +32,15 @@ runs:
# It's important that these values are NOT passed as parameters, because then their values would always be taken from PR HEAD
# -------
# allow overriding Maven command
export MAVEN="./mvnw --offline"
# maven.wagon.rto is in millis, defaults to 30m
MAVEN_INSTALL_OPTS="-Xmx3G -XX:+ExitOnOutOfMemoryError -Dmaven.wagon.rto=60000"
MAVEN_COMPILE_COMMITS="-B --strict-checksums --quiet -T 1C -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all=true -Dmaven.javadoc.skip=true --no-snapshot-updates --no-transfer-progress -pl !:trino-server-rpm"
export MAVEN="./mvnw"
MAVEN_INSTALL_OPTS="-Xmx3G -XX:+ExitOnOutOfMemoryError"
MAVEN_COMPILE_COMMITS="-B --strict-checksums --quiet -T 1C -b smart -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all=true -Dmaven.javadoc.skip=true --no-snapshot-updates --no-transfer-progress -pl !:trino-server-rpm"
export MAVEN_GIB="-P gib -Dgib.referenceBranch=refs/remotes/origin/${{ inputs.base_ref }}"
RETRY=.github/bin/retry
# -------

# For building with Maven we need MAVEN_OPTS to equal MAVEN_INSTALL_OPTS
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
./.github/bin/download-maven-dependencies.sh
$MAVEN package \
${MAVEN_COMPILE_COMMITS} `# defaults, kept in sync with ci.yml` \
-Dair.check.skip-all=false -Dair.check.skip-basic=true -Dair.check.skip-extended=true -Dair.check.skip-checkstyle=false \
Expand Down
9 changes: 1 addition & 8 deletions .github/actions/setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ inputs:
cleanup-node:
description: "Clean up node (true/false) to increase free disk space"
default: false # Disabled by default as it adds ~4 minutes of test runtime. Should be enabled case by case.
electrum marked this conversation as resolved.
Show resolved Hide resolved
download_dependencies:
description: "Download all Maven dependencies so Maven can work in offline mode"
default: true

runs:
using: composite
Expand Down Expand Up @@ -50,7 +47,7 @@ runs:
if: ${{ format('{0}', inputs.cache) == 'true' }}
uses: actions/cache@v4
with:
path: |
path: |
~/.m2/repository
/tmp/pt_java_downloads
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
Expand All @@ -65,10 +62,6 @@ runs:
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-maven-
- name: Fetch any missing dependencies
shell: bash
if: ${{ format('{0}', inputs.download_dependencies) == 'true' }}
run: ./.github/bin/download-maven-dependencies.sh
- name: Configure Problem Matchers
if: ${{ inputs.java-version != '' }}
shell: bash
Expand Down
23 changes: 0 additions & 23 deletions .github/bin/download-maven-dependencies.sh

This file was deleted.

29 changes: 15 additions & 14 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@ env:
# An envar that signals to tests we are executing in the CI environment
CONTINUOUS_INTEGRATION: true
# allow overriding Maven command
MAVEN: ./mvnw --offline
# maven.wagon.rto is in millis, defaults to 30m
MAVEN_OPTS: "-Xmx512M -XX:+ExitOnOutOfMemoryError -Dmaven.wagon.rto=60000"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why -Xmx512M -XX:+ExitOnOutOfMemoryError was removed?
is this related to removal of the go-offline?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAVEN_INSTALL_OPTS: "-Xmx3G -XX:+ExitOnOutOfMemoryError -Dmaven.wagon.rto=60000"
MAVEN_FAST_INSTALL: "-B --strict-checksums -V --quiet -T 1C -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all"
MAVEN_COMPILE_COMMITS: "-B --strict-checksums --quiet -T 1C -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all=true -Dmaven.javadoc.skip=true --no-snapshot-updates --no-transfer-progress -pl '!:trino-server-rpm'"
MAVEN: ./mvnw
MAVEN_OPTS: ""
MAVEN_INSTALL_OPTS: ""
MAVEN_FAST_INSTALL: "-B -V --quiet -T 1C -b smart -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all"
MAVEN_COMPILE_COMMITS: "-B --quiet -T 1C -b smart -DskipTests -Dmaven.source.skip=true -Dair.check.skip-all=true -Dmaven.javadoc.skip=true --no-snapshot-updates --no-transfer-progress -pl '!:trino-server-rpm'"
MAVEN_GIB: "-P gib -Dgib.referenceBranch=refs/remotes/origin/${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || github.event.repository.default_branch }}"
MAVEN_TEST: "-B --strict-checksums -Dmaven.source.skip=true -Dair.check.skip-all --fail-at-end -P gib -Dgib.referenceBranch=refs/remotes/origin/${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || github.event.repository.default_branch }}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is --strict-checksums no longer necessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAVEN_TEST: "-B -Dmaven.source.skip=true -Dair.check.skip-all --fail-at-end -P gib -Dgib.referenceBranch=refs/remotes/origin/${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || github.event.repository.default_branch }}"
# Testcontainers kills image pulls if they don't make progress for > 30s and retries for 2m before failing. This means
# that if an image doesn't download all it's layers within ~2m then any other concurrent pull will be killed because
# the Docker daemon only downloads 3 layers concurrently which prevents the other pull from making any progress.
Expand Down Expand Up @@ -68,17 +67,18 @@ jobs:
format('refs/pull/{0}/head', github.event.client_payload.pull_request.number) || '' }}
- uses: ./.github/actions/setup
with:
cache: 'restore'
java-version: ${{ matrix.java-version }}
cleanup-node: true
- name: Check SPI backward compatibility
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
$MAVEN clean install ${MAVEN_FAST_INSTALL} -pl :trino-spi -am
${MAVEN//--offline/} clean verify -B --strict-checksums -DskipTests -pl :trino-spi
$MAVEN clean verify -B --strict-checksums -DskipTests -pl :trino-spi
- name: Maven Checks
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
$MAVEN clean verify -B --strict-checksums -V -T 1C -DskipTests -P ci -pl '!:trino-server-rpm'
$MAVEN clean verify -B --strict-checksums -V -T 1C -b smart -DskipTests -P ci -pl '!:trino-server-rpm'
- name: Remove Trino from local Maven repo to avoid caching it
# Avoid caching artifacts built in this job, cache should only include dependencies
if: steps.cache.outputs.cache-hit != 'true'
Expand All @@ -97,7 +97,8 @@ jobs:
format('refs/pull/{0}/head', github.event.client_payload.pull_request.number) || '' }}
- uses: ./.github/actions/setup
with:
cleanup-node: true
cache: 'restore'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like no job populates the cache anymore, all uses of ./.github/actions/setup are done with cache: restore.

cleanup-node: 'true'
- name: Maven Install
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
Expand Down Expand Up @@ -154,8 +155,9 @@ jobs:
echo "matrix=$(jq -c '.' commit-matrix.json)" >> $GITHUB_OUTPUT

check-commit:
needs:
- check-commits-dispatcher
runs-on: ubuntu-latest
needs: check-commits-dispatcher
if: github.event_name == 'pull_request' && needs.check-commits-dispatcher.outputs.matrix != ''
strategy:
fail-fast: false
Expand Down Expand Up @@ -197,7 +199,7 @@ jobs:
- name: Error Prone Checks
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
$MAVEN ${MAVEN_TEST} -T 1C clean verify -DskipTests ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \
$MAVEN ${MAVEN_TEST} -T 1C -b smart clean verify -DskipTests ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \
-pl '!:trino-docs,!:trino-server,!:trino-server-rpm'

web-ui-checks:
Expand Down Expand Up @@ -958,8 +960,7 @@ jobs:
format('refs/pull/{0}/head', github.event.client_payload.pull_request.number) || '' }}
- uses: ./.github/actions/setup
with:
cache: false
download_dependencies: false
cache: restore
- name: Product tests artifact
uses: actions/download-artifact@v4
with:
Expand Down
6 changes: 3 additions & 3 deletions .mvn/extensions.xml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<extensions>
<extension>
<groupId>io.takari.aether</groupId>
<artifactId>takari-local-repository</artifactId>
<version>0.11.3</version>
<groupId>io.takari.maven</groupId>
<artifactId>takari-smart-builder</artifactId>
<version>0.6.4</version>
</extension>
</extensions>
1 change: 1 addition & 0 deletions .mvn/jvm.config
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED
-XX:+UnlockDiagnosticVMOptions
-XX:GCLockerRetryAllocationCount=100
-XX:+ExitOnOutOfMemoryError
10 changes: 10 additions & 0 deletions .mvn/maven.config
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
--strict-checksums
# these below are documented here https://maven.apache.org/resolver/configuration.html
-Daether.syncContext.named.factory=file-lock
-Daether.syncContext.named.nameMapper=file-hgav
-Daether.syncContext.named.retry=5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any reference docs for these properties, does this mean downloads will be retried?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, can you add it in a comment in this file? I see aether.connector.http.retryHandler.count has a default value of 3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these retries always available in Maven?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Maven 3.9.x, with introduction of locking

Copy link
Member Author

@cstamas cstamas Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Resolver 2 this page is generated and contains more data, like "since" and "source": https://maven.apache.org/resolver-archives/resolver-2.0.0-alpha-7/configuration.html

BUT, do not use this page are reference for Resolver 1.x (Maven 3.x) as there has been renamed property keys! Still, if you can match the property (unchanged since 1.x or figure out what was renamed from) you can figure out "since".

-Daether.dependencyCollector.impl=bf
-Daether.remoteRepositoryFilter.prefixes=true
-Daether.remoteRepositoryFilter.prefixes.basedir=${session.rootDirectory}/.mvn/rrf/
-Daether.remoteRepositoryFilter.groupId=true
-Daether.remoteRepositoryFilter.groupId.basedir=${session.rootDirectory}/.mvn/rrf/

Empty file.
3 changes: 3 additions & 0 deletions .mvn/rrf/groupId-confluent.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
io.confluent
org.apache.kafka

Empty file.
Empty file.
Empty file added .mvn/rrf/groupId-jitpack.io.txt
Empty file.
Empty file.
Empty file added .mvn/rrf/groupId-ossrh.txt
Empty file.
Empty file.
5 changes: 5 additions & 0 deletions core/trino-server/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
<!-- Launcher properties -->
<main-class>io.trino.server.TrinoServer</main-class>
<process-name>${project.artifactId}</process-name>

<!-- Special consideration for Takari Lifecycle -->
<!-- This works as trino-server have no sources (is just provisio packaged) -->
<takari.skip>false</takari.skip>
</properties>

<dependencies>
Expand Down Expand Up @@ -64,6 +68,7 @@
<version>${dep.takari.version}</version>
<configuration>
<proc>none</proc>
<skip>${takari.skip}</skip>
</configuration>
</plugin>
</plugins>
Expand Down
Loading
Loading