Skip to content

Commit 04f9ead

Browse files
[CP-stable]🐛 Use consist slashes when generating dep files (#169630)
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? flutter/flutter#163591 ### Changelog Description: Normalizes file paths in every depfile, especially on Windows. It eliminates the inconsistency that can occur when other codes find the file paths are different and produce unexpected results. ### Impact Description: The most noticeable impact so far is that people are unable to build flavored Android packages on Windows repeatedly until the next clean. ### Workaround: Is there a workaround for this issue? The workaround is to manually patch the project's gradle script: flutter/flutter#163591 (comment) From my experience, the patch is not always working and is hard to maintain. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? Follow the *steps to reproduce* section in flutter/flutter#163591 (comment).
1 parent 1091508 commit 04f9ead

File tree

2 files changed

+75
-19
lines changed

2 files changed

+75
-19
lines changed

packages/flutter_tools/lib/src/build_system/depfile.dart

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,19 @@ class DepfileService {
6565
}
6666

6767
void _writeFilesToBuffer(List<File> files, StringBuffer buffer) {
68+
final bool backslash = _fileSystem.path.style.separator == r'\';
6869
for (final File outputFile in files) {
69-
if (_fileSystem.path.style.separator == r'\') {
70-
// backslashes and spaces in a depfile have to be escaped if the
71-
// platform separator is a backslash.
72-
final String path = outputFile.path.replaceAll(r'\', r'\\').replaceAll(r' ', r'\ ');
73-
buffer.write(' $path');
70+
String path = _fileSystem.path.normalize(outputFile.path);
71+
if (backslash) {
72+
// Backslashes in a depfile have to be escaped if the platform separator is a backslash.
73+
path = path.replaceAll(r'\', r'\\');
7474
} else {
75-
final String path = outputFile.path.replaceAll(r' ', r'\ ');
76-
buffer.write(' $path');
75+
// Convert all path separators to forward slashes.
76+
path = path.replaceAll(r'\', r'/');
7777
}
78+
// Escape spaces.
79+
path = path.replaceAll(r' ', r'\ ');
80+
buffer.write(' $path');
7881
}
7982
}
8083

@@ -92,7 +95,8 @@ class DepfileService {
9295
// The tool doesn't write duplicates to these lists. This call is an attempt to
9396
// be resilient to the outputs of other tools which write or user edits to depfiles.
9497
.toSet()
95-
.map(_fileSystem.file)
98+
// Normalize the path before creating a file object.
99+
.map((String path) => _fileSystem.file(_fileSystem.path.normalize(path)))
96100
.toList();
97101
}
98102
}

packages/flutter_tools/test/general.shard/build_system/depfile_test.dart

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ void main() {
1717
fileSystem = MemoryFileSystem.test();
1818
depfileService = DepfileService(logger: BufferLogger.test(), fileSystem: fileSystem);
1919
});
20+
2021
testWithoutContext('Can parse depfile from file', () {
2122
final File depfileSource = fileSystem.file('example.d')..writeAsStringSync('''
2223
a.txt: b.txt
@@ -48,22 +49,34 @@ a.txt c.txt d.txt: b.txt
4849
});
4950

5051
testWithoutContext('Can parse depfile with windows file paths', () {
51-
fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows);
52-
depfileService = DepfileService(logger: BufferLogger.test(), fileSystem: fileSystem);
52+
final FileSystem fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows);
53+
final DepfileService depfileService = DepfileService(
54+
logger: BufferLogger.test(),
55+
fileSystem: fileSystem,
56+
);
5357
final File depfileSource = fileSystem.file('example.d')..writeAsStringSync(r'''
54-
C:\\a.txt: C:\\b.txt
58+
C:\\a1.txt C:\\a2/a3.txt: C:\\b1.txt C:\\b2/b3.txt
5559
''');
5660
final Depfile depfile = depfileService.parse(depfileSource);
5761

58-
expect(depfile.inputs.single.path, r'C:\b.txt');
59-
expect(depfile.outputs.single.path, r'C:\a.txt');
62+
expect(depfile.inputs.map((File e) => e.path).toList(), <String>[
63+
r'C:\b1.txt',
64+
r'C:\b2\b3.txt',
65+
]);
66+
expect(depfile.outputs.map((File e) => e.path).toList(), <String>[
67+
r'C:\a1.txt',
68+
r'C:\a2\a3.txt',
69+
]);
6070
});
6171

6272
testWithoutContext(
6373
'Can escape depfile with windows file paths and spaces in directory names',
6474
() {
65-
fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows);
66-
depfileService = DepfileService(logger: BufferLogger.test(), fileSystem: fileSystem);
75+
final FileSystem fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows);
76+
final DepfileService depfileService = DepfileService(
77+
logger: BufferLogger.test(),
78+
fileSystem: fileSystem,
79+
);
6780
final File inputFile =
6881
fileSystem.directory(r'Hello Flutter').childFile('a.txt').absolute
6982
..createSync(recursive: true);
@@ -73,8 +86,9 @@ C:\\a.txt: C:\\b.txt
7386
final File outputDepfile = fileSystem.file('depfile');
7487
depfileService.writeToFile(depfile, outputDepfile);
7588

76-
expect(outputDepfile.readAsStringSync(), contains(r'C:\\Hello\ Flutter\\a.txt'));
77-
expect(outputDepfile.readAsStringSync(), contains(r'C:\\Hello\ Flutter\\b.txt'));
89+
final String output = outputDepfile.readAsStringSync();
90+
expect(output, contains(r'C:\\Hello\ Flutter\\a.txt'));
91+
expect(output, contains(r'C:\\Hello\ Flutter\\b.txt'));
7892
},
7993
);
8094

@@ -88,8 +102,46 @@ C:\\a.txt: C:\\b.txt
88102
final File outputDepfile = fileSystem.file('depfile');
89103
depfileService.writeToFile(depfile, outputDepfile);
90104

91-
expect(outputDepfile.readAsStringSync(), contains(r'/Hello\ Flutter/a.txt'));
92-
expect(outputDepfile.readAsStringSync(), contains(r'/Hello\ Flutter/b.txt'));
105+
final String output = outputDepfile.readAsStringSync();
106+
expect(output, contains(r'/Hello\ Flutter/a.txt'));
107+
expect(output, contains(r'/Hello\ Flutter/b.txt'));
108+
});
109+
110+
testWithoutContext('Can produce normalized paths', () {
111+
final List<(FileSystemStyle style, String input, String output, List<String> expects)> pairs =
112+
<(FileSystemStyle style, String input, String output, List<String> expects)>[
113+
(
114+
FileSystemStyle.posix,
115+
r'Hello Flutter\a.txt',
116+
r'Hello Flutter\b.txt',
117+
<String>[r'/Hello\ Flutter/a.txt', r'/Hello\ Flutter/b.txt'],
118+
),
119+
(
120+
FileSystemStyle.windows,
121+
r'Hello Flutter/a.txt',
122+
r'Hello Flutter/b.txt',
123+
<String>[r'\\Hello\ Flutter\\a.txt', r'\\Hello\ Flutter\\b.txt'],
124+
),
125+
];
126+
127+
for (final (FileSystemStyle style, String input, String output, List<String> expects)
128+
in pairs) {
129+
final FileSystem fileSystem = MemoryFileSystem.test(style: style);
130+
final DepfileService depfileService = DepfileService(
131+
logger: BufferLogger.test(),
132+
fileSystem: fileSystem,
133+
);
134+
final File inputFile = fileSystem.file(input).absolute..createSync(recursive: true);
135+
final File outputFile = fileSystem.file(output).absolute..createSync();
136+
final Depfile depfile = Depfile(<File>[inputFile], <File>[outputFile]);
137+
final File outputDepfile = fileSystem.file('depfile');
138+
depfileService.writeToFile(depfile, outputDepfile);
139+
140+
final String outputString = outputDepfile.readAsStringSync();
141+
for (final String path in expects) {
142+
expect(outputString, contains(path));
143+
}
144+
}
93145
});
94146

95147
testWithoutContext('Resilient to weird whitespace', () {

0 commit comments

Comments
 (0)