-
Notifications
You must be signed in to change notification settings - Fork 524
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
[Xamarin.Android.Build.Tasks] fix aapt2 incremental build for GPS #3856
Merged
jonathanpeppers
merged 1 commit into
dotnet:master
from
jonathanpeppers:playservices-aapt2
Oct 30, 2019
Merged
[Xamarin.Android.Build.Tasks] fix aapt2 incremental build for GPS #3856
jonathanpeppers
merged 1 commit into
dotnet:master
from
jonathanpeppers:playservices-aapt2
Oct 30, 2019
Conversation
This file contains 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
Context: xamarin/GooglePlayServicesComponents#271 In an application with Google Play Services, a json configuration file is used, such as: <GoogleServicesJson Include="google-services.json" /> There are MSBuild targets in the NuGet package that process this file, and add to the `@(LibraryResourceDirectories)` item group: https://github.com/xamarin/GooglePlayServicesComponents/blob/fc057c754e04d3e719d8c111d03d60eb2467b9ce/source/com.google.android.gms/play-services-basement/merge.targets#L44-L80 Unfortunately, this causes MSBuild targets to run every time if `$(AndroidUseAapt2)` is `True`: Building target "_ConvertLibraryResourcesCases" completely. Input file "obj\Debug.stamp" does not exist. Such a project will run `aapt2` every time, even with no changes. The `<CollectNonEmptyDirectories/>` MSBuild task has some logic that doesn't take custom `@(LibraryResourceDirectories)` items into account: { "StampFile", Path.GetFullPath (Path.Combine (directory.ItemSpec, "..", "..")) + ".stamp" }, This code assumes that the file is in `obj\Debug\lp\1\res`, and so using `..\..` is completely fine. If not, we get a weird `obj\Debug.stamp` file! The fix would be: * Check if the file is in the `lp` directory, if so, use the existing behavior. * Create a new stamp file in `obj\Debug\stamp` that is a hash of path. * Create any `%(StampFile)` files in `_CompileAndroidLibraryResources` that do not exist. This should get things working for the current released GPS NuGet. When #GooglePlayServicesComponents/271 lands in a future update of the GPS NuGet package, it will supply a proper `%(StampFile)`.
dellis1972
approved these changes
Oct 30, 2019
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.
Cool, this is exactly what I had in mind when we talked last week.
Only failures are network-related or zipalign, so looks good to merge. |
jonathanpeppers
added a commit
to jonathanpeppers/mobileapp
that referenced
this pull request
Oct 30, 2019
I heard from @JonDouglas that the developer loop on this app was slow. Like 1-2 minutes or something... I noticed a few settings that will drastically improve your Debug build times: * The linker should be set to `None` for Debug builds. * Only use ProGuard in Release builds. * I moved the settings for Fast Deployment around, so things were explicit for Debug vs Release. For details, see: https://devblogs.microsoft.com/xamarin/optimize-xamarin-android-builds/ However, I did find some bugs in Xamarin.Android's build system, when profiling builds of this app. ~~ Missing AndroidResource files ~~ This app lists two files that do not exist: <AndroidResource Include="Resources\layout\SelectDateFormatFragment.axml" /> <AndroidResource Include="Resources\layout\SelectDateFormatFragmentCell.axml" /> Due to the way MSBuild works, this causes: Building target "_CompileResources" completely. Input file "Resources\layout\SelectDateFormatFragment.xml" does not exist. Building target "_UpdateAndroidResgen" completely. Input file "Resources\layout\SelectDateFormatFragment.xml" does not exist. Basically expensive `aapt2` calls happen on every build... We can just remove these lines for now, until we get a Xamarin.Android update. Fixed in: dotnet/android#3837 ~~ Issue with GPS and aapt2 ~~ I noticed usage of Google Play Services and aapt2 causes a few MSBuild targets to always run. I don't have a workaround for this, but we have fixes in: xamarin/GooglePlayServicesComponents#271 dotnet/android#3856
jonpryor
pushed a commit
that referenced
this pull request
Oct 30, 2019
) Context: xamarin/GooglePlayServicesComponents#271 In an application with Google Play Services, a json configuration file is used, such as: <GoogleServicesJson Include="google-services.json" /> There are MSBuild targets in the NuGet package that process this file, and add to the `@(LibraryResourceDirectories)` item group: https://github.com/xamarin/GooglePlayServicesComponents/blob/fc057c754e04d3e719d8c111d03d60eb2467b9ce/source/com.google.android.gms/play-services-basement/merge.targets#L44-L80 Unfortunately, this causes MSBuild targets to run every time if `$(AndroidUseAapt2)` is `True`: Building target "_ConvertLibraryResourcesCases" completely. Input file "obj\Debug.stamp" does not exist. Such a project will run `aapt2` every time, even with no changes. The `<CollectNonEmptyDirectories/>` MSBuild task has some logic that doesn't take custom `@(LibraryResourceDirectories)` items into account: { "StampFile", Path.GetFullPath (Path.Combine (directory.ItemSpec, "..", "..")) + ".stamp" }, This code assumes that the file is in `obj\Debug\lp\1\res`, and so using `..\..` is completely fine. If not, we get a weird `obj\Debug.stamp` file! The fix would be: * Check if the file is in the `lp` directory, if so, use the existing behavior. * Create a new stamp file in `obj\Debug\stamp` that is a hash of path. * Create any `%(StampFile)` files in `_CompileAndroidLibraryResources` that do not exist. This should get things working for the current released GPS NuGet. When #GooglePlayServicesComponents/271 lands in a future update of the GPS NuGet package, it will supply a proper `%(StampFile)`.
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.
Context: xamarin/GooglePlayServicesComponents#271
In an application with Google Play Services, a json configuration file
is used, such as:
There are MSBuild targets in the NuGet package that process this file,
and add to the
@(LibraryResourceDirectories)
item group:https://github.com/xamarin/GooglePlayServicesComponents/blob/fc057c754e04d3e719d8c111d03d60eb2467b9ce/source/com.google.android.gms/play-services-basement/merge.targets#L44-L80
Unfortunately, this causes MSBuild targets to run every time if
$(AndroidUseAapt2)
isTrue
:Such a project will run
aapt2
every time, even with no changes.The
<CollectNonEmptyDirectories/>
MSBuild task has some logic thatdoesn't take custom
@(LibraryResourceDirectories)
items intoaccount:
This code assumes that the file is in
obj\Debug\lp\1\res
, and sousing
..\..
is completely fine. If not, we get a weirdobj\Debug.stamp
file!The fix would be:
lp
directory, if so, use the existingbehavior.
obj\Debug\stamp
that is a hash of path.%(StampFile)
files in_CompileAndroidLibraryResources
that do not exist.
This should get things working for the current released GPS NuGet.
When #GooglePlayServicesComponents/271 lands in a future update of the
GPS NuGet package, it will supply a proper
%(StampFile)
./cc @Redth