[setup-windows] Provide a Windows installer #708
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The macOS+msbuild PR Build failure is due to a Visual Studio for Mac bug: |
jonpryor
force-pushed
the
jonp-windows-setup
branch
from
6c730b2 to
52e2ee4
Compare
I really didn't want to do this. I wanted the Xamarin.Android SDK on Windows to be usable side-by-side, so that multiple `oss-xamarin.android*.zip` files could be extracted, and developers could switch between them by overriding MSBuild properties. This is what was documented in commit 87ca273, by overriding the `$(TargetFrameworkRootPath)` MSBuild property. There's just one "minor" problem with that approach: it only works if the project that is being built, and *all* project dependencies, are Xamarin.Android projects. If *any* other kind of dependency is brought in, this approach will no longer work, as the `GetReferenceAssemblies` target -- which looks for assemblies *rooted within `$(TargetFrameworkRootPath)`* -- won't be able to find them. Unfortunately, *everything of interest* doesn't fit within this restriction. A Xamarin.Forms app, or any other app using PCL assemblies, quickly runs afoul of it: error MSB3644: The reference assemblies for framework ".NETPortable,Version=v4.5,Profile=Profile259" were not found. Consequently, the instructions from commit 87ca273 are borderline worthless. There is only one way to actually build such projects, and that's to install Xamarin.Android *system-wide*, so that MSBuild's `GetReferenceAssemblies` target can find everything it needs. :-( Thus, we need an "installer." I was hoping for a simple `.cmd` file, but that stymied me. Then I hoped for a PowerShell script, but installation requires access to the `%VSINSTALLDIR%` environment variable, which is only exported from Visual Studio Developer Command Prompts, and all the solutions I found to import the VS command prompt environment info into PowerShell looked decidedly ugly. Which brings us to a minimal effort command-line installer: `setup-windows.exe`. This utility backs up existing installs, then creates symbolic links within the system-wide directories, pointing them into the extracted `oss-xamarin.android*.zip` contents which contains `setup-windows.exe`. `setup-windows.exe /uninstall` is also provided, to put directories back the way they were found. `Documentation/UsingJenkinsBuildArtifacts.md` has been updated accordingly.
jonpryor
force-pushed
the
jonp-windows-setup
branch
from
52e2ee4 to
907e108
Compare
jonpryor
added a commit
that referenced
this pull request
Sep 5, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1193891 Fixes: #4989 Changes: dotnet/java-interop@b4c2a67...a807961 * dotnet/java-interop@a8079616: [Java.Interop] Dispose, *then* Remove (#708) ~~~ Also cherry-picks d4e0738, which *should* have been done as part of commit 62a5709, but was overlooked: What happens if you dispose an instance *while disposing the instance*? // C# public class MyDisposableObject : Java.Lang.Object { bool _isDisposed; protected override void Dispose (bool disposing) { if (_isDisposed) { return; } _isDisposed = true; if (this.Handle != IntPtr.Zero) this.Dispose (); base.Dispose (disposing); } } // … void MyTestMethod () { var value = new MyDisposableObject (); value.Dispose (); value.Dispose (); } Here, `MyDisposableObject.Dispose(bool)` calls `Object.Dispose()` when `Object.Handle` is valid. This "feels" admittedly unusual, but `IDisposable.Dispose()` is *supposed to be* Idempotent: it can be called multiple times with no ill effects. Shouldn't this be the same? Unfortunately, it *isn't* the same; it crashes, hard: ================================================================= Native stacktrace: ================================================================= 0x10245bc49 - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : mono_dump_native_crash_info 0x1023f3d35 - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : mono_handle_native_crash 0x10245b20f - /Library/Frameworks/Mono.framework/Versions/Current/Commands/mono : sigabrt_signal_handler 0x7fff6983d5fd - /usr/lib/system/libsystem_platform.dylib : _sigtramp 0x700009b716b0 - Unknown 0x7fff69713808 - /usr/lib/system/libsystem_c.dylib : abort 0x10658377a - …/lib/server/libjvm.dylib : _ZN2os5abortEbPvPKv 0x10637ea5f - …/lib/server/libjvm.dylib : _ZN8jniCheck15validate_handleEP10JavaThreadP8_jobject 0x10637eb08 - …/lib/server/libjvm.dylib : _ZN8jniCheck15validate_objectEP10JavaThreadP8_jobject 0x1063818a1 - …/lib/server/libjvm.dylib : checked_jni_GetObjectClass 0x10594cfa8 - …/Java.Interop/bin/TestDebug/libjava-interop.dylib : java_interop_jnienv_get_object_class 0x107a1c636 - Unknown 0x107aa2c73 - Unknown … ================================================================= Managed Stacktrace: ================================================================= at <unknown> <0xffffffff> at Java.Interop.NativeMethods:java_interop_jnienv_get_object_class <0x000a5> at Types:GetObjectClass <0x0010a> at Types:GetJniTypeNameFromInstance <0x000a2> at JniValueManager:DisposePeer <0x002c2> at JniValueManager:DisposePeer <0x000f2> at Java.Interop.JavaObject:Dispose <0x000ea> at Java.InteropTests.JavaObjectTest:NestedDisposeInvocations <0x00072> at System.Object:runtime_invoke_void__this__ <0x000b0> at <unknown> <0xffffffff> at System.Reflection.RuntimeMethodInfo:InternalInvoke <0x000b8> at System.Reflection.RuntimeMethodInfo:Invoke <0x00152> at System.Reflection.MethodBase:Invoke <0x00058> Ouch. The cause of the crash isn't the "successive" `.Dispose()` invocations within `MyTestMethod()`, but rather the *nested* `.Dispose()` invocation within `MyDisposableObject.Dispose()`. Runtime execution is thus: 1. `JavaObject.Dispose()` 2. `JniRuntime.JniValueManager.DisposePeer(this)` 3. `var h = this.PeerReference` 4. `JniRuntime.JniValueManager.DisposePeer(h, this)` 5. `JavaObject.Disposed()` 6. `MyDisposableObject.Dispose(disposing:true)` 7. `JavaObject.Dispose()` // back to (1)? 8. `JniRuntime.JniValueManager.DisposePeer(this)` 9. `var h = this.PeerReference` // *second* ref to `h` 10. `JniRuntime.JniValueManager.DisposePeer(h, this)`, which passes `h` to e.g. `JniEnvironment.Types.GetJniTypeNameFromInstance()`, thus requiring that `h` be a valid JNI reference, and also calls `JniObjectReference.Dispose()`, invalidating `h`. 11. "Unwinding" (4), call `Types.GetJniTypeNameFromInstance()` with the `h` from (3). The problem *appears to be* the recursive `Dispose()` invocation on (7), but the *actual* problem is step (3): by holding a cached/"old" value of `this.PeerReference` -- and then later using that *same* value in `JniRuntime.JniValueManager.DisposePeer()`-- when the nested `JavaObject.Dispose()` invocation continues execution, `this.PeerReference` will be *invalidated*, but the copy of the handle from (3) will still be used! This causes the JVM to very loudly abort. The fix is to defer the "caching" present in (3): instead of storing the `PeerReference` value "immediately" -- and disposing the same value "later" -- don't store the value until *after* `IJavaPeerable.Disposed()` is called. This gives the `Dispose(disposing:true)` method a chance to execute *before* retaining any cached references to `PeerReference` -- which may in turn invalidate `PeerReference`! -- thus ensuring that we only attempt to dispose *valid* JNI handles. Commit dotnet/java-interop@4141d848 contains fixes to `JniRuntime.JniValueManager.DisposePeer()`, and add appropriate tests.
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Sep 5, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1193891 Changes: dotnet/java-interop@afbc5b3...75354b9 * dotnet/java-interop@75354b9f: [Java.Interop] Dispose, *then* Remove (dotnet#708) * dotnet/java-interop@8ea0bb28: [jnimarshalmethod-gen] Localizable errors (dotnet#696)
jonpryor
pushed a commit
that referenced
this pull request
Sep 9, 2020
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1193891 Changes: dotnet/java-interop@afbc5b3...ac914ce * dotnet/java-interop@ac914ce3: [ci] Add DevDiv required Roslyn analyzers, fix errors (#704) * dotnet/java-interop@a98c1ae1: [build] fix env vars causing gradle build error (#705) * dotnet/java-interop@75354b9f: [Java.Interop] Dispose, *then* Remove (#708) * dotnet/java-interop@8ea0bb28: [jnimarshalmethod-gen] Localizable errors (#696) Commit dotnet/java-interop@ac914ce3 moved the location of `NullableAttributes.cs`; update `Xamarin.Android.Tools.JavadocImporter.csproj` appropriately.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I really didn't want to do this. I wanted the Xamarin.Android SDK on
Windows to be usable side-by-side, so that multiple
oss-xamarin.android*.zipfiles could be extracted, and developerscould switch between them by overriding MSBuild properties.
This is what was documented in commit 87ca273, by overriding the
$(TargetFrameworkRootPath)MSBuild property.There's just one "minor" problem with that approach: it only works if
the project that is being built, and all project dependencies, are
Xamarin.Android projects. If any other kind of dependency is brought
in, this approach will no longer work, as the
GetReferenceAssembliestarget -- which looks for assemblies rooted within
$(TargetFrameworkRootPath)-- won't be able to find them.Unfortunately, everything of interest doesn't fit within this
restriction. A Xamarin.Forms app, or any other app using PCL
assemblies, quickly runs afoul of it:
Consequently, the instructions from commit 87ca273 are borderline
worthless. There is only one way to actually build such projects, and
that's to install Xamarin.Android system-wide, so that MSBuild's
GetReferenceAssembliestarget can find everything it needs.:-(
Thus, we need an "installer." I was hoping for a simple
.cmdfile,but that stymied me. Then I hoped for a PowerShell script, but
installation requires access to the
%VSINSTALLDIR%environmentvariable, which is only exported from
Visual Studio Developer Command Prompts, and all the solutions I found
to import the VS command prompt environment info into PowerShell
looked decidedly ugly.
Which brings us to a minimal effort command-line installer:
setup-windows.exe. This utility backs up existing installs, thencreates symbolic links within the system-wide directories, pointing
them into the extracted
oss-xamarin.android*.zipcontents whichcontains
setup-windows.exe.setup-windows.exe /uninstallis also provided, to put directoriesback the way they were found.
Documentation/UsingJenkinsBuildArtifacts.mdhas been updatedaccordingly.