-
Notifications
You must be signed in to change notification settings - Fork 64
Fix mono master build #112
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
Conversation
|
Hi @akoeplinger, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
94d3c47 to
07eb607
Compare
| <ItemGroup> | ||
| <Reference Include="System" /> | ||
| <Reference Include="nunit.framework"> | ||
| <HintPath>..\..\..\lib\NUnit-2.6.3\bin\nunit.framework.dll</HintPath> |
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.
...what? Java.Interop.Dynamic-Tests doesn't reference nunit.framework?
What's odd is that (1) it does here, and (2) this PR built with the PR builder, so I am very confused:
$ monodis --assemblyref bin/TestDebug/Java.Interop.Dynamic-Tests.dll
...
3: Version=2.6.3.13283
Name=nunit.framework
Flags=0x00000000
Public Key:
0x00000000: 96 D0 9A 1E B7 F4 4A 77
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.
From the commit message:
the latter one is deprecated as of Mono 4.9.
I guess the problem is that nunit.framework.dll was present in mono, and in mono 4.9 it isn't anymore, which is why this used to work but now it doesn't.
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.
It's probably getting the reference from one of the projects that are referenced with my change.
I'll redo this leaving the reference in but just fix the the path.
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.
BTW nunit.framework.dll IS still present in Mono 4.9, it's just that types like TestFixtureAttribute have the Obsolete attribute applied with IsError=true (temporarily, to catch cases like this).
In this case I guess what happens is that since the HintPath doesn't exist it'll instead use the assembly from the GAC, which results in the obsolete error.
| <Reference Include="System" /> | ||
| <Reference Include="nunit.framework"> | ||
| <HintPath>..\..\lib\NUnit-2.6.3\bin\nunit.framework.dll</HintPath> | ||
| <HintPath>..\..\packages\NUnit.2.6.3\lib\nunit.framework.dll</HintPath> |
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.
Changes like this make me wonder how this repo ever built... :-/
Make sure we're using the NuGet one instead of the GAC since the latter one is deprecated as of Mono 4.9.
07eb607 to
0ce2535
Compare
|
@jonpryor updated |
Context: https://developercommunity.visualstudio.com/t/illegal-character-exception-in-xamarinandroid-afte/1363149 Changes: dotnet/android-tools@554d45a...d92fc3e * dotnet/android-tools@d92fc3e: [Xamarin.Android.Tools.AndroidSdk] Probe for AdoptOpenJDK Locations (dotnet#115) * dotnet/android-tools@dca30d9: [Xamarin.Android.Tools.AndroidSdk] Probe for Zulu JDK (dotnet#114) * dotnet/android-tools@237642c: [Xamarin.Android.Tools.AndroidSdk] Probe for Microsoft OpenJDK dirs (dotnet#113) * dotnet/android-tools@e618e00: [Xamarin.Android.Tools.AndroidSdk] Fix quotes in %PATH% or %PATHEXT% (dotnet#112)
Context: https://developercommunity.visualstudio.com/t/illegal-character-exception-in-xamarinandroid-afte/1363149 Changes: dotnet/android-tools@554d45a...d92fc3e * dotnet/android-tools@d92fc3e: [Xamarin.Android.Tools.AndroidSdk] Probe for AdoptOpenJDK Locations (#115) * dotnet/android-tools@dca30d9: [Xamarin.Android.Tools.AndroidSdk] Probe for Zulu JDK (#114) * dotnet/android-tools@237642c: [Xamarin.Android.Tools.AndroidSdk] Probe for Microsoft OpenJDK dirs (#113) * dotnet/android-tools@e618e00: [Xamarin.Android.Tools.AndroidSdk] Fix quotes in %PATH% or %PATHEXT% (#112) Finally, *prefer* JDK 8, as hilarious as that is, for two reasons: 1. `gradlew` fails to execute when running under OpenJDK 14: [ERROR] [system.err] java.lang.NoClassDefFoundError: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7 2. The unit tests added in 69e1b80 currently depend upon the XML formatting conventions of JDK 8, and the tests fail when running under JDK 11 because of whitespace changes in `javax.xml` output. For now, it's currently easier to just require JDK 8. The preferred JDK version is controlled by the new `$(MaxJdkVersion)` YAML variable, which in turn sets `$(JI_MAX_JDK)` (55c56f7) and/or the `$(MaxJdkVersion)` MSBuild property, which `dotnet build -t:Prepare` uses to generate `bin/Build*/JdkInfo.props`. Finally, *for consistency*, update `TestJVM` to look for the generated `JdkInfo.props`. If found, use the `$(JdkJvmPath)` value at `/Project/Chooose/When/PropertyGroup/JdkJvmPath` instead of using `JdkInfo.GetKnownSystemJdkInfos()`, so that the JVM that we built against is also used for unit test execution.
Makes sure the build works with Mono master/4.9+.