-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
CI: add Arm support to travis CI matrix #6436
Conversation
This patch adds arm64 to TravisCI matrix. Difference comparing to amd64: 1. For CMake, as no official arm64 release ready on Kitware page, a third party (conda-forge) released one is used instead of building from source. The main reason is to save CI time. 2. Explicit export JAVA_HOME on arm64 3. Disable mingw test Change-Id: I22fc5620f1942c341c28e4c82b3b4daaea3f9819 Signed-off-by: Yuqi Gu <yuqi.gu@arm.com>
In order to make Arm64 CI work properly, PR #6437 also needs to be merged , thanks! |
@guyuqi so this does not require to install Java 8? Which version of Java does it use? |
Hi, @adamretter , from TravisCI outputs, it has JDK-11 pre-installed: |
@JunHe77 I would prefer to stick with Java 8. |
@adamretter I'm fine to use either Java8 or Java11. But to align with x86 tests it would be good to use same Java version on Arm64. For x86 test Travis uses JDK11 preinstalled. A reference log is here. |
@JunHe77 that link for the reference log is from building Redis not RocksDB. |
Ya, different repos, but same Travis runtime. |
@JunHe77 Okay, but my point was that RocksDB's Java code is designed for compatibility with Java 7. The x64 testing of RocksDB happens against Java 7, the ARM64 testing therefore should also happen against Java 7. Unfortunately, I could not find Java 7 for ARM64, so the next best option IMHO is Java 8. |
Yes, Arm64 support in JDK starts from Java8. Java version on x86: https://travis-ci.org/facebook/rocksdb/jobs/652883614#L315 |
@JunHe77 Oh blast! I see that the RocksDB Travis CI is now using Java 11 for x64 as well - https://travis-ci.org/facebook/rocksdb/jobs/653848197 It was and still should be Java 7 AFAIK. We actually specify in our
So I am not sure why Travis is ignoring that and using Java 11. |
@siying Could you please help to loop some guys in this PR and have a look into it? |
@guyuqi I'm responsible for RocksJava so I can look into that |
From what Travis guy's reply:
So it seems extra commands need to add to .travis.yml if Java8 should be used. |
@adamretter That‘s great and look forward to your conclusion. |
898ddc8
to
02474da
Compare
@adamretter Add JDK8 installation in the new commit. |
Change-Id: I60aa077012f66b7d722874b75a26f3a4ced94b7e Signed-off-by: Yuqi Gu <yuqi.gu@arm.com>
02474da
to
911cc43
Compare
@Little-Wallace Any advice on DBCompactionTestWithParam::FlushAfterIntraL0CompactionCheckConsistencyFail failing? (Re: #6061) |
That was in TEST_GROUP=2. Last failure was something in TEST_GROUP=3 (which Travis seems to have cleared from my web browser tab, and from history, with the log open when I restarted the build in another tab 😡) |
I think same failure again: ExternalSSTFileTest.MultiThreaded... |
I haven't been able to reproduce this using the same OS and compiler version in Amazon EC2. |
@pdillinger Seems that all tests are now passing on Travis ARM64 too :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@pdillinger merged this pull request in dd7a4a8. |
This patch based on #5932 offers a better solution to add arm64 to TravisCI matrix.
Really thank @adamretter for initiating Arm CI setup.
Difference comparing to amd64:
a third party (conda-forge) released one is used instead of
building from source. The main reason is to save CI time.
Signed-off-by: Yuqi Gu yuqi.gu@arm.com