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] ignore aapt/AndroidManifest.xml #2144

Merged
merged 2 commits into from
Sep 7, 2018

Conversation

jonathanpeppers
Copy link
Member

Context: https://stackoverflow.com/questions/41592744/function-of-aapt-androidmanifest-xml-in-aar
Fixes: #2141

Apparently Android Studio is now shipping a duplicate, pre-formatted
AndroidManifest.xml file, inside of AAR files. Its purpose is to be
used with aapt invocations, since errors may be thrown related to
{ or } characters.

Since this file may exist in AAR files used by Xamarin.Android, we
should ignore aapt/AndroidManifest.xml files the same way we ignore
them inside manifest or bin directories.

Changes:

  • Added a IgnoredManifestDirectories list, since we are getting to a
    point where != && != && != would be a lot of noise.
  • Switched the LINQ expression to simple foreach loop, which should
    also give a slight performance benefit.

Context: https://stackoverflow.com/questions/41592744/function-of-aapt-androidmanifest-xml-in-aar
Fixes: dotnet#2141

Apparently Android Studio is now shipping a duplicate, pre-formatted
`AndroidManifest.xml` file, inside of AAR files. Its purpose is to be
used with `aapt` invocations, since errors may be thrown related to
`{` or `}` characters.

Since this file may exist in AAR files used by Xamarin.Android, we
should ignore `aapt/AndroidManifest.xml` files the same way we ignore
them inside `manifest` or `bin` directories.

Changes:
- Added a `IgnoredManifestDirectories` list, since we are getting to a
  point where `!= && != && !=` would be a lot of noise.
- Switched the LINQ expression to simple `foreach` loop, which should
  also give a slight performance benefit.
@dellis1972
Copy link
Contributor

Looks good. merge when green :)

@dellis1972
Copy link
Contributor

@jonathanpeppers this broke you new test

  "/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ExtraAaptManifest/UnnamedProject.csproj" (Build target) (1) ->
  (_CompileJava target) -> 
obj/Debug/android/src/mono/io/fabric/sdk/android/services/events/EventsStorageListenerImplementor.java(8,40): error : error: package io.fabric.sdk.android.services.events does not exist [/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ExtraAaptManifest/UnnamedProject.csproj] [/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/build-tools/scripts/RunTests.targets]
obj/Debug/android/src/mono/io/fabric/sdk/android/services/events/EventsStorageListenerImplementor.java(8,40): error : io.fabric.sdk.android.services.events.EventsStorageListener [/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ExtraAaptManifest/UnnamedProject.csproj] [/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/build-tools/scripts/RunTests.targets]
obj/Debug/android/src/mono/io/fabric/sdk/android/services/events/EventsStorageListenerImplementor.java(8,40): error : [/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ExtraAaptManifest/UnnamedProject.csproj] [/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/build-tools/scripts/RunTests.targets]
  
      0 Warning(s)
      1 Error(s)

@jonathanpeppers
Copy link
Member Author

Weird it didn't show up as a failed test.

Trying locally again, maybe it works on Windows-only.

@dellis1972 dellis1972 merged commit f6c3288 into dotnet:master Sep 7, 2018
@jonathanpeppers jonathanpeppers deleted the androidmanifest-aapt branch December 14, 2018 16:30
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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