You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/04/28 11:30:37 UTC
[GitHub] [thrift] Jimexist opened a new pull request, #2593: THRIFT-5564: setup java and kotlin lib building
Jimexist opened a new pull request, #2593:
URL: https://github.com/apache/thrift/pull/2593
<!-- Explain the changes in the pull request below: -->
setup java and kotlin lib building
this is a second step after:
- #2592
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [ ] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
- [ ] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [ ] Did you squash your changes to a single commit? (not required, but preferred)
- [ ] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
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: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on a diff in pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2593:
URL: https://github.com/apache/thrift/pull/2593#discussion_r861159853
##########
.github/workflows/build.yml:
##########
@@ -45,3 +45,111 @@ jobs:
- name: Run thrift version
run: /usr/local/bin/thrift -version
+
+ # only upload while building ubuntu-20.04
+ - name: Archive built thrift compiler
+ if: matrix.os == 'ubuntu-20.04'
+ uses: actions/upload-artifact@v3
+ with:
+ name: thrift-compiler
+ path: compiler/cpp/thrift
+ retention-days: 3
+
+ build-java-and-kotlin:
+ needs: build-thrift-compiler-ubuntu
+ strategy:
+ matrix:
+ java: [8, 11]
+ distribution: ['temurin']
+ os: [ubuntu-20.04]
+ runs-on: ${{ matrix.os }}
+ env:
+ GRADLE_VERSION: 6.9.2
+ steps:
+ - uses: actions/checkout@v3
+
+ - uses: actions/setup-java@v3
+ with:
+ distribution: ${{ matrix.distribution }}
+ java-version: ${{ matrix.java }}
+ cache: 'gradle'
+
+ - name: Install dependencies
+ run: |
+ # https://docs.github.com/en/actions/using-github-hosted-runners/customizing-github-hosted-runners
Review Comment:
This step is going to occur on each job. You could consolidate the installation of dependencies into a script that is called in each job, rather than inline the script, which results in a lot of duplicate apt-get stuff.
##########
.github/workflows/build.yml:
##########
@@ -45,3 +45,111 @@ jobs:
- name: Run thrift version
run: /usr/local/bin/thrift -version
+
+ # only upload while building ubuntu-20.04
+ - name: Archive built thrift compiler
+ if: matrix.os == 'ubuntu-20.04'
+ uses: actions/upload-artifact@v3
+ with:
+ name: thrift-compiler
+ path: compiler/cpp/thrift
+ retention-days: 3
+
+ build-java-and-kotlin:
+ needs: build-thrift-compiler-ubuntu
+ strategy:
+ matrix:
+ java: [8, 11]
+ distribution: ['temurin']
+ os: [ubuntu-20.04]
+ runs-on: ${{ matrix.os }}
+ env:
+ GRADLE_VERSION: 6.9.2
+ steps:
+ - uses: actions/checkout@v3
+
+ - uses: actions/setup-java@v3
+ with:
+ distribution: ${{ matrix.distribution }}
+ java-version: ${{ matrix.java }}
+ cache: 'gradle'
+
+ - name: Install dependencies
+ run: |
+ # https://docs.github.com/en/actions/using-github-hosted-runners/customizing-github-hosted-runners
+ sudo apt-get update -yq
+ sudo apt-get install -y --no-install-recommends \
+ automake \
+ bison \
+ flex \
+ g++ \
+ git \
+ libboost-all-dev \
+ libevent-dev \
+ libssl-dev \
+ libtool \
+ make \
+ pkg-config \
+ wget \
+ unzip \
+ curl \
+ ant \
+ maven
+
+ - name: Setup gradle
+ run: |
+ wget https://services.gradle.org/distributions/gradle-$GRADLE_VERSION-bin.zip -q -O /tmp/gradle-$GRADLE_VERSION-bin.zip
+ (echo "8b356fd8702d5ffa2e066ed0be45a023a779bba4dd1a68fd11bc2a6bdc981e8f /tmp/gradle-$GRADLE_VERSION-bin.zip" | sha256sum -c -)
+ unzip -d /tmp /tmp/gradle-$GRADLE_VERSION-bin.zip
+ sudo mv /tmp/gradle-$GRADLE_VERSION /usr/local/gradle
+ sudo ln -s /usr/local/gradle/bin/gradle /usr/local/bin
+ gradle --version
+
+ - name: Run bootstrap
+ run: ./bootstrap.sh
+
+ - name: Run configure
+ run: |
+ ./configure \
+ --disable-debug \
+ --disable-tests \
+ --disable-dependency-tracking \
+ --without-cpp \
+ --without-c_glib \
+ --with-java \
+ --with-kotlin \
+ --without-python \
+ --without-py3 \
+ --without-ruby \
+ --without-haxe \
+ --without-netstd \
+ --without-perl \
+ --without-php \
+ --without-php_extension \
+ --without-dart \
+ --without-erlang \
+ --without-go \
+ --without-d \
+ --without-nodejs \
+ --without-nodets \
+ --without-lua \
+ --without-rs \
+ --without-swift
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: thrift-compiler
+ path: compiler/cpp/
+
+ - name: Run thrift-compiler
+ run: |
+ # we do a touch to make sure it's newer than the makefile
+ touch compiler/cpp/thrift
Review Comment:
I'm curious why this is necessary.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2593:
URL: https://github.com/apache/thrift/pull/2593#issuecomment-1112492713
I'm noticing the job name is quite long, which makes it hard to see the matrix params in the PR display. You can probably shorten it from "Build Thrift Compiler and Libraries" to just "Build" or similar.
You can also reduce the number of items in the matrix, and the output, by not making the Java distribution a matrix. It's probably sufficient to just use a single stable Java distribution. If temurin works, then just use that, but don't need to put it in the matrix.
You can also probably remove the Java 8 builds, since Java 11 should be using `--release 8` when it builds anyway, if the intent is to support Java 8. It'd be more useful to include Java 17 (which is the current LTS), and Java 11 (former LTS, still popular).
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2593:
URL: https://github.com/apache/thrift/pull/2593#issuecomment-1112800132
> It'd be more useful to include Java 17 (which is the current LTS)
for now we can't because we are on gradle 6.9; we'll have to upgrade to gradle 7 first but that in turn replies on migrating from `maven` to `maven-publish` plugin
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2593:
URL: https://github.com/apache/thrift/pull/2593#discussion_r862310191
##########
.github/workflows/build.yml:
##########
@@ -1,35 +1,28 @@
-name: 'Build Thrift Compiler and Libraries'
+name: 'Build'
+
on:
push:
branches: [ '*' ]
pull_request:
branches: [ '*' ]
+
+env:
+ BUILD_DEPS: automake bison flex g++ git libboost-all-dev libevent-dev libssl-dev libtool make pkg-config
Review Comment:
Thanks! Do you mind merging this some time later because I don't have write access
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
Jens-G commented on PR #2593:
URL: https://github.com/apache/thrift/pull/2593#issuecomment-1113850667
Just want to drop the usual reminder of maintaining the ReleaseManagement.md because we will need this.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on a diff in pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2593:
URL: https://github.com/apache/thrift/pull/2593#discussion_r862294527
##########
.github/workflows/build.yml:
##########
@@ -1,35 +1,28 @@
-name: 'Build Thrift Compiler and Libraries'
+name: 'Build'
+
on:
push:
branches: [ '*' ]
pull_request:
branches: [ '*' ]
+
+env:
+ BUILD_DEPS: automake bison flex g++ git libboost-all-dev libevent-dev libssl-dev libtool make pkg-config
Review Comment:
Clever! I didn't know about `env`
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2593:
URL: https://github.com/apache/thrift/pull/2593#issuecomment-1113879719
> Just want to drop the usual reminder of maintaining the ReleaseManagement.md because we will need this.
thanks for the reminder @Jens-G, for now this and the currently open pull requests only adds CI part, not the CD part, also the travis CI is intact - so I have no changes to the `ReleaseManagement.md` just yet. When I do later make the final switch, I'll post to dev@thrift and update accordingly. For now, there's no change needed.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii merged pull request #2593: THRIFT-5564: setup java and kotlin lib building
Posted by GitBox <gi...@apache.org>.
ctubbsii merged PR #2593:
URL: https://github.com/apache/thrift/pull/2593
--
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: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org