Add Microsoft.Android.Build.BaseTasks project #101
Merged
Conversation
This file contains hidden or 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
We are hoping to sever the xamarin-android dependency from monodroid. In order to do this we'll want to share some common MSBuild code across our two build task assemblies, `Xamarin.Android.Build.Tasks` and `Xamarin.Android.Build.Debugging.Tasks`. This common code has been moved into this repo as part of a new project named `Microsoft.Android.Build.BaseTasks`. More utilities can be migrated here in the future if we so desire.
pjcollins
commented
Dec 16, 2020
src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj
Outdated
Show resolved
Hide resolved
jonpryor
reviewed
Jan 1, 2021
jonpryor
reviewed
Jan 1, 2021
|
|
||
| public abstract string TaskPrefix { get; } | ||
|
|
||
| public override bool Execute () |
Contributor
There was a problem hiding this comment.
Should this be override sealed, so that it can't be further overridden?
Contributor
There was a problem hiding this comment.
No, because under certain circumstances we still need to override execute. One example is when calling the MSBuild GetRegisterTaskObject, that needs to be done from the UI thread.
jonpryor
reviewed
Jan 1, 2021
|
|
||
| // Most ToolTask's do not override Execute and | ||
| // just expect the base to be called | ||
| public virtual bool RunTask () => base.Execute (); |
Contributor
There was a problem hiding this comment.
I'm not sure I understand how this "works" w/o causing stack overflow if the subclass doesn't "do" anything:
class ExampleTask : AndroidToolTask {
public override string TaskPrefix => "FOO";
}Wouldn't AndroidToolTask.Execute() all AndroidToolTask.RunTask() call .Execute() call .RunTask() call…?
"Feels like" this should instead have RunTask() be abstract, and Execute() should be override sealed.
Member
There was a problem hiding this comment.
This is:
AndroidToolTask.Execute()AndroidToolTask.RunTask()base.Execute()orToolTask.Execute()from MSBuild
Hopefully, should not be a stack overflow.
I don't think we can mark anything sealed as mentioned: #101 (comment)
jonpryor
reviewed
Jan 1, 2021
pjcollins
added a commit
to dotnet/android
that referenced
this pull request
Jan 5, 2021
Context: dotnet/android-tools#101 Context: xamarin/monodroid#1145 We would like to eventually remove the dependency that xamarin/monodroid has on xamarin/xamarin-android. As a first step towards this effort, shared MSBuild code is being moved to xamarin/xamarin-android-tools. Xamarin.Android.Build.Tasks has been updated to use the new shared Microsoft.Android.Build.Tasks sources. Our build steps have also been updated to use the new and simplified monodroid build logic.
pjcollins
added a commit
to dotnet/android
that referenced
this pull request
Jan 5, 2021
Context: dotnet/android-tools#101 Context: xamarin/monodroid#1145 We would like to eventually remove the dependency that xamarin/monodroid has on xamarin/xamarin-android. As a first step towards this effort, shared MSBuild code is being moved to xamarin/xamarin-android-tools. Xamarin.Android.Build.Tasks has been updated to use the new shared Microsoft.Android.Build.Tasks sources. Our build steps have also been updated to use the new and simplified monodroid build logic.
pjcollins
added a commit
to dotnet/android
that referenced
this pull request
Jan 5, 2021
Context: dotnet/android-tools#101 We would like to eventually remove the dependency that xamarin/monodroid has on xamarin/xamarin-android. As a first step towards this effort, shared MSBuild code is being moved to xamarin/xamarin-android-tools. Xamarin.Android.Build.Tasks has been updated to use the new shared Microsoft.Android.Build.Tasks sources. Relevant tests have been moved into xamarin/xamarin-android-tools to a new `Microsoft.Android.Build.BaseTasks-Tests` assembly.
pjcollins
added a commit
to dotnet/android
that referenced
this pull request
Jan 5, 2021
Context: dotnet/android-tools#101 We would like to eventually remove the dependency that xamarin/monodroid has on xamarin/xamarin-android. As a first step towards this effort, shared MSBuild code is being moved to xamarin/xamarin-android-tools. Xamarin.Android.Build.Tasks has been updated to use the new shared Microsoft.Android.Build.Tasks sources. Relevant tests have been moved into xamarin/xamarin-android-tools to a new `Microsoft.Android.Build.BaseTasks-Tests` assembly.
pjcollins
added a commit
to dotnet/android
that referenced
this pull request
Jan 6, 2021
Context: dotnet/android-tools#101 We would like to eventually remove the dependency that xamarin/monodroid has on xamarin/xamarin-android. As a first step towards this effort, shared MSBuild code is being moved to xamarin/xamarin-android-tools. Xamarin.Android.Build.Tasks has been updated to use the new shared Microsoft.Android.Build.Tasks sources. Relevant tests have been moved into xamarin/xamarin-android-tools to a new `Microsoft.Android.Build.BaseTasks-Tests` assembly.
Member
Author
|
This should be ready to go, but we're going to wait to merge this until the week of the 18th or later. There are a lot of connected pieces that should land at the same time, and we want to give d16-9 a little more time before merging all of them as all of these changes will make backporting difficult. |
jonathanpeppers
approved these changes
Feb 1, 2021
pjcollins
added a commit
to dotnet/android
that referenced
this pull request
Feb 5, 2021
…5464) Context: dotnet/android-tools#101 Context: xamarin/monodroid#1145 We would like to eventually remove the dependency that xamarin/monodroid has on xamarin/xamarin-android. As a first step towards this effort, shared MSBuild code is being moved to xamarin/xamarin-android-tools. `Xamarin.Android.Build.Tasks` has been updated to use the new shared `Microsoft.Android.Build.BaseTasks` sources. All of the files that were moved to xamarin/xamarin-android-tools have been deleted. The inverted monodroid build has been replaced with the new monodroid build system, see xamarin/monodroid#1145 for more context. The one build task test that was still being ran from the xamarin/monodroid repo has been moved into `DebuggingTasksTests.cs`, which is conditionally included in `Xamarin.Android.Build.Tests`. Tests associated with the build task source that was moved to xamarin/xamarin-android-tools have also been moved to a new `Microsoft.Android.Build.BaseTasks-Tests` assembly in that repo.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We are hoping to sever the xamarin-android dependency from monodroid.
In order to do this we'll want to share some common MSBuild code across
our two build task assemblies,
Xamarin.Android.Build.TasksandXamarin.Android.Build.Debugging.Tasks.This common code has been moved from xamarin/xamarin-android into
a new project in this repo named
Microsoft.Android.Build.BaseTasks.More utilities can be migrated here in the future if we so desire.
A new test project
Microsoft.Android.Build.BaseTasks-Tests.csprojhasalso been added to bring over the relevant tests from xamarin-android.