Get OpenJDK from well-known paths, not registry #72
Conversation
This method will location Microsoft OpenJdk based on well-known install location. As of AB#735565 we can no longer rely on regristry keys. GetJdkInfos will prefer these well-known paths.
Fixes AB#735565
Fixes CI build
Although I don't think case matters in paths on Windows.
| if (CheckRegistryKeyForExecutable (RegistryEx.CurrentUser, regKey, MDREG_JAVA_SDK, wow, "bin", JarSigner)) | ||
| return RegistryEx.GetValueString (RegistryEx.CurrentUser, regKey, MDREG_JAVA_SDK, wow); | ||
| return null; | ||
| return GetKnownOpenJdkPaths ().FirstOrDefault (); |
There was a problem hiding this comment.
This is where things Go Bad™: the intended semantics is that if you call AndroidSdkInfo. SetPreferredJavaSdkPath(), then the default/preferred Java SDK path will be what was specified:
AndroidSdkInfo.SetPreferredJavaSdkPath ("/my/new/JDK/location");
var sdk = new AndroidSdkInfo ();
Console.WriteLine(sdk.JavaSdkPath);
// prints "/my/new/JDK/location"
That semantic works because AndroidSdkWindows.SetPreferredJavaSdkPath() and AndroidSdkWindows.PreferedJavaSdkPath were consistent with each other.
This PR breaks that semantic, and turns AndroidSdkWindows.SetPreferredJavaSdkPath() into a no-op, effectively, because the default (preferred) Java SDK may not be what was ever provided to AndroidSdkWindows.SetPreferredJavaSdkPath() (unless through sheer happenstance the default value is passed to AndroidSdkWindows.SetPreferredJavaSdkPath()...).
Hence my 2019-Mar-26 comment on that issue:
The way that VS sets a custom path is by setting a registry key.
If we don't check that registry key first, then the custom path (effectively) does not exist and it cannot be overridden by the user.
Which returns us to the originally posed (and not yet answered to my satisfaction): what are we trying to solve?
Is it reading from the registry which is bad? Or is it just writing? Both?
Should Windows copy Unix and instead use a plain text file to store this information instead of the Registry? Would that be better?
What about migration and compatibility? Visual Studio 2019 (Windows) has Tools > Options... > Xamarin / Android Settings, which allows setting the Java Development Kit Location value. This value in turn (eventually) uses AndroidSdkWindows.SetPreferredJavaSdkPath().
Which means this change breaks the IDE semantics.
There was a problem hiding this comment.
Good points, sorry I missed SetPreferredJavaSdkPath ...
I will revert so the change doesn't affect this, but at least we'll be able to find based on well-known paths.
pjcollins
left a comment
pjcollins
left a comment
There was a problem hiding this comment.
I am confused as to why the change in https://github.com/xamarin/XamarinVS/pull/10406 didn't first require a change to xamarin-android-tools (such as this) and then a reference update in XamarinVS. However, this extension to OpenJDK resolution makes sense to me given the somewhat limited context I have.
Context: http://work.azdo.io/735565 Turns Out™ that certain anti-virus programs block or otherwise mitigate access to various Windows Registry keys. (No idea which anti-virus, or what the symptoms are; this is just What I've Heard.) Reduce reliance on the registry by looking for the JDK within the `%ProgramW6432%\Android\jdk\microsoft_dist_openjdk_*` directories, in which the `*` suffix is a `System.Version`-parsable value, and prefer the highest JDK found there. The Registry value is still preferred, if set.
Reason: http://work.azdo.io/735565