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

Only conditionally include <uses-sdk /> in the AndroidManifest.xml wh… #227

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

jstedfast
Copy link
Member

@jstedfast jstedfast requested a review from tondat February 7, 2024 15:58
@tondat tondat requested a review from jonpryor February 7, 2024 16:12
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

So the design for these should be that:

  • $(TargetPlatformVersion) is <uses-sdk android:targetSdkVersion
  • $(SupportedOSPlatformVersion) is <uses-sdk android:minSdkVersion

$(SupportedOSPlatformVersion) is the only one that is possible to be blank, but it's in the project template.

Details here: https://learn.microsoft.com/en-us/dotnet/maui/migration/android-projects?view=net-maui-8.0#changes-to-androidmanifestxml

In what case are you able to make both of these blank?

@jstedfast
Copy link
Member Author

@jonathanpeppers net-android projects apparently don't build if AndroidManifest.xml has <uses-sdk />

What this patch does is remove the uses-sdk XML node if and only if it has no attributes.

Without this change, the <uses-sdk /> XML element keeps getting added (back) to the AndroidManifest.xml file whenever the user edits that file in the UI editor in VS.

In other words, there's no way for the user to remove it unless they edit it in a text editor and manually remove it.

But if they go back into the UI editor ever in the future to change something, it will get added right back.

This patch fixes that scenario.

@jstedfast
Copy link
Member Author

@jonathanpeppers I just added some tests

@jstedfast jstedfast merged commit a698a33 into main Feb 8, 2024
4 checks passed
@jstedfast jstedfast deleted the dev/jestedfa/uses-sdk branch February 8, 2024 16:35
jonpryor pushed a commit to dotnet/java-interop that referenced this pull request Feb 9, 2024
Changes: dotnet/android-tools@ed102fc...a698a33

  * dotnet/android-tools@a698a33: Merge pull request dotnet/android-tools#227 from xamarin/dev/jestedfa/uses-sdk
  * dotnet/android-tools@8b13954: Added unit tests
  * dotnet/android-tools@84c2911: Only conditionally include <uses-sdk /> in the AndroidManifest.xml when saving

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
jonpryor pushed a commit to dotnet/android that referenced this pull request Feb 9, 2024
Changes: dotnet/android-tools@ed102fc...a698a33

  * dotnet/android-tools@a698a33: Merge pull request dotnet/android-tools#227 from xamarin/dev/jestedfa/uses-sdk
  * dotnet/android-tools@8b13954: Added unit tests
  * dotnet/android-tools@84c2911: Only conditionally include <uses-sdk /> in the AndroidManifest.xml when saving

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants