[Xamarin.Android.Build.Tasks] _LinkAssembliesNoShrink timestamps #2028
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
dellis1972
approved these changes
Aug 2, 2018
Context: dotnet@3d999d3#diff-42a9402e6466c65d49d0ee7caf21f327R164 Since 3d999d3, a test is failing downstream in monodroid that is testing the following scenario: - With `Fast Deployment` enabled, run `/t:Install` - Make a small code change - Run `/t:Install` again - Assemblies are getting uploaded to the device that should already be up to date! 3d999d3 was addressing a symptom of the problem, but not the root cause. The `OutputStep` of the linker has various uses of `File.Copy`, which *preserves* the timestamp of the source file. https://github.com/mono/linker/blob/615f62c32705322aeb0f3f15212a3dbb94b7f0aa/linker/Linker.Steps/OutputStep.cs#L187 The solution, then, is to run the `<Touch />` MSBuild task on the linker's output. This will keep the proper timestamps, and a portion of the changes in 3d999d3 are no longer needed. I updated the `CheckTimestamps` test to validate `Debug` and `Release` configurations, so `_LinkAssembliesShrink` is also tested in this manner. It turns out `_LinkAssembliesShrink` is currently working fine, since it uses `$(_AndroidLinkFlag)` as its `Outputs`.
jonathanpeppers
force-pushed
the
linkassembliesnoshrink-timestamps
branch
from
ce8db9f
to
9e6a305
Compare
jonpryor
pushed a commit
that referenced
this pull request
Aug 3, 2018
Context: xamarin/monodroid#821 Context: 3d999d3#diff-42a9402e6466c65d49d0ee7caf21f327R164 Since commit 3d999d3, a test is failing downstream in monodroid that is testing the following scenario: 1. With `Fast Deployment` enabled, run `/t:Install` 2. Make a small code change 3. Run `/t:Install` again 4. Assemblies are getting uploaded to the device that should already be up to date! 3d999d3 was addressing a symptom of the problem, but not the root cause. The `OutputStep` of the linker has various uses of [`File.Copy()`][0], which preserves the timestamp of the source file: > The attributes of the original file are retained in the copied file. File timestamps are considered "attributes of the original file". The solution, then, is to run the `<Touch/>` MSBuild task on the linker's output. This will create appropriate timestamps, and a portion of the changes in 3d999d3 are no longer needed. I updated the `CheckTimestamps()` test to validate `Debug` and `Release` configurations, so the `_LinkAssembliesShrink` target is also tested in this manner. It turns out `_LinkAssembliesShrink` is currently working fine, since it uses `$(_AndroidLinkFlag)` as its `Outputs` Target attribute. [0]: https://docs.microsoft.com/en-us/dotnet/api/system.io.file.copy?view=netframework-4.7.2#System_IO_File_Copy_System_String_System_String_
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Aug 21, 2018
…s projects @StephaneDelcroix noticed a build performance problem in Xamarin.Forms projects: - Build - Build again with no changes - Slow targets such as `_UpdateAndroidResgen` and `_GenerateJavaStubs` run again! The cause appeared to be this: Building target "_UpdateAndroidResgen" completely. Input file "obj/Debug/res/layout/tabbar.xml" is newer than output file "obj/Debug/R.cs.flag". I modified the `CheckTimestamps` test to replicate this issue: - Added Xamarin.Forms NuGet packages - Added `Tabbar.axml` from the Xamarin.Forms template This surfaced several problems with timestamps and Xamarin.Android building incrementally... ~~ Problem 1 ~~ In the `_GenerateJavaStubs` target, the `ConvertResourcesCases` MSBuild task is not supplied a `AndroidConversionFlagFile`. It uses this file to compare timestamps and decide if XML files need fixed up or not. Supplying `$(_AndroidResgenFlagFile)` seemed to be the way to go here, since `_UpdateAndroidResgen` runs right before this task. It also solves the original problem from the `Tabbar.axml` file. ~~ Problem 2 ~~ Timestamps of Xamarin.Forms assemblies in `obj\Debug\linksrc` were out of date! I updated the `_CopyIntermediateAssemblies` target to update the timestamps of all files within this directory. It did not appear to be doing it correctly, and was only running `<Touch />` on a subset of assemblies. ~~ Problem 3 ~~ After fixing No. 2, the next problem I noticed were `*.pdb` and `*.mdb` files in `obj\Debug\linksrc` were out of date! The problem here was a call to `<Touch />`: <Touch Files="@(_DebugFilesCopiedToLinkerSrc)" /> The `_DebugFilesCopiedToLinkerSrc` did not appear to contain any items! I updated the `_CopyPdbFiles` and `_CopyMdbFiles` targets to use the correct item groups. ~~ Problem 4 ~~ Lastly, after fixing the previous three issues, the timestamps of output assemblies in `obj\Release\android\assets` were not correct for `Release` builds. I had made a change for this in the past, which was making `Debug` builds work correctly: dotnet#2028 At the time I wrote dotnet#2028, since the test wasn't using Xamarin.Forms, I did not see the problem ocurring in `Release` mode. It *does* occur if you are using NuGet packages. I replicated what I changed in the `_LinkAssembliesNoShrink` target, and also renamed the item groups to be unique: - `_LinkAssembliesNoShrinkFiles` - `_LinkAssembliesShrinkFiles` ~~ Other changes ~~ The `CheckTimestamps` test also was verifying timestamps in `$(OutputPath)` such as `bin\Debug`. It appears the core MSBuild targets are leaving timestamps as-is in the `CopyFilesToOutputDirectory` and `_CopyFilesMarkedCopyLocal` targets. Assemblies from Xamarin.Forms NuGet are out of date here, but I don't think we should do anything, since the core MSBuild targets are doing this. For now, I removed the checks for `$(OutputPath)` in this test, and cleaned up the test a bit.
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Aug 23, 2018
…s projects @StephaneDelcroix noticed a build performance problem in Xamarin.Forms projects: - Build - Build again with no changes - Slow targets such as `_UpdateAndroidResgen` and `_GenerateJavaStubs` run again! The cause appeared to be this: Building target "_UpdateAndroidResgen" completely. Input file "obj/Debug/res/layout/tabbar.xml" is newer than output file "obj/Debug/R.cs.flag". I modified the `CheckTimestamps` test to replicate this issue: - Added Xamarin.Forms NuGet packages - Added `Tabbar.axml` from the Xamarin.Forms template This surfaced several problems with timestamps and Xamarin.Android building incrementally... ~~ Problem 1 ~~ In the `_GenerateJavaStubs` target, the `ConvertResourcesCases` MSBuild task is not supplied a `AndroidConversionFlagFile`. It uses this file to compare timestamps and decide if XML files need fixed up or not. Supplying `$(_AndroidResgenFlagFile)` seemed to be the way to go here, since `_UpdateAndroidResgen` runs right before this task. It also solves the original problem from the `Tabbar.axml` file. I also realized we should be using a "stamp" file for the `_GenerateJavaStubs` MSBuild target, in general. This gives us several benefits: - `ConvertResourcesCases` has a proper `AndroidConversionFlagFile` it can use for checking timestamps - The `Outputs` of the target are drastically simplified, this should help peformance in MSBuild evaluating if the target should run - It is likely more precise/correct. So I added a new file in `$(IntermediateOutputPath)`, `obj\Debug\_javastubs.stamp` we use to determine if `_GenerateJavaStubs` should run. I also tried to do things "the right way", such as: - Adding the stamp file to the `FileWrites` item group - Made sure the file is deleted during a `Clean` After these changes, two instances of `ConvertResourcesCases` were *still* fighting each other. They would always run and update the timestamps on one file: causing the other one to run. `ConvertResourcesCases` runs twice: - In the `_UpdateAndroidResgen` target - In the `_GenerateJavaStubs` target But these are both working with the same files, so in the second case, we need to make sure `_GenerateJavaStubs` updates the timestamp on the *first* flag file so the target doesn't run every time. ~~ Problem 2 ~~ Timestamps of Xamarin.Forms assemblies in `obj\Debug\linksrc` were out of date! I updated the `_CopyIntermediateAssemblies` target to update the timestamps of all files within this directory. It did not appear to be doing it correctly, and was only running `<Touch />` on a subset of assemblies. ~~ Problem 3 ~~ After fixing No. 2, the next problem I noticed were `*.pdb` and `*.mdb` files in `obj\Debug\linksrc` were out of date! The problem here was a call to `<Touch />`: <Touch Files="@(_DebugFilesCopiedToLinkerSrc)" /> The `_DebugFilesCopiedToLinkerSrc` did not appear to contain any items! I updated the `_CopyPdbFiles` and `_CopyMdbFiles` targets to use the correct item groups. ~~ Problem 4 ~~ Lastly, after fixing the previous three issues, the timestamps of output assemblies in `obj\Release\android\assets` were not correct for `Release` builds. I had made a change for this in the past, which was making `Debug` builds work correctly: dotnet#2028 At the time I wrote dotnet#2028, since the test wasn't using Xamarin.Forms, I did not see the problem ocurring in `Release` mode. It *does* occur if you are using NuGet packages. I replicated what I changed in the `_LinkAssembliesNoShrink` target, and also renamed the item groups to be unique: - `_LinkAssembliesNoShrinkFiles` - `_LinkAssembliesShrinkFiles` ~~ Other changes ~~ The `CheckTimestamps` test also was verifying timestamps in `$(OutputPath)` such as `bin\Debug`. It appears the core MSBuild targets are leaving timestamps as-is in the `CopyFilesToOutputDirectory` and `_CopyFilesMarkedCopyLocal` targets. Assemblies from Xamarin.Forms NuGet are out of date here, but I don't think we should do anything, since the core MSBuild targets are doing this. For now, I removed the checks for `$(OutputPath)` in this test, and cleaned up the test a bit.
jonathanpeppers
added a commit
to jonathanpeppers/xamarin-android
that referenced
this pull request
Aug 30, 2018
…s projects @StephaneDelcroix noticed a build performance problem in Xamarin.Forms projects: - Build - Build again with no changes - Slow targets such as `_UpdateAndroidResgen` and `_GenerateJavaStubs` run again! The cause appeared to be this: Building target "_UpdateAndroidResgen" completely. Input file "obj/Debug/res/layout/tabbar.xml" is newer than output file "obj/Debug/R.cs.flag". I modified the `CheckTimestamps` test to replicate this issue: - Added Xamarin.Forms NuGet packages - Added `Tabbar.axml` from the Xamarin.Forms template This surfaced several problems with timestamps and Xamarin.Android building incrementally... ~~ Problem 1 ~~ In the `_GenerateJavaStubs` target, the `ConvertResourcesCases` MSBuild task is not supplied a `AndroidConversionFlagFile`. It uses this file to compare timestamps and decide if XML files need fixed up or not. Supplying `$(_AndroidResgenFlagFile)` seemed to be the way to go here, since `_UpdateAndroidResgen` runs right before this task. It also solves the original problem from the `Tabbar.axml` file. I also realized we should be using a "stamp" file for the `_GenerateJavaStubs` MSBuild target, in general. This gives us several benefits: - `ConvertResourcesCases` has a proper `AndroidConversionFlagFile` it can use for checking timestamps - The `Outputs` of the target are drastically simplified, this should help peformance in MSBuild evaluating if the target should run - It is likely more precise/correct. So I added a new file in `$(IntermediateOutputPath)`, `obj\Debug\_javastubs.stamp` we use to determine if `_GenerateJavaStubs` should run. I also tried to do things "the right way", such as: - Adding the stamp file to the `FileWrites` item group - Made sure the file is deleted during a `Clean` After these changes, two instances of `ConvertResourcesCases` were *still* fighting each other. They would always run and update the timestamps on one file: causing the other one to run. `ConvertResourcesCases` runs twice: - In the `_UpdateAndroidResgen` target - In the `_GenerateJavaStubs` target But these are both working with the same files, so in the second case, we need to make sure `_GenerateJavaStubs` updates the timestamp on the *first* flag file so the target doesn't run every time. ~~ Problem 2 ~~ Timestamps of Xamarin.Forms assemblies in `obj\Debug\linksrc` were out of date! I updated the `_CopyIntermediateAssemblies` target to update the timestamps of all files within this directory. It did not appear to be doing it correctly, and was only running `<Touch />` on a subset of assemblies. ~~ Problem 3 ~~ After fixing No. 2, the next problem I noticed were `*.pdb` and `*.mdb` files in `obj\Debug\linksrc` were out of date! The problem here was a call to `<Touch />`: <Touch Files="@(_DebugFilesCopiedToLinkerSrc)" /> The `_DebugFilesCopiedToLinkerSrc` did not appear to contain any items! I updated the `_CopyPdbFiles` and `_CopyMdbFiles` targets to use the correct item groups. ~~ Problem 4 ~~ Lastly, after fixing the previous three issues, the timestamps of output assemblies in `obj\Release\android\assets` were not correct for `Release` builds. I had made a change for this in the past, which was making `Debug` builds work correctly: dotnet#2028 At the time I wrote dotnet#2028, since the test wasn't using Xamarin.Forms, I did not see the problem ocurring in `Release` mode. It *does* occur if you are using NuGet packages. I replicated what I changed in the `_LinkAssembliesNoShrink` target, and also renamed the item groups to be unique: - `_LinkAssembliesNoShrinkFiles` - `_LinkAssembliesShrinkFiles` ~~ Other changes ~~ The `CheckTimestamps` test also was verifying timestamps in `$(OutputPath)` such as `bin\Debug`. It appears the core MSBuild targets are leaving timestamps as-is in the `CopyFilesToOutputDirectory` and `_CopyFilesMarkedCopyLocal` targets. Assemblies from Xamarin.Forms NuGet are out of date here, but I don't think we should do anything, since the core MSBuild targets are doing this. For now, I removed the checks for `$(OutputPath)` in this test, and cleaned up the test a bit.
jonpryor
pushed a commit
that referenced
this pull request
Aug 31, 2018
…s projects (#2088) @StephaneDelcroix noticed a build performance problem when building Xamarin.Forms projects: - Build - Build again with no changes - Slow targets such as `_UpdateAndroidResgen` and `_GenerateJavaStubs` run again! The cause appeared to be this: Building target "_UpdateAndroidResgen" completely. Input file "obj/Debug/res/layout/tabbar.xml" is newer than output file "obj/Debug/R.cs.flag". I modified the `CheckTimestamps()` test to replicate this issue: - Added Xamarin.Forms NuGet packages - Added `Tabbar.axml` from the Xamarin.Forms template This surfaced several problems with timestamps and Xamarin.Android building incrementally. ~~ Problem 1 ~~ In the `_GenerateJavaStubs` target, the `<ConvertResourcesCases/>` MSBuild task is not supplied an `AndroidConversionFlagFile`. It uses this file to compare timestamps and decide if XML files need to be fixed up or not. Supplying `$(_AndroidResgenFlagFile)` seemed to be the way to go here, since `_UpdateAndroidResgen` runs right before this task. It also solves the original problem from the `Tabbar.axml` file. I also realized we should be using a "stamp" file for the `_GenerateJavaStubs` MSBuild target, in general. This gives us several benefits: - `<ConvertResourcesCases/>` has a proper `AndroidConversionFlagFile` it can use for checking timestamps - The `Outputs` of the target are drastically simplified, which may help performance in MSBuild evaluating if the target should run - It is likely more precise/correct. So I added a new file in `$(IntermediateOutputPath)`, `obj\Debug\_javastubs.stamp` we use to determine if `_GenerateJavaStubs` should run. I also tried to do things "the right way", such as: - Adding the stamp file to the `@(FileWrites)` item group - Made sure the file is deleted during a `Clean` After these changes, two instances of `<ConvertResourcesCases/>` were *still* fighting each other. They would always run and update the timestamps on one file, causing the other one to run. `ConvertResourcesCases` runs twice: - In the `_UpdateAndroidResgen` target - In the `_GenerateJavaStubs` target But these are both working with the same files, so in the second case, we need to make sure `_GenerateJavaStubs` updates the timestamp on the *first* flag file so the target doesn't run every time. ~~ Problem 2 ~~ Timestamps of Xamarin.Forms assemblies in `obj\Debug\linksrc` were out of date! I updated the `_CopyIntermediateAssemblies` target to update the timestamps of all files within this directory. It did not appear to be doing it correctly, and was only running `<Touch/>` on a subset of assemblies. ~~ Problem 3 ~~ After fixing Problem 2, the next problem I noticed were `*.pdb` and `*.mdb` files in `obj\Debug\linksrc` were out of date! The problem here was a call to `<Touch />`: <Touch Files="@(_DebugFilesCopiedToLinkerSrc)" /> The `@(_DebugFilesCopiedToLinkerSrc)` did not appear to contain any items! I updated the `_CopyPdbFiles` and `_CopyMdbFiles` targets to use the correct item groups. ~~ Problem 4 ~~ Lastly, after fixing the previous three problems, the timestamps of output assemblies in `obj\Release\android\assets` were not correct for `Release` builds. I had made a change for this in the past, which was making `Debug` builds work correctly: #2028 At the time I wrote #2028, since the test wasn't using Xamarin.Forms, I did not see the problem occurring in `Release` mode. It *does* occur if you are using NuGet packages. I replicated what I changed in the `_LinkAssembliesNoShrink` target, and also renamed the item groups to be unique: - `_LinkAssembliesNoShrinkFiles` - `_LinkAssembliesShrinkFiles` ~~ Other changes ~~ The `CheckTimestamps()` test also was verifying timestamps in `$(OutputPath)` such as `bin\Debug`. It appears the core MSBuild targets are leaving timestamps as-is in the `CopyFilesToOutputDirectory` and `_CopyFilesMarkedCopyLocal` targets. Assemblies from Xamarin.Forms NuGet are out of date here, but I don't think we should do anything, since the core MSBuild targets are doing this. For now, I removed the checks for `$(OutputPath)` in this test, and cleaned up the test a bit. Update `Files.ExtractAll()` so that the timestamps of extracted files are set to "now", in order to fix some failing unit tests on macOS.
jonpryor
pushed a commit
that referenced
this pull request
Sep 5, 2018
…s projects (#2088) @StephaneDelcroix noticed a build performance problem when building Xamarin.Forms projects: - Build - Build again with no changes - Slow targets such as `_UpdateAndroidResgen` and `_GenerateJavaStubs` run again! The cause appeared to be this: Building target "_UpdateAndroidResgen" completely. Input file "obj/Debug/res/layout/tabbar.xml" is newer than output file "obj/Debug/R.cs.flag". I modified the `CheckTimestamps()` test to replicate this issue: - Added Xamarin.Forms NuGet packages - Added `Tabbar.axml` from the Xamarin.Forms template This surfaced several problems with timestamps and Xamarin.Android building incrementally. ~~ Problem 1 ~~ In the `_GenerateJavaStubs` target, the `<ConvertResourcesCases/>` MSBuild task is not supplied an `AndroidConversionFlagFile`. It uses this file to compare timestamps and decide if XML files need to be fixed up or not. Supplying `$(_AndroidResgenFlagFile)` seemed to be the way to go here, since `_UpdateAndroidResgen` runs right before this task. It also solves the original problem from the `Tabbar.axml` file. I also realized we should be using a "stamp" file for the `_GenerateJavaStubs` MSBuild target, in general. This gives us several benefits: - `<ConvertResourcesCases/>` has a proper `AndroidConversionFlagFile` it can use for checking timestamps - The `Outputs` of the target are drastically simplified, which may help performance in MSBuild evaluating if the target should run - It is likely more precise/correct. So I added a new file in `$(IntermediateOutputPath)`, `obj\Debug\_javastubs.stamp` we use to determine if `_GenerateJavaStubs` should run. I also tried to do things "the right way", such as: - Adding the stamp file to the `@(FileWrites)` item group - Made sure the file is deleted during a `Clean` After these changes, two instances of `<ConvertResourcesCases/>` were *still* fighting each other. They would always run and update the timestamps on one file, causing the other one to run. `ConvertResourcesCases` runs twice: - In the `_UpdateAndroidResgen` target - In the `_GenerateJavaStubs` target But these are both working with the same files, so in the second case, we need to make sure `_GenerateJavaStubs` updates the timestamp on the *first* flag file so the target doesn't run every time. ~~ Problem 2 ~~ Timestamps of Xamarin.Forms assemblies in `obj\Debug\linksrc` were out of date! I updated the `_CopyIntermediateAssemblies` target to update the timestamps of all files within this directory. It did not appear to be doing it correctly, and was only running `<Touch/>` on a subset of assemblies. ~~ Problem 3 ~~ After fixing Problem 2, the next problem I noticed were `*.pdb` and `*.mdb` files in `obj\Debug\linksrc` were out of date! The problem here was a call to `<Touch />`: <Touch Files="@(_DebugFilesCopiedToLinkerSrc)" /> The `@(_DebugFilesCopiedToLinkerSrc)` did not appear to contain any items! I updated the `_CopyPdbFiles` and `_CopyMdbFiles` targets to use the correct item groups. ~~ Problem 4 ~~ Lastly, after fixing the previous three problems, the timestamps of output assemblies in `obj\Release\android\assets` were not correct for `Release` builds. I had made a change for this in the past, which was making `Debug` builds work correctly: #2028 At the time I wrote #2028, since the test wasn't using Xamarin.Forms, I did not see the problem occurring in `Release` mode. It *does* occur if you are using NuGet packages. I replicated what I changed in the `_LinkAssembliesNoShrink` target, and also renamed the item groups to be unique: - `_LinkAssembliesNoShrinkFiles` - `_LinkAssembliesShrinkFiles` ~~ Other changes ~~ The `CheckTimestamps()` test also was verifying timestamps in `$(OutputPath)` such as `bin\Debug`. It appears the core MSBuild targets are leaving timestamps as-is in the `CopyFilesToOutputDirectory` and `_CopyFilesMarkedCopyLocal` targets. Assemblies from Xamarin.Forms NuGet are out of date here, but I don't think we should do anything, since the core MSBuild targets are doing this. For now, I removed the checks for `$(OutputPath)` in this test, and cleaned up the test a bit. Update `Files.ExtractAll()` so that the timestamps of extracted files are set to "now", in order to fix some failing unit tests on macOS.
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: 3d999d3#diff-42a9402e6466c65d49d0ee7caf21f327R164
Since 3d999d3, a test is failing downstream in monodroid that is
testing the following scenario:
Fast Deploymentenabled, run/t:Install/t:Installagainup to date!
3d999d3 was addressing a symptom of the problem, but not the root
cause. The
OutputStepof the linker has various uses ofFile.Copy,which preserves the timestamp of the source file.
https://github.com/mono/linker/blob/615f62c32705322aeb0f3f15212a3dbb94b7f0aa/linker/Linker.Steps/OutputStep.cs#L187
The solution, then, is to run the
<Touch />MSBuild task on thelinker's output. This will keep the proper timestamps, and a portion
of the changes in 3d999d3 are no longer needed.
I updated the
CheckTimestampstest to validateDebugandReleaseconfigurations, so
_LinkAssembliesShrinkis also tested in thismanner. It turns out
_LinkAssembliesShrinkis currently workingfine, since it uses
$(_AndroidLinkFlag)as itsOutputs.