-
Notifications
You must be signed in to change notification settings - Fork 528
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 part of Mono.Posix build issue regarding MonoDroidSDK discovery. #59
Fix part of Mono.Posix build issue regarding MonoDroidSDK discovery. #59
Conversation
@@ -22,6 +22,9 @@ class MonoDroidSdkUnix : MonoDroidSdkBase | |||
protected override string FindRuntime () | |||
{ | |||
string monoAndroidPath = Environment.GetEnvironmentVariable ("MONO_ANDROID_PATH"); | |||
if (monoAndroidPath == null) | |||
// for Mono.Posix and Mono.Data.Sqlite builds in xamarin-android. | |||
monoAndroidPath = Path.GetFullPath (Path.Combine (new Uri (GetType ().Assembly.CodeBase).LocalPath, "..", "..", "..", "..", "..")); |
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.
@atsushieno would it not be better to make use of the "SearchPaths" property? its resolved later in the FindRuntime method.
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.
SearchPaths need to be absolute paths, which do not fit with this not-fixed path. Or do you mean something else?
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.
My point was this feels like a hack. We could instead add the relative path to the "additionalSearchPaths" variable [1] which would be more in keeping of how this method works.
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.
I don't think that is actually better, because in xabuild
script we set MONO_ANDROID_PATH and in that case my code will never hit that workaround path, while your suggestion will.
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.
if MONO_ANDROID_PATH is a valid value (via the xabuild script) the code will exit [1] so in that case your code path will not be hit and neither would mine. We already have code in place to search alternate directories, I'm just suggesting we use it :) e.g
var additionalSearchPaths = new [] {
Path.GetFullPath (Path.Combine (new Uri (GetType ().Assembly.CodeBase).LocalPath, "..", "..", "..", "..", "..", "lib", "mandroid")),
Path.Combine (personal, @".xamarin.android/lib/mandroid"),
};
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.
Alright, I was confused about the formal(?) behaviour of the path probe, thought that user home thing is not kind of hack around some issue.
LGTM |
8db5456
to
3c44821
Compare
We can build xamarin-android on Linux only because we somehow had a working monodroid setup with MONO_ANDROID_PATH. Hence, fresh build hits: bin/Debug/lib/xbuild/Xamarin/Android/Xamarin.Android.Common.targets: error : Error executing task ResolveSdks: Value cannot be null. Now that Mono.Posix and some other libs depends on existing SDK, MonoDroidSdkUnix needs to also deal with bootstrap build. So far, the SDK sanity check uses generator.exe which does not exist when we are building Mono.Posix, so use class-parse.exe which exists instead. Also we only build 6.0 and UseLatestSDK somehow tries to find 6.0.99 and fails, so use 6.0 instead.
3c44821
to
30c168e
Compare
Commit 3b038a8 broke `make run-tests` because starting with 3b038a8 `generator` would emit an `api.xml.fixed` file into the output directory, which wasn't present in the source directory. This resulted in a unit test execution failures when running `generator-Tests.dll`: generatortests.Adapters.GeneratedOK : Expected out/Adapters/Adapters.xml.fixed but it was not generated. at generatortests.BaseGeneratorTest.CompareOutputs (System.String sourceDir, System.String destinationDir) [0x0004f] in .../Java.Interop/tools/generator/Tests/BaseGeneratorTest.cs:57 at generatortests.BaseGeneratorTest.Run (CodeGenerationTarget target, System.String outputPath, System.String apiDescriptionFile, System.String expectedPath, System.String[] additionalSupportPaths) [0x0004a] in .../Java.Interop/tools/generator/Tests/BaseGeneratorTest.cs:112 at generatortests.BaseGeneratorTest.RunAllTargets (System.String outputRelativePath, System.String apiDescriptionFile, System.String expectedRelativePath, System.String[] additionalSupportPaths) [0x0001c] in .../Java.Interop/tools/generator/Tests/BaseGeneratorTest.cs:95 at generatortests.Adapters.GeneratedOK () [0x0001f] in .../Java.Interop/tools/generator/Tests/Adapters.cs:12 at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&) at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00038] in /private/tmp/source-mono-4.4.0-c7sr0/bockbuild-mono-4.4.0-branch-c7sr0/profiles/mono-mac-xamarin/build-root/mono-x86/mcs/class/corlib/System.Reflection/MonoMethod.cs:295 Fix this error by ignoring the `api.xml.fixed` files.
Fixes: dotnet/android-libzipsharp#64 Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide Changes: dotnet/android-libzipsharp@1.0.10...1.0.20 * dotnet/android-libzipsharp@1752f95: Statically Linux libzip.so (#70) * dotnet/android-libzipsharp@2b1762f: Fix Dll SearchPath to include Assembly Directory. (#69) * dotnet/android-libzipsharp@28b4639: Merge pull request #68 from dellis1972/fixwindowssearch * dotnet/android-libzipsharp@fdabcda: Fix Windows to look in Assembly Directory for 32bit dll. * dotnet/android-libzipsharp@b332af0: Fix Native Crash on Windows. (#67) * dotnet/android-libzipsharp@96eb5e3: Use DefaultDllImportSearchPathsAttribute (#66) * dotnet/android-libzipsharp@755a42a: Bump libzip to 1.7.3 and go back to mingw (#65) * dotnet/android-libzipsharp@5ae5e70: Merge pull request #60 from xamarin/msvc-static-link * dotnet/android-libzipsharp@30ff680: Build libzip with static CRT and VS2019 * dotnet/android-libzipsharp@bad320e: Merge pull request #59 from dellis1972/theswitcharoo * dotnet/android-libzipsharp@d7bc2c5: Merge pull request #58 from xamarin/optimize-winbuild * dotnet/android-libzipsharp@34dc213: Make 64 bit Linux native lib the default. * dotnet/android-libzipsharp@d3aad35: fixup! Optimize libzip build * dotnet/android-libzipsharp@0970a01: Optimize libzip build * dotnet/android-libzipsharp@d321af1: Merge pull request #57 from xamarin/fix-win32-packaging * dotnet/android-libzipsharp@80b739d: Fix a typo which caused 64-bit dll to be packaged for 32-bit Windows * dotnet/android-libzipsharp@1665db0: Bump the version (to 1.0.12), to prepare to release (#56) * dotnet/android-libzipsharp@dd5e939: Throw exception instead of silently failing if zip save/close fails (#54) * dotnet/android-libzipsharp@2df5b16: Fix enumerating zip with deleted entries (#53) * dotnet/android-libzipsharp@a042554: Add .editorconfig, copied from xamarin-android (#55) * dotnet/android-libzipsharp@a0973d4: Bump libzip to 1.6.1 (#49) Changes: nih-at/libzip@rel-1-5-1...v1.7.3 * Context: https://libzip.org/news/release-1.7.3.html * Context: https://libzip.org/news/release-1.7.2.html * Context: https://libzip.org/news/release-1.7.1.html * Context: https://libzip.org/news/release-1.7.0.html Two primary changes of note in this xamarin/LibZipSharp bump: 1. Bump to `libzip` 1.7.3, which contains numerious fixes and improvements over the previously used 1.5.1 release. 2. Use of `DefaultDllImportSearchPathsAttribute` so that native library dependencies are loaded securely, i.e. w/o allowing "other" libraries to be loaded from unsafe directories. Unfortunately, the `libzip` bump itself caused issues, PR #4751 and PR #4937 each had integration tests "randomly" SIGSEGV. This was eventually tracked down to a bug within `libzip` itself, fixed at: * nih-at/libzip#202
Fixes: dotnet/android-libzipsharp#64 Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578 Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide Changes: dotnet/android-libzipsharp@1.0.10...1.0.20 * dotnet/android-libzipsharp@1752f95: Statically Linux libzip.so (#70) * dotnet/android-libzipsharp@2b1762f: Fix Dll SearchPath to include Assembly Directory. (#69) * dotnet/android-libzipsharp@28b4639: Merge pull request #68 from dellis1972/fixwindowssearch * dotnet/android-libzipsharp@fdabcda: Fix Windows to look in Assembly Directory for 32bit dll. * dotnet/android-libzipsharp@b332af0: Fix Native Crash on Windows. (#67) * dotnet/android-libzipsharp@96eb5e3: Use DefaultDllImportSearchPathsAttribute (#66) * dotnet/android-libzipsharp@755a42a: Bump libzip to 1.7.3 and go back to mingw (#65) * dotnet/android-libzipsharp@5ae5e70: Merge pull request #60 from xamarin/msvc-static-link * dotnet/android-libzipsharp@30ff680: Build libzip with static CRT and VS2019 * dotnet/android-libzipsharp@bad320e: Merge pull request #59 from dellis1972/theswitcharoo * dotnet/android-libzipsharp@d7bc2c5: Merge pull request #58 from xamarin/optimize-winbuild * dotnet/android-libzipsharp@34dc213: Make 64 bit Linux native lib the default. * dotnet/android-libzipsharp@d3aad35: fixup! Optimize libzip build * dotnet/android-libzipsharp@0970a01: Optimize libzip build * dotnet/android-libzipsharp@d321af1: Merge pull request #57 from xamarin/fix-win32-packaging * dotnet/android-libzipsharp@80b739d: Fix a typo which caused 64-bit dll to be packaged for 32-bit Windows * dotnet/android-libzipsharp@1665db0: Bump the version (to 1.0.12), to prepare to release (#56) * dotnet/android-libzipsharp@dd5e939: Throw exception instead of silently failing if zip save/close fails (#54) * dotnet/android-libzipsharp@2df5b16: Fix enumerating zip with deleted entries (#53) * dotnet/android-libzipsharp@a042554: Add .editorconfig, copied from xamarin-android (#55) * dotnet/android-libzipsharp@a0973d4: Bump libzip to 1.6.1 (#49) Changes: nih-at/libzip@rel-1-5-1...v1.7.3 * Context: https://libzip.org/news/release-1.7.3.html * Context: https://libzip.org/news/release-1.7.2.html * Context: https://libzip.org/news/release-1.7.1.html * Context: https://libzip.org/news/release-1.7.0.html Two primary changes of note in this xamarin/LibZipSharp bump: 1. Bump to `libzip` 1.7.3, which contains numerious fixes and improvements over the previously used 1.5.1 release. 2. Use of `DefaultDllImportSearchPathsAttribute` so that native library dependencies are loaded securely, i.e. w/o allowing "other" libraries to be loaded from unsafe directories. Unfortunately, the `libzip` bump itself caused issues, PR #4751 and PR #4937 each had integration tests "randomly" SIGSEGV. This was eventually tracked down to a bug within `libzip` itself, fixed at: * nih-at/libzip#202
We can build xamarin-android on Linux only because we somehow had a
working monodroid setup with MONO_ANDROID_PATH. Hence, fresh build hits:
Now that Mono.Posix and some other libs depends on existing SDK,
MonoDroidSdkUnix needs to also deal with bootstrap build.
So far, the SDK sanity check uses generator.exe which does not exist when
we are building Mono.Posix, so use class-parse.exe which exists instead.
Also we only build 6.0 and UseLatestSDK somehow tries to find 6.0.99 and
fails, so use 6.0 instead.