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

[build] option to filter by JI_MAX_JDK on make prepare #226

Merged
merged 2 commits into from
Dec 21, 2017

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Dec 18, 2017

Context:
https://stackoverflow.com/questions/47627499/does-android-studio-3-support-java-9-for-android-development

At the current time, we will not be able to use JDK 9 for Android. Some
of our build agents (VSTS), now have JDK 9 installed, so we need to
make a few changes to make sure JDK 8 is picked up instead.

Changes:

  • Create a JI_MAX_JDK option, as a way for xamarin-android to
    exclude JDK 9
  • Create new JI_JAVAC_PATH and JI_JAR_PATH make variables, which
    will be the full path to javac and jar
  • Use awk to filter on <= JI_MAX_JDK
  • Use sed to find the JDK version number, see options of folder names
    below
  • sort -n should be used to sort numerically
  • Set JI_JAVAC_PATH and JI_JAR_PATH to their full paths
  • Support both Darwin and Linux, Windows support is handled in
    xamarin-android currently

Known JDK folder names

macOS:

1.6.0.jdk
jdk1.7.0_79.jdk
jdk1.8.0_101.jdk
jdk1.8.0_152.jdk
jdk-9.0.1.jdk

Ubuntu:

java-8-openjdk-amd64
java-9-openjdk-amd64

@atsushieno
Copy link
Contributor

Are you ignoring Windows and Linux?

@jonpryor
Copy link
Member

I like the sentiment. I don't like the idea.

The underlying idea behind Java.Interop is that it should be Java version agnostic, to the degree that this is supportable and makes sense.

It certainly makes sense that we be able to run Java.Interop against JDK 9.

The problem with JDK 9 isn't Java.Interop; it's xamarin-android, and the Android SDK. Hardcoding this restriction into Java.Interop sounds like a violation of separation of concerns.

What I'd prefer -- and offhand I have no idea how this would actually work in practice -- is that the make prepare target "somehow" take a "JDK filter", which xamarin-android could provide. This would allow Java.Interop to pick the latest-and-greatest JDK running around, while allowing xamarin-android to restrict things to just JDK 8.

@@ -33,16 +35,17 @@ endif # 32-bit

# Darwin supports three possible search locations:
#
# 1. `/Library/Java/JavaVirtualMachines/jdk*`
# 1. `/Library/Java/JavaVirtualMachines/jdk1.*`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this filtering correct? On Linux I have I have /usr/lib/jvm/java-8-openjdk-amd64 which would be filtered out by this if this were also applied to Linux.

Copy link
Member

Choose a reason for hiding this comment

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

This is within an ifeq ($(OS),Darwin) block, so this change won't impact Linux.

Copy link
Contributor

Choose a reason for hiding this comment

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

I keep writing "if it were... on Linux".

Copy link
Member Author

Choose a reason for hiding this comment

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

I think @atsushieno is asking if 1.8 will work on Linux since his folder says 8.

Looked on ubuntu and I have symbolic links:

$ ls -la /usr/lib/jvm/
...
lrwxrwxrwx   1 root root   20 Oct 23 17:43 java-1.8.0-openjdk-amd64 -> java-8-openjdk-amd64
lrwxrwxrwx   1 root root   20 Aug 24 12:15 java-1.9.0-openjdk-amd64 -> java-9-openjdk-amd64
drwxr-xr-x   5 root root 4096 Dec 18 22:58 java-8-openjdk-amd64
drwxr-xr-x   8 root root 4096 Dec 18 23:00 java-9-openjdk-amd64

I just did apt install open-jdk-8-headless and open-jdk-9-headless.

But still, I need to:

  • Make the Java 9 restriction an option that can be passed in by xamarin-android
  • Fix this for Linux, too
  • On Windows, JdkInfo.props is generated by an MSBuild task in xamarin-android, so I won't have to deal with it until then

@jonathanpeppers jonathanpeppers changed the title [build] use JDK 8 when JDK 9 is installed [build] option to filter by JI_MAX_JDK on make prepare Dec 19, 2017
Context:
https://stackoverflow.com/questions/47627499/does-android-studio-3-support-java-9-for-android-development

At the current time, we will not be able to use JDK 9 for Android. Some
of our build agents (VSTS), now have JDK 9 installed, so we need to
make a few changes to make sure JDK 8 is picked up instead.

Changes:
- Create a `JI_MAX_JDK` option, as a way for `xamarin-android` to
exclude JDK 9
- Create new `JI_JAVAC_PATH` and `JI_JAR_PATH` make variables, which
will be the full path to `javac` and `jar`
- Use `awk` to filter on <= `JI_MAX_JDK`
- Use `sed` to find the JDK version number, see options of folder names
below
- `sort -n` should be used to sort numerically
- Set `JI_JAVAC_PATH` and `JI_JAR_PATH` to their full paths
- Support both `Darwin` and `Linux`, Windows support is handled in
`xamarin-android` currently

~~Known JDK folder names~~

macOS:
```
1.6.0.jdk
jdk1.7.0_79.jdk
jdk1.8.0_101.jdk
jdk1.8.0_152.jdk
jdk-9.0.1.jdk
```

Ubuntu:
```
java-8-openjdk-amd64
java-9-openjdk-amd64
```
@jonathanpeppers
Copy link
Member Author

@jonpryor this has the sed/regex changes we discussed today, should be good to go

@jonpryor jonpryor merged commit 55c56f7 into dotnet:master Dec 21, 2017
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Dec 22, 2017
Context: dotnet/java-interop#226

At the current time, we will not be able to use JDK 9 for Android. Some
of our build agents (VSTS), now have JDK 9 installed, so we need to
make a few changes to make sure JDK 8 is picked up instead.

Changes:
- Bump to java.interop/master/55c56f7
- Calls to `make -C external/Java.Interop` need to pass `JI_MAX_JDK=8`
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Dec 22, 2017
Context: dotnet/java-interop#226

At the current time, we will not be able to use JDK 9 for Android. Some
of our build agents (VSTS), now have JDK 9 installed, so we need to
make a few changes to make sure JDK 8 is picked up instead.

Changes:
- Bump to java.interop/master/55c56f7
- Calls to `make -C external/Java.Interop` need to pass `JI_MAX_JDK=8`
jonpryor pushed a commit that referenced this pull request Jan 26, 2024
Changes: dotnet/android-tools@4889bf0...ed102fc

  * dotnet/android-tools@ed102fc: [Xamarin.Android.Tools.Versions] Add JavaSdkVersion (#226)
  * dotnet/android-tools@b175674: [ci] Only enable CodeQL on Windows build job (#224)
  * dotnet/android-tools@2a2e64b: [ci] Add API Scan job (#225)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants