Fix monodroid SDK detection that depended on non-existent file. #49

atsushieno · 2016-05-26T06:07:30Z

We don't build DebugRuntime apk in this new source set anymore, so
do not look for the file in our SDK sanity checker.

Additional warning logs are added because, we should do that.
(Otherwise we will keep ignorant of the actual cause of the
problem forever.)

jonpryor · 2016-05-26T21:59:15Z

src/Xamarin.Android.Build.Utilities/Sdks/MonoDroidSdkUnix.cs

@@ -38,7 +42,7 @@ protected override string FindRuntime ()
 		protected override bool ValidateBin (string binPath)
 		{
 			return !string.IsNullOrWhiteSpace (binPath) &&
-				File.Exists (Path.Combine (binPath, "generator"));
+				File.Exists (Path.Combine (binPath, "generator.exe"));


The problem with this change is that in a "real" Xamarin.Android install, we'll have:

$prefix/bin/generator $prefix/lib/mandroid/generator.exe

wherein $prefix/bin/generator will be a shell script that invokes $prefix/lib/mandroid/generator.exe. The BindingsGenerator task expects to execute generator on non-Windows platforms, so if $prefix/bin/generator doesn't exist then bindings will fail.

Whether this is desirable or not is debatable. On the one hand, since generator.exe is an assembly then Xamarin.Android.Build.Tasks.dll can just reference that assembly and use the types from the BindingsGenerator task, no problem.

The flip side is that generator.exe does lots of XML parsing and manipulation, which presumably will use some amount of memory. Do we really want this additional memory use in Xamarin.Android.Build.Tasks.dll? Or is it better to just keep it in a separate process?

I don't know. (Numbers would be handy...)

In the meantime, ValidateBin() should probably check for either generator or generator.exe, just to simplify things. Or, ValidateBin() should be a no-op. Or we should create the $prefix/bin/generator script now.

atsushieno · 2016-05-27T13:49:03Z

How about this?

atsushieno · 2016-05-27T15:43:45Z

Sorry, I messed my own changes in the previous fix, now FindBin() should find "bin" dir as it used to.

jonpryor · 2016-05-27T16:10:51Z

Xamarin.Android.sln

@@ -3,7 +3,7 @@ Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio 2012
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Build-Tools", "Build-Tools", "{E351F97D-EA4F-4E7F-AAA0-8EBB1F2A4A62}"
 EndProject
-Project("{9344BDBB-3E7F-41FC-A0DD-8665D75EE146}") = "android-toolchain", "build-tools\android-toolchain\android-toolchain.mdproj", "{8FF78EB6-6FC8-46A7-8A15-EBBA9045C5FA}"
+Project("{9344BDBB-3E7F-41FC-A0DD-8665D75EE146}") = "unix-distribution-setup", "build-tools\unix-distribution-setup\unix-distribution-setup.mdproj", "{8FF78EB6-6FC8-46A7-8A15-EBBA9045C5FA}"


This is a rather odd change. Are you sure you wanted to remove android-toolchain from the solution?

jonpryor · 2016-05-31T13:41:08Z

Xamarin.Android.sln

@@ -64,6 +64,7 @@ EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Mono.Android-Tests", "src\Mono.Android\Test\Mono.Android-Tests.csproj", "{40EAD437-216B-4DF4-8258-3F47E1672C3A}"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Mono.Posix", "src\Mono.Posix\Mono.Posix.csproj", "{83F00D30-0AC6-40D8-834B-DD39B6CAA8B3}"
+Project("{9344BDBB-3E7F-41FC-A0DD-8665D75EE146}") = "unix-distribution-setup", "build-tools\unix-distribution-setup\unix-distribution-setup.mdproj", "{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}"


Invalid syntax here. The syntax is:

Project(...) EndProject

You have:

Project(...) Project(...) EndProject

You need to place an EndProject before this line.

oops, I removed it during the merge with master...!

jonpryor · 2016-06-01T13:30:34Z

build-tools/unix-distribution-setup/unix-distribution-setup.mdproj

+
+  <Target Name="Build">
+    <Copy SourceFiles="..\..\tools\scripts\generator" DestinationFiles="$(OutputPath)bin\generator" />
+    <Exec Command="chmod +x $(OutputPath)bin\generator" />


This should be Condition=" '$(HostOS)' != 'Windows' ", so that we don't execute this on Windows.

We don't build DebugRuntime apk in this new source set anymore, so do not look for the file in our SDK sanity checker. Additional warning logs are added because, we should do that. (Otherwise we will keep ignorant of the actual cause of the problem forever.)

The internal Xamarin.Android CI system is reporting an error when running the `generator` unit tests, but it's a false positive. It's *not* a bug in `generator`. Rather, it's a bug in Mono 4.2, in which [`CSharpCodeCompiler` treats multi-line warnings as errors][0]. Since the `generator` unit tests emit such multi-line warnings when hiding `Java.Lang.Object.Handle` (via `Xamarin.Test.SomeObject.Handle()`) and `System.Exception.Message` (via `Java.Lang.Throwable.Message`), and Mono 4.2 treats the multi-line warnings as errors, the tests fail. There are three plausible solutions so that we don't get erroneous reports from the `generator` tests when running on Mono 4.2: 1. Remove those tests. (Uh...no.) 2. Upgrade Mono on the CI machine to Mono 4.4, which has the fix. 3. Fix `generator` so that the warning isn't emitted. (2) would require an unknown timeframe with unknown repercussions. Thus, the chosen fix: improve `generator` so that the warning isn't generated. [0]: mono/mono#2248

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

dnfclas added the cla-already-signed label May 26, 2016

jonpryor reviewed May 26, 2016
View reviewed changes

atsushieno force-pushed the do-not-try-to-find-debug-runtime branch from 9d3140b to e695623 Compare May 27, 2016 13:48

atsushieno force-pushed the do-not-try-to-find-debug-runtime branch from e695623 to b9d9907 Compare May 27, 2016 15:42

jonpryor reviewed May 27, 2016
View reviewed changes

atsushieno force-pushed the do-not-try-to-find-debug-runtime branch 2 times, most recently from f38c341 to 027df7c Compare May 31, 2016 04:10

jonpryor reviewed May 31, 2016
View reviewed changes

atsushieno force-pushed the do-not-try-to-find-debug-runtime branch from 027df7c to 70516d6 Compare June 1, 2016 05:01

jonpryor reviewed Jun 1, 2016
View reviewed changes

atsushieno force-pushed the do-not-try-to-find-debug-runtime branch from 70516d6 to 4ba5c37 Compare June 2, 2016 04:56

atsushieno force-pushed the do-not-try-to-find-debug-runtime branch from 4ba5c37 to 784c32e Compare June 3, 2016 02:30

jonpryor merged commit 41bbf80 into dotnet:master Jun 3, 2016

github-actions bot locked and limited conversation to collaborators Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix monodroid SDK detection that depended on non-existent file. #49

Fix monodroid SDK detection that depended on non-existent file. #49

atsushieno commented May 26, 2016

jonpryor May 26, 2016

atsushieno commented May 27, 2016

atsushieno commented May 27, 2016

jonpryor May 27, 2016

jonpryor May 31, 2016

atsushieno Jun 1, 2016

jonpryor Jun 1, 2016

Fix monodroid SDK detection that depended on non-existent file. #49

Fix monodroid SDK detection that depended on non-existent file. #49

Conversation

atsushieno commented May 26, 2016

jonpryor May 26, 2016

Choose a reason for hiding this comment

atsushieno commented May 27, 2016

atsushieno commented May 27, 2016

jonpryor May 27, 2016

Choose a reason for hiding this comment

jonpryor May 31, 2016

Choose a reason for hiding this comment

atsushieno Jun 1, 2016

Choose a reason for hiding this comment

jonpryor Jun 1, 2016

Choose a reason for hiding this comment