dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.93k stars 527 forks source link

The AdjustJavacVersionArguments task can fail during a rebuild in VS 2019 #2583

Closed pjcollins closed 5 years ago

pjcollins commented 5 years ago

Steps to Reproduce

  1. Download, build, and rebuild the attached project: Bug24206.zip

  2. msbuild /t:Build Bug24206\AndroidApp1\AndroidApp1.csproj

  3. msbuild /t:Rebuild Bug24206\AndroidApp1\AndroidApp1.csproj

Expected Behavior

Building and then rebuilding a project should succeed.

Actual Behavior

error MSB4044: The "AdjustJavacVersionArguments" task was not given a value for the required parameter "JdkVersion".

This appears to be a regression as compared to VS 2017 (15.9.4).

Version Information

Xamarin.Android SDK 9.1.103.6 (HEAD/1127dc3a) Xamarin.Android Reference Assemblies and MSBuild support. Mono: mono/mono/2018-08@23f2024af80 Java.Interop: xamarin/java.interop/d16-0@c987483 LibZipSharp: grendello/LibZipSharp/master@44de300 LibZip: nih-at/libzip/rel-1-5-1@b95cf3f MXE: xamarin/mxe/xamarin@b9cbb535 ProGuard: xamarin/proguard/master@905836d SQLite: xamarin/sqlite/3.25.2@4ea4c69 Xamarin.Android Tools: xamarin/xamarin-android-tools/master@f05c0aa

Log File

https://gist.githubusercontent.com/pjcollins/f5bff3f70a3ab743b0c8aac4e34be858/raw/9c39397848d24227e303b43f32de0d308ed5ff55/gistfile1.txt

dellis1972 commented 5 years ago

Looks like a problem with it skipping a task

Task "ResolveJdkJvmPath" skipped, due to false condition; ( '$(DesignTimeBuild)' != 'True' And '$(AndroidGenerateJniMarshalMethods)' == 'True' And '$(JdkJvmPath)' == '' ) was evaluated as ( 'true' != 'True' And 'False' == 'True' And '' == '' ).
Task "ValidateJavaVersion" skipped, due to false condition; ( '$(DesignTimeBuild)' != 'True' Or '$(AndroidUseManagedDesignTimeResourceGenerator)' != 'True' ) was evaluated as ( 'true' != 'True' Or 'True' != 'True' ).
dellis1972 commented 5 years ago

So why is DesignTimeBuild true for a ReBuild....

dellis1972 commented 5 years ago

Ok. It might not be that.

dellis1972 commented 5 years ago

This is really weird

Target "_SetupDesignTimeBuildForCompile: (TargetId:41)" in file "C:\Program Files (x86)\Microsoft Visual Studio\2019\IntPreview\MSBuild\Xamarin\Android\Xamarin.Android.Common.targets" from project "C:\Users\peter\source\QualityAssurance\Samples\android\Bug24206\AndroidApp1\AndroidApp1.csproj" (target "UpdateAndroidResources" depends on it):
Set Property: DesignTimeBuild=true
Set Property: ManagedDesignTimeBuild=False
Set Property: _AndroidResourcePathsCache=obj\Debug\designtime\resourcepaths.cache
Set Property: _AndroidLibraryImportsCache=obj\Debug\designtime\libraryimports.cache
Set Property: _AndroidLibraryProjectImportsCache=obj\Debug\designtime\libraryprojectimports.cache
Set Property: _AndroidBuildPropertiesCache=obj\Debug\designtime\build.props
Task "MakeDir" (TaskId:110)
  Task Parameter:Directories=obj\Debug\stamp\ (TaskId:110)
  Creating directory "obj\Debug\stamp\". (TaskId:110)
Done executing task "MakeDir". (TaskId:110)

That is for the App project.. That means that the _SetupDesignTimeBuildForCompile target is running BEFORE _SetupDesignTimeBuildForBuild. I'm not sure how that is possible. Unless something has changed in the way MSBuild does a ReBuild and our old technique is now not working :/

dellis1972 commented 5 years ago

ok, this might be the problem

RebuildDependsOn = 
      BeforeRebuild;
      Clean;
      UpdateAndroidResources;SignAndroidPackage;
      AfterRebuild;
dellis1972 commented 5 years ago

UpdateAndroidResources will call _SetupDesignTimeBuildForCompile. Then when SignAndroidPackage is run the property will already be set.

But that does open another question..... why when doing a ReBuild are UpdateAndroidResources;SignAndroidPackage; being called in the first place? That is doing to proceed an apk.. a normal Build should NOT be doing that.

dellis1972 commented 5 years ago

OK. The cause of this issue is in the app itself by the looks of things

DefaultTargets="UpdateAndroidResources;SignAndroidPackage"

Normally the DefaultTargets=Build . In this case if you call MsBuild without specifying a target it will call UpdateAndroidResources;SignAndroidPackage. But because of [1], this also means that calling ReBuild will call UpdateAndroidResources;SignAndroidPackage.

Now we can work around this by hooking into the BeforeRebuild target to call _SetupDesignTimeBuildForBuild which will fix the issue. But that does seem like a bit of a hack. Or we can update the test to not set those default targets (unless that was by design? @pjcollins )

[1] https://github.com/Microsoft/msbuild/blob/0f8106a6ef9b6347e43815cc842130d2ed349ef5/src/Tasks/Microsoft.Common.CurrentVersion.targets#L914

dellis1972 commented 5 years ago

@pjcollins This might have been fixed in p2 . The conditionals on ValidateJavaVersion have been removed between d16-0 and d16-0-p2.

That said. the condition still seems to be on master, but I can't repo on master.

[1] https://github.com/xamarin/xamarin-android/blob/d16-0/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L717 [2] https://github.com/xamarin/xamarin-android/blob/d16-0-p2/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L722

jonathanpeppers commented 5 years ago

@pjcollins yeah this is really weird to have in the Android project?

DefaultTargets="UpdateAndroidResources;SignAndroidPackage"

Was that to test something specific?

jonathanpeppers commented 5 years ago

if I do this with 15.9:

msbuild AndroidApp1.csproj
adb install .\bin\Debug\AndroidApp1.AndroidApp1-Signed.apk

The app doesn't crash, but it's plain white (no resources load).

pjcollins commented 5 years ago

The overridden DefaultTargets element in that project doesn't appear to be directly relevant to the original issue discussed in https://bugzilla.xamarin.com/show_bug.cgi?id=24206. I've updated this test scenario to only use Build as the default target, and will try to keep an eye out for any issues in AdjustJavacVersionArguments in other incremental build or "dev loop" scenarios. I think it's safe to close this for now, it can be reopened if the issue pops up anywhere else.