Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jul 8, 2020

Description

Fixes #45075
Fixes #57210

If an asset was included directly from the project root directory, then the same asset when copied to various output or ephemeral directories would also be picked up as an asset variant. This could cause assets to be recursively copied into asset/build/ephemeral directories, as each time it would run it would pick up all of the previous "variants".

The solution is to include project ephemeral directories, in addition to the build directory.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 8, 2020
@jonahwilliams jonahwilliams requested a review from jmagman July 8, 2020 21:33
final String path = entity.path;
if (globals.fs.isFileSync(path) && !_excluded.any((String exclude) => path.startsWith(exclude))) {
if (globals.fs.isFileSync(path)
&& assetPath != path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to check the assetPath != path, otherwise the tool recognizes assets: - foo.txt as its own asset variant.

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM

@jmagman
Copy link
Member

jmagman commented Jul 8, 2020

info • Unused import: 'package:flutter_tools/src/project.dart' • packages/flutter_tools/test/general.shard/asset_bundle_test.dart:16:8 • unused_import

/// Constructing DeviceManagers is cheap; they only do expensive work if some
/// of their methods are called.
List<DeviceDiscovery> get deviceDiscoverers => _deviceDiscoverers;
final List<DeviceDiscovery> _deviceDiscoverers = List<DeviceDiscovery>.unmodifiable(<DeviceDiscovery>[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was getting hit in devices_test and failing if it grabbed the wrong context, I did some minor cleanup but it needs to be fixed later so that there is no accidental caching

@jonahwilliams jonahwilliams merged commit 741608a into flutter:master Jul 9, 2020
@jonahwilliams jonahwilliams deleted the exclude_dirs branch July 9, 2020 01:07
christopherfujino pushed a commit to chris-forks/flutter that referenced this pull request Sep 1, 2020
Fixes flutter#45075
Fixes flutter#57210

If an asset was included directly from the project root directory, then the same asset when copied to various output or ephemeral directories would also be picked up as an asset variant. This could cause assets to be recursively copied into asset/build/ephemeral directories, as each time it would run it would pick up all of the previous "variants".

The solution is to include project ephemeral directories, in addition to the build directory.
christopherfujino added a commit that referenced this pull request Sep 1, 2020
* Update engine hash to 1.20.3
* Re-enable image_list test with updated certificate(good for 3650 days). (#64519)
* [Material] Relanding fix to ensure time picker input mode lays out correctly in RTL (#64097)
* allow null in compute for weak mode (#63515)
* [flutter_tools] fix recursive asset variant issue (#61129)
* [flutter_tool] Handle Windows line endings in packages_test.dart (#63806)
* [flutter_tool] Fix some create_test.dart tests on Windows (#63796)

Co-authored-by: Alexander Aprelev <[email protected]>
Co-authored-by: Rami <[email protected]>
Co-authored-by: Alexandre Ardhuin <[email protected]>
Co-authored-by: Jonah Williams <[email protected]>
Co-authored-by: Zachary Anderson <[email protected]>
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
Fixes flutter#45075
Fixes flutter#57210

If an asset was included directly from the project root directory, then the same asset when copied to various output or ephemeral directories would also be picked up as an asset variant. This could cause assets to be recursively copied into asset/build/ephemeral directories, as each time it would run it would pick up all of the previous "variants".

The solution is to include project ephemeral directories, in addition to the build directory.
mohammedJ-Sadiq added a commit to mohammedJ-Sadiq/flutter that referenced this pull request Jan 17, 2021
* Update engine hash to 1.20.3
* Re-enable image_list test with updated certificate(good for 3650 days). (flutter#64519)
* [Material] Relanding fix to ensure time picker input mode lays out correctly in RTL (flutter#64097)
* allow null in compute for weak mode (flutter#63515)
* [flutter_tools] fix recursive asset variant issue (flutter#61129)
* [flutter_tool] Handle Windows line endings in packages_test.dart (flutter#63806)
* [flutter_tool] Fix some create_test.dart tests on Windows (flutter#63796)

Co-authored-by: Alexander Aprelev <[email protected]>
Co-authored-by: Rami <[email protected]>
Co-authored-by: Alexandre Ardhuin <[email protected]>
Co-authored-by: Jonah Williams <[email protected]>
Co-authored-by: Zachary Anderson <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

4 participants