Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Nov 5, 2019

Description

Works on my computer?

@fluttergithubbot fluttergithubbot added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Nov 5, 2019
@jonahwilliams jonahwilliams marked this pull request as ready for review November 6, 2019 00:53
final PoolResource resource = await pool.request();
try {
final File file = fs.file(fs.path.join(outputDirectory.path, entry.key));
final File file = fs.file(outputDirectory.uri.resolve(entry.key));
Copy link
Member

Choose a reason for hiding this comment

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

Is this fix needed anywhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated bundle.dart

await buildSystem.build(const CopyAssets(), environment);

expect(fs.file(fs.path.join(environment.buildDir.path, 'flutter_assets', 'assets/foo/bar.png')).existsSync(), true);
expect(fs.file(fs.path.join(environment.buildDir.path, 'flutter_assets', 'assets', 'foo', 'bar.png')).existsSync(), true);
Copy link
Member

Choose a reason for hiding this comment

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

Is it important for the test that you get different instances for the two repetitions of:

fs.file(fs.path.join(environment.buildDir.path, 'flutter_assets', 'assets', 'foo', 'bar.png') ?

If so, please add a comment. If not, then stash either the File or the String for the path in a local.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@jonahwilliams jonahwilliams merged commit 383e90e into flutter:master Nov 6, 2019
@jonahwilliams jonahwilliams deleted the remove_weird_key branch November 6, 2019 20:50
@jonahwilliams
Copy link
Contributor Author

Fixes #35293

@dnfield
Copy link
Contributor

dnfield commented Dec 19, 2019

This also causes #46163.

I think we want ot keep all these as URI encoded because the framework sends over the file names as URI encoded. If anything we should make sure we copy only the URI encoded file, and not the unencoded file.

@jonahwilliams
Copy link
Contributor Author

It should be safe to revert this PR. That revert should include tests of the currently failing to load assets, lest we repeat this in 10-12 months

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 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

Development

Successfully merging this pull request may close these issues.

5 participants