Skip to content

ARROW-9701: [CI][Java] Add a job for s390x Java on TravisCI#7938

Closed
kiszk wants to merge 12 commits intoapache:masterfrom
kiszk:ARROW-9701
Closed

ARROW-9701: [CI][Java] Add a job for s390x Java on TravisCI#7938
kiszk wants to merge 12 commits intoapache:masterfrom
kiszk:ARROW-9701

Conversation

@kiszk
Copy link
Member

@kiszk kiszk commented Aug 12, 2020

This PR adds a new test job for big-endian Java environment to TravisCI. It still keeps as

   allow_failures:
     - arch: s390x

@github-actions
Copy link

github-actions bot commented Aug 12, 2020

https://issues.apache.org/jira/browse/ARROW-9701

kou
kou reviewed Aug 12, 2020
View reviewed changes
.travis.yml Outdated
Copy link
Member

@kou kou Aug 12, 2020

Choose a reason for hiding this comment

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

This is needless because we don't use Ubuntu based docker image.

.travis.yml Outdated
Copy link
Member

@kou kou Aug 12, 2020

Choose a reason for hiding this comment

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

This is needless because ARCH is already passed as environment variable.

.travis.yml Outdated
Copy link
Member

@kou kou Aug 12, 2020

Choose a reason for hiding this comment

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

How about using a common script for all jobs like the following?

diff --git a/.travis.yml b/.travis.yml
index fa5d84c82..551d0a029 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -47,15 +47,23 @@ jobs:
       env:
         ARCH: s390x
         ARROW_CI_MODULES: "CPP"
-        ARROW_FLIGHT: "ON"
-        ARROW_PARQUET: "OFF"
         DOCKER_IMAGE_ID: ubuntu-cpp
-        PARQUET_BUILD_EXAMPLES: "OFF"
-        PARQUET_BUILD_EXECUTABLES: "OFF"
-        Protobuf_SOURCE: "BUNDLED"
+        DOCKER_RUN_ARGS: >-
+          -e ARROW_FLIGHT=ON
+          -e ARROW_PARQUET=OFF
+          -e PARQUET_BUILD_EXAMPLES=OFF
+          -e PARQUET_BUILD_EXECUTABLES=OFF
+          -e Protobuf_SOURCE=BUNDLED
+          -e cares_SOURCE=BUNDLED
+          -e gRPC_SOURCE=BUNDLED
         UBUNTU: "20.04"
-        cares_SOURCE: "BUNDLED"
-        gRPC_SOURCE: "BUNDLED"
+    - name: "Java on s390x"
+      os: linux
+      arch: s390x
+      env:
+        ARCH: s390x
+        ARROW_CI_MODULES: "JAVA"
+        DOCKER_IMAGE_ID: debian-java
   allow_failures:
     - arch: s390x
 
@@ -89,13 +97,7 @@ script:
     ulimit -c unlimited || :
   - |
     archery docker run \
-      -e ARROW_FLIGHT=${ARROW_FLIGHT:-OFF} \
-      -e ARROW_PARQUET=${ARROW_PARQUET:-ON} \
-      -e PARQUET_BUILD_EXAMPLES=${PARQUET_BUILD_EXAMPLES:-ON} \
-      -e PARQUET_BUILD_EXECUTABLES=${PARQUET_BUILD_EXECUTABLES:-ON} \
-      -e Protobuf_SOURCE=${Protobuf_SOURCE:-} \
-      -e cares_SOURCE=${cares_SOURCE:-} \
-      -e gRPC_SOURCE=${gRPC_SOURCE:-} \
+      ${DOCKER_RUN_ARGS} \
       --volume ${PWD}/build:/build \
       ${DOCKER_IMAGE_ID}

ci/scripts/java_build.sh Outdated
Copy link
Member

@kou kou Aug 12, 2020

Choose a reason for hiding this comment

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

Suggested change
grp="com.github.icexelloss"
group="com.github.icexelloss"

ci/scripts/java_build.sh Outdated
Copy link
Member

@kou kou Aug 12, 2020

Choose a reason for hiding this comment

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

Suggested change
cls="linux-s390_64"
classifier="linux-s390_64"

ci/scripts/java_build.sh Outdated
Copy link
Member

@kou kou Aug 12, 2020

Choose a reason for hiding this comment

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

Generally, $(...) is preferred than `...`:

Suggested change
${mvn_install} -DgroupId=${grp} -DartifactId=${artifact} -Dversion=${ver} -Dpackaging=${extension} -Dfile=`pwd`/${target}
${mvn_install} -DgroupId=${grp} -DartifactId=${artifact} -Dversion=${ver} -Dpackaging=${extension} -Dfile=$(pwd)/${target}

@kiszk kiszk marked this pull request as draft August 15, 2020 12:49
@kou kou mentioned this pull request Aug 19, 2020
Closed
@kiszk kiszk force-pushed the ARROW-9701 branch 2 times, most recently from ffbb910 to 6fd6dac Compare August 27, 2020 02:43
@kiszk kiszk marked this pull request as ready for review August 28, 2020 00:31
@kiszk
Copy link
Member Author

kiszk commented Aug 28, 2020

@kou Thank you. Now, a TravisCI job is finished correctly with many failures. Could you review this again?

kou
kou reviewed Aug 28, 2020
View reviewed changes
ci/scripts/java_build.sh Outdated
Copy link
Member

@kou kou Aug 28, 2020

Choose a reason for hiding this comment

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

Can we remove -Dmaven.compiler.verbose=true and remove the else clause?

Copy link
Member Author

@kiszk kiszk Aug 29, 2020

Choose a reason for hiding this comment

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

sure, done

java/pom.xml Outdated
Copy link
Member

@kou kou Aug 28, 2020

Choose a reason for hiding this comment

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

Do we still need this?

Copy link
Member Author

@kiszk kiszk Aug 29, 2020

Choose a reason for hiding this comment

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

No, use the original setting. Done

kou
kou approved these changes Aug 30, 2020
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

We hold off merging this until we come to consensus on https://lists.apache.org/thread.html/r3d4ab3ccc8db7a7a68baa727bf9b6911930b61609c0faaed14e2b773%40%3Cdev.arrow.apache.org%3E .

See also: #8047 (comment)

@kiszk
Copy link
Member Author

kiszk commented Oct 3, 2020

@emkornfield @kou Is it ok to merge this PR now? Or, do we still hold off for a while?

@emkornfield
Copy link
Contributor

emkornfield commented Oct 5, 2020

Yes, I think so. I'd still like to update the contributor guidelines to make it clear what the level of support we are aiming for in each language (I hope to get to it tonight but if I don't it might take me a few days), but I think CI should be harmless. @kou it looks like you are happy with this PR?

@kou
Copy link
Member

kou commented Oct 5, 2020

Yes.
But I want to confirm that this still works with the latest master before we merge.
@kiszk Could you rebase this on the latest master?

@kiszk
Copy link
Member Author

kiszk commented Oct 5, 2020

@emkornfield @kou Thank you. I want to merge only this PR for CI. We should still hold off other PRs.

I rebased with master branch and confirmed it works well.

@kou
Copy link
Member

kou commented Oct 5, 2020

Thanks.
I'll merge this.

@kou kou closed this in 8c75941 Oct 5, 2020
This was referenced Oct 6, 2020
Closed
Closed
kou pushed a commit that referenced this pull request Oct 6, 2020
@kiszk @kou
ARROW-10200: [CI][Java] Fix a job failure for s390x Java on TravisCI
03dea9b 
This is a follow-up of #7938. #7938 forgot downloading a required dll `libprotobuf.so.18` at the build time.

Closes #8368 from kiszk/ARROW-10200

Authored-by: Kazuaki Ishizaki <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
@asfimport asfimport mentioned this pull request Oct 5, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kou kou kou approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kiszk @emkornfield @kou