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
[flutter_tools] fix path escaping on in depfile generation #50538
[flutter_tools] fix path escaping on in depfile generation #50538
Conversation
final String escapedPath = outputFile.path.replaceAll(r'\', r'\\'); | ||
final String escapedPath = outputFile.path | ||
.replaceAll(r'\', r'\\') | ||
.replaceAll(r' ', r'\ '); | ||
buffer.write(' $escapedPath'); | ||
} else { | ||
buffer.write(' ${outputFile.path}'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does _separatorExpr
imply that spaces have to be escaped on all platforms? That is, a space preceded by anything but a '' is a separator not just on Windows, but on other platforms as well.
expect(outputDepfile.readAsStringSync(), contains(r'C:\\Hello\ Flutter\\b.txt')); | ||
}, overrides: <Type, Generator>{ | ||
FileSystem: () => MemoryFileSystem(style: FileSystemStyle.windows), | ||
Platform: () => FakePlatform(operatingSystem: 'windows'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test spaces in paths on other platforms, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I double checked, and you're right - they have to be escaped on macOS/Linux too. Updating PR. 🤦♂ |
Description
When a depfile is generated for gradle build, empty spaces
need to be escaped when creating the depfile. I verified this works locally on my computer, and created a unit test for the right escaping.
#16604