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

fix: add native image configurations for Spanner classes #1858

Merged
merged 28 commits into from
May 25, 2022

Conversation

mpeddada1
Copy link
Contributor

@mpeddada1 mpeddada1 commented Apr 27, 2022

No description provided.

@mpeddada1 mpeddada1 requested review from a team as code owners April 27, 2022 20:51
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. labels Apr 27, 2022
@mpeddada1 mpeddada1 changed the title fix: branch to test all native image configuration changes on CI fix: branch to test all native image configuration changes in one place Apr 27, 2022
@mpeddada1
Copy link
Contributor Author

First run of CI succeeded. Pulling in changes from main to verify again

@mpeddada1
Copy link
Contributor Author

Native Image tests still green after pulling in recent changes in main

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 12, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 12, 2022
@mpeddada1
Copy link
Contributor Author

Checks still passing after merging recent changes from main.

Copy link
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

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

LGTM.

@mpeddada1 : Please close the other PRs around native image testing if this is the final one to be merged.

@mpeddada1
Copy link
Contributor Author

Looks like we're experiencing some failures due to Java 8 compilation issues. Will wait for #1870 to be merged.

@ansh0l
Copy link
Contributor

ansh0l commented May 23, 2022

Closing since native image changes have been released with #1878

@ansh0l ansh0l closed this May 23, 2022
@mpeddada1
Copy link
Contributor Author

Sorry @ansh0l, I'll have to reopen this PR. We were blocked on some Java compatibility issues which was introduces with the most recent version of GraalVM so this PR is yet to be merged.

@mpeddada1 mpeddada1 reopened this May 23, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 24, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 24, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 25, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels May 25, 2022
@suztomo
Copy link
Member

suztomo commented May 25, 2022

samples / compile (8)

It failed:

2022-05-25T13:54:58.6638474Z [INFO] --- maven-compiler-plugin:3.10.1:compile (default-compile) @ google-cloud-spanner ---
2022-05-25T13:54:58.6675784Z [INFO] Changes detected - recompiling the module!
2022-05-25T13:54:58.6684037Z [INFO] Compiling 194 source files to /home/runner/work/java-spanner/java-spanner/google-cloud-spanner/target/classes
2022-05-25T13:54:58.9751742Z [INFO] -------------------------------------------------------------
2022-05-25T13:54:58.9754868Z [ERROR] COMPILATION ERROR : 
2022-05-25T13:54:58.9757245Z [INFO] -------------------------------------------------------------
2022-05-25T13:54:58.9760200Z [ERROR] /home/runner/work/java-spanner/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/nativeimage/SpannerFeature.java:[20,38] cannot access org.graalvm.nativeimage.hosted.Feature
2022-05-25T13:54:58.9761016Z   bad class file: /home/runner/.m2/repository/org/graalvm/sdk/graal-sdk/22.1.0/graal-sdk-22.1.0.jar(org/graalvm/nativeimage/hosted/Feature.class)
2022-05-25T13:54:58.9761667Z     class file has wrong version 55.0, should be 52.0
2022-05-25T13:54:58.9763912Z     Please remove or make sure it appears in the correct subdirectory of the classpath.

Checking...

The root cause is that the "samples / compile (8)" first compiles the root project (which now requires JDK 11).

  compile:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        java: [8, 11]
    steps:
    - uses: actions/checkout@v2
    - uses: actions/setup-java@v1
      with:
        java-version: ${{matrix.java}}
    - name: Compile Spanner
      run: mvn clean install
    - name: Compile samples
      run: mvn compile
      working-directory: samples

Kokoro - Test: Integration

The compile "JDK11 and running test JDK8" somehow didn't work. It tries to recompile the google-cloud-spanner module saying "Change detected":

[INFO] --- flatten-maven-plugin:1.2.7:flatten (flatten) @ google-cloud-spanner ---
[INFO] Generating flattened POM of project com.google.cloud:google-cloud-spanner:jar:6.25.2-SNAPSHOT...
[INFO] 
[INFO] --- maven-compiler-plugin:3.10.1:compile (default-compile) @ google-cloud-spanner ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 194 source files to /tmpfs/src/github/java-spanner/google-cloud-spanner/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /tmpfs/src/github/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/nativeimage/SpannerFeature.java:[20,38] cannot access org.graalvm.nativeimage.hosted.Feature
  bad class file: /root/.m2/repository/org/graalvm/sdk/graal-sdk/22.1.0/graal-sdk-22.1.0.jar(org/graalvm/nativeimage/hosted/Feature.class)
    class file has wrong version 55.0, should be 52.0

integration-tests-against-emulator / units

The maven command in .github/workflows/integration-tests-against-emulator.yaml needs to be updated.

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels May 25, 2022
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • .github/workflows/ci.yaml
  • .kokoro/common.sh
  • .kokoro/dependencies.sh

@mpeddada1
Copy link
Contributor Author

All checks are passing after #1890.

@mpeddada1 mpeddada1 changed the title fix: branch to test all native image configuration changes in one place fix: add native image configurations for Spanner classes May 25, 2022
@mpeddada1 mpeddada1 merged commit 92d0292 into main May 25, 2022
@mpeddada1 mpeddada1 deleted the test-compatibility branch May 25, 2022 23:31
gcf-merge-on-green bot pushed a commit that referenced this pull request May 26, 2022
🤖 I have created a release *beep* *boop*
---


### [6.25.3](v6.25.2...v6.25.3) (2022-05-25)


### Bug Fixes

* add native image configurations for Spanner classes ([#1858](#1858)) ([92d0292](92d0292))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants