[Xamarin.Android.Build.Tasks] Fix CollectNonEmptyDirectories to handle file collisions. #5038
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
|
Draft release notes During the finalization of this PR for merging, when you get a chance, you can add a short release note for it to |
dellis1972
force-pushed
the
FixFilesCache
branch
from
f3ecc7a
to
75dc4da
Compare
…e file collisions. Context Redth/ResizetizerNT#23 The `CollectNonEmptyDirectories` task is responsible for creating a cache file `files.cache`. This file contains a list of all the resource files for that particular directory. We hit a problem when using `ResizetizerNT` and a `google-services.json` file where the `files.cache` for one would be overwritten by the other. This is because of a simple assuption within the Task itself. That if the "directory" was not in the Intermediate `lp` directory (for libraries) then we could just create the `files.cache` file in the parent directory. This woud be the root of the `IntermediateOutputPath`. The problem with `ResizetizerNT` and `google-services.json` is that they are BOTH outside of the `lp` directory. They also only create a directory structure which is one level deep. So `..` will ALWAYS end up in the `IntermediateOutputPath`. The other issue is that they were both using the same file name. #facepalm. The `files.cache` should instead be handled in the same manner as the `stamp` file. In that if the file is NOT in the `lp` directory we should use a unique filename. In this case we can use the same technique and use the directory hash. This way the two directories do not collide with each other.
dellis1972
force-pushed
the
FixFilesCache
branch
from
75dc4da
to
072e415
Compare
jonathanpeppers
approved these changes
Aug 24, 2020
There was a problem hiding this comment.
The BCL test failures are expected: https://discordapp.com/channels/732297728826277939/732297837953679412/746334964462780476
Something broke with the he-IL calendar's new year.
The one performance test looks OK, I didn't see any regression in the .binlog: Exceeded expected time of 7250ms, actual 7274.528ms.
brendanzagaeski
approved these changes
Aug 25, 2020
There was a problem hiding this comment.
Release note approved. Thanks for the correction on the error message. In the final collated release notes, I'll plan to adjust the formatting to put the example error message up front and limit the resolution details for compactness. Feel free to comment back if anything looks bad about this adjustment though.
- [GitHub PR 5038](https://github.com/xamarin/xamarin-android/pull/5038):
Errors similar to _error APT2260: resource drawable/icon (aka
com.contoso.androidapp:drawable/icon) not found_ could prevent
building projects that were using ResizetizerNT and
`google-services.json` together.
The build system has now been updated to ensure each resource
directory has its own unique cache file.
jonpryor
pushed a commit
that referenced
this pull request
Aug 27, 2020
…e file collisions. (#5038) Context Redth/ResizetizerNT#23 The `CollectNonEmptyDirectories` task is responsible for creating a cache file `files.cache`. This file contains a list of all the resource files for that particular directory. We hit a problem when using `ResizetizerNT` and a `google-services.json` file where the `files.cache` for one would be overwritten by the other. This is because of a simple assuption within the Task itself. That if the "directory" was not in the Intermediate `lp` directory (for libraries) then we could just create the `files.cache` file in the parent directory. This woud be the root of the `IntermediateOutputPath`. The problem with `ResizetizerNT` and `google-services.json` is that they are BOTH outside of the `lp` directory. They also only create a directory structure which is one level deep. So `..` will ALWAYS end up in the `IntermediateOutputPath`. The other issue is that they were both using the same file name. #facepalm. The `files.cache` should instead be handled in the same manner as the `stamp` file. In that if the file is NOT in the `lp` directory we should use a unique filename. In this case we can use the same technique and use the directory hash. This way the two directories do not collide with each other.
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 Redth/ResizetizerNT#23
The
CollectNonEmptyDirectoriestask is responsible forcreating a cache file
files.cache. This file containsa list of all the resource files for that particular
directory.
We hit a problem when using
ResizetizerNTand agoogle-services.jsonfile where thefiles.cacheforone would be overwritten by the other. This is because of
a simple assuption within the Task itself. That if the
"directory" was not in the Intermediate
lpdirectory(for libraries) then we could just create the
files.cachefile in the parent directory. This woud be the root of
the
IntermediateOutputPath.The problem with
ResizetizerNTandgoogle-services.jsonis thatthey are BOTH outside of the
lpdirectory. They also only createa directory structure which is one level deep. So
..will ALWAYSend up in the
IntermediateOutputPath. The other issue is that theywere both using the same file name. #facepalm.
The
files.cacheshould instead be handled in the same manner asthe
stampfile. In that if the file is NOT in thelpdirectorywe should use a unique filename. In this case we can use the same
technique and use the directory hash. This way the two directories
do not collide with each other.