[Xamarin.Android.Build.Tasks] fix asset subfolders with aapt2 on Windows #2480
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
jonathanpeppers
commented
Dec 3, 2018
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildAssetsTest.cs
Outdated
Show resolved
Hide resolved
jonathanpeppers
commented
Dec 3, 2018
This should be validated, and an upstream bug filed if possible. If it is an |
jonathanpeppers
commented
Dec 3, 2018
|
Bug filed for |
|
It looks like this fix is working. The Windows failures here are tests that can randomly fail on a new build machine (I'll be looking into that later). Let's get #2482 in and I can clean this PR up, and get it to a spot it can be merged. |
jonpryor
pushed a commit
that referenced
this pull request
Dec 5, 2018
Context: #2480 The `ZipEntry.FullName` property was normalizing paths to always use `/` as a directory separator character. This meant that it was not possible to [detect an `aapt2` bug][0] in which `aapt2`-generated `.flata` files would contain `\` within the filename, e.g. assets\subfolder/asset1.txt `ZipEntry.FullName` would return `assets/subfolder/asset1.txt`, and there was no way to obtain the original/"source" value of `assets\subfolder/asset1.txt`. Bump to grendello/LibZipSharp/master@de1712c8 which adds a new `ZipEntry.NativeFullName` property which contains the original filename as present in the zip file -- `assets\subfolder/asset1.txt` in the above example -- so that we can intelligently check for this scenario and react accordingly in PR #2480. Additionally, `ZipEntry.Rename()` was added, which allows renaming a path within a zip file without needing to stream/rewrite the file contents of that path. [0]: https://issuetracker.google.com/issues/120436372
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2249299&view=ms.vss-test-web.test-result-details Since 0f91aea enabled aapt2 support by default, the following test has been failing on Windows: Xamarin.Android.Build.Tests.BuildAssetsTest.CheckAssetsAreIncludedInAPK / Debug assets/subfolder/asset2.txt should be in the apk. Expected: not null But was: null Looking into it locally, the `packaged_resources` file has a weird file path for assets in sub-directories: assets\subfolder/asset2.txt It appears this command line was invoked for aapt2: Executing link --manifest C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckAssetsAreIncludedInAPK\App1\obj\Release\android\manifest\AndroidManifest.xml --java C:\Users\myuser\AppData\Local\Temp\pjkd3dqd.4na --custom-package unnamedproject.unnamedproject -R obj\Release\flata\227168ee3afdc4839bf05d172c1a56a15a421ac7.flata -R obj\Release\flata\\compiled.flata --auto-add-overlay -I C:\Users\myuser\android-toolchain\sdk\platforms\android-28\android.jar -A C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckAssetsAreIncludedInAPK\App1\obj\Release\assets -o C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckAssetsAreIncludedInAPK\App1\obj\Release\android\bin\packaged_resources But what makes matters worse, is the asset in the sub-directory is not useable if the app was built on Windows!!! Everything is pointing to a bug in aapt2 on Windows, so I filed: https://issuetracker.google.com/issues/120436372 ~~ Changes ~~ - Updated `CheckAssetsAreIncludedInAPK` test to check aapt and aapt2. - Added an `AssetManagerTest` to `Mono.Android-Tests`, to verify we can load asset files at runtime. - Updated the `<BuildApk/>` MSBuild task to call a new `FixupWindowsPathSeparators` method on `ZipArchiveEx`. - `FixupWindowsPathSeparators` now looks for the `\` characters in entries within the zip file, and swaps with the `/` if any are found.
jonathanpeppers
force-pushed
the
aapt2-assets-windows
branch
from
bdc6a83
to
ebf206f
Compare
jonathanpeppers
changed the title
|
macOS test failure seems unrelated? |
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: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2249299&view=ms.vss-test-web.test-result-details
Since 0f91aea enabled aapt2 support by default, the following test
has been failing on Windows:
Looking into it locally, the
packaged_resourcesfile has a weirdfile path for assets in sub-directories:
It appears this command line was invoked for aapt2:
But what makes matters worse, is the asset in the sub-directory is not
useable if the app was built on Windows!!!
Everything is pointing to a bug in aapt2 on Windows, so I filed:
https://issuetracker.google.com/issues/120436372
Changes
CheckAssetsAreIncludedInAPKtest to check aapt and aapt2.AssetManagerTesttoMono.Android-Tests, to verify wecan load asset files at runtime.
<BuildApk/>MSBuild task to call a newFixupWindowsPathSeparatorsmethod onZipArchiveEx.FixupWindowsPathSeparatorsnow looks for the\characters inentries within the zip file, and swaps with the
/if any arefound.