Skip to content
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] use designtime/build.props #1957

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Jul 12, 2018

Fixes: #1958
Context: #1933
Context: #1943

One of the issues I noticed while debugging the issue with #1933 is
that $(IntermediateOutputPath)build.props gets invalidated if
$(DesignTimeBuild) changes. build.props triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a obj/Debug/designtime/build.props that
works independantly of build.props. This prevents some slower
targets from running when they shouldn't, such as
_UpdateAndroidResgen.

I added a test to validate these changes, which also verify that
IncrementalClean isn't deleting these files.

It also is important to point out that any files in the designtime
directory are not deleted during a Clean. Context on this here:
34a3774

Other changes:

  • Updated .gitignore for *.binlog
  • Renamed _AndroidDesignTimeResDirIntermediate to
    _AndroidIntermediateDesignTimeBuildDirectory and declared it
    earlier in Xamarin.Android.Common.targets

@jonathanpeppers jonathanpeppers added the do-not-merge PR should not be merged. label Jul 12, 2018
@jonathanpeppers
Copy link
Member Author

I think we should wait on #1930 to merge this, as I want to verify _LinkAssembliesNoShrink isn't running.

@dellis1972
Copy link
Contributor

dellis1972 commented Jul 13, 2018 via email

@dellis1972
Copy link
Contributor

Can we rebase on master, so we get the #1930 fix in.

@@ -305,6 +305,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<_AndroidSequencePointsMode Condition=" '$(_AndroidSequencePointsMode)' == ''">None</_AndroidSequencePointsMode>
<_InstantRunEnabled Condition=" '$(_InstantRunEnabled)' == '' ">False</_InstantRunEnabled>
<_AndroidBuildPropertiesCache>$(IntermediateOutputPath)build.props</_AndroidBuildPropertiesCache>
<_AndroidDesignTimeBuildPropertiesCache>$(IntermediateOutputPath)designtimebuild.props</_AndroidDesignTimeBuildPropertiesCache>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know its just a file however I would place this in the designtime folder like the other files. This keeps it completely isolated.

Copy link
Member Author

@jonathanpeppers jonathanpeppers Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I was having was that $(_AndroidDesignTimeResDirIntermediate) was defined later.

I can probably move things around to get this to work.

@jonathanpeppers
Copy link
Member Author

The problem I'm having now is _LinkAssembliesNoShrink is still running after rebasing... 😕

Something due to obj\Debug\UnnamedProject.csproj.CoreCompileInputs.cache changing. Looking into it.

@jonathanpeppers jonathanpeppers force-pushed the designtimebuild.props branch from df00b0f to 4898245 Compare July 13, 2018 14:49
@jonathanpeppers jonathanpeppers removed the do-not-merge PR should not be merged. label Jul 13, 2018
@jonathanpeppers
Copy link
Member Author

I think _LinkAssembliesNoShrink is always going to run here, because:

  • The <Compile /> item group changes: $(IntermediateOutputPath)designtime\Resource.designer.cs -> Resources\Resource.designer.cs
  • A design-time build with write the assembly to obj\Debug\MyAssembly.dll
  • Both of these cause _LinkAssembliesNoShrink to run

@jonathanpeppers jonathanpeppers changed the title [Xamarin.Android.Build.Tasks] use designtimebuild.props [Xamarin.Android.Build.Tasks] use designtime/build.props Jul 13, 2018
@jonathanpeppers jonathanpeppers force-pushed the designtimebuild.props branch from 4898245 to 9bb5298 Compare July 17, 2018 16:01
Fixes: dotnet#1958
Context: dotnet#1933
Context: dotnet#1943

One of the issues I noticed while debugging the issue with dotnet#1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

It also is important to point out that any files in the `designtime`
directory are not deleted during a `Clean`. Context on this here:
dotnet@34a3774

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
@jonathanpeppers jonathanpeppers force-pushed the designtimebuild.props branch from 9bb5298 to 6fa4a61 Compare July 17, 2018 18:31
@dellis1972
Copy link
Contributor

I'm hitting this issue on my instant run since I updated x-a to the latest master. I'll give this a another review and merge if it looks ok

@dellis1972 dellis1972 merged commit 3b3eba4 into dotnet:master Jul 19, 2018
@jonathanpeppers jonathanpeppers deleted the designtimebuild.props branch July 19, 2018 14:27
jonpryor pushed a commit that referenced this pull request Aug 3, 2018
Fixes: #1958
Context: #1933
Context: #1943

One of the issues I noticed while debugging the issue with #1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

It also is important to point out that any files in the `designtime`
directory are not deleted during a `Clean`. Context on this here:
34a3774

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants