Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit 9532b91

Browse files
christopherfujinochristopherfujino
authored
[flutter_tools] normalize windows file path cases in flutter validator (#115889)
* normalize windows file path cases in flutter validator * fix * make comparison more accurate by checking .startsWith() rather than .contains() * fix method name * call path.canonicalize * fix
1 parent a9c2f8b commit 9532b91

File tree

2 files changed

+111
-21
lines changed

2 files changed

+111
-21
lines changed

‎packages/flutter_tools/lib/src/doctor.dart

+8-1
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ class FlutterValidator extends DoctorValidator {
619619
);
620620
}
621621
final String resolvedFlutterPath = flutterBin.resolveSymbolicLinksSync();
622-
if (!resolvedFlutterPath.contains(flutterRoot)) {
622+
if (!_filePathContainsDirPath(flutterRoot, resolvedFlutterPath)) {
623623
final String hint = 'Warning: `$binary` on your path resolves to '
624624
'$resolvedFlutterPath, which is not inside your current Flutter '
625625
'SDK checkout at $flutterRoot. Consider adding $flutterBinDir to '
@@ -629,6 +629,13 @@ class FlutterValidator extends DoctorValidator {
629629
return null;
630630
}
631631

632+
bool _filePathContainsDirPath(String directory, String file) {
633+
// calling .canonicalize() will normalize for alphabetic case and path
634+
// separators
635+
return (_fileSystem.path.canonicalize(file))
636+
.startsWith(_fileSystem.path.canonicalize(directory) + _fileSystem.path.separator);
637+
}
638+
632639
ValidationMessage _getFlutterUpstreamMessage(FlutterVersion version) {
633640
final String? repositoryUrl = version.repositoryUrl;
634641
final VersionCheckError? upstreamValidationError = VersionUpstreamValidator(version: version, platform: _platform).run();

‎packages/flutter_tools/test/general.shard/flutter_validator_test.dart

+103-20
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ void main() {
4848
userMessages: UserMessages(),
4949
artifacts: artifacts,
5050
fileSystem: fileSystem,
51-
flutterRoot: () => 'sdk/flutter',
51+
flutterRoot: () => '/sdk/flutter',
5252
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
5353
processManager: FakeProcessManager.list(<FakeCommand>[
5454
const FakeCommand(
@@ -93,7 +93,7 @@ void main() {
9393
fileSystem: MemoryFileSystem.test(),
9494
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Windows'),
9595
processManager: FakeProcessManager.empty(),
96-
flutterRoot: () => 'sdk/flutter',
96+
flutterRoot: () => '/sdk/flutter',
9797
);
9898

9999
// gen_snapshot is downloaded on demand, and the doctor should not
@@ -115,14 +115,14 @@ void main() {
115115
fileSystem: MemoryFileSystem.test(),
116116
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Windows'),
117117
processManager: FakeProcessManager.empty(),
118-
flutterRoot: () => 'sdk/flutter',
118+
flutterRoot: () => '/sdk/flutter',
119119
);
120120

121121
expect(await flutterValidator.validate(), _matchDoctorValidation(
122122
validationType: ValidationType.partial,
123123
statusInfo: 'Channel beta, 0.0.0, on Windows, locale en_US.UTF-8',
124124
messages: containsAll(const <ValidationMessage>[
125-
ValidationMessage('Flutter version 0.0.0 on channel beta at sdk/flutter'),
125+
ValidationMessage('Flutter version 0.0.0 on channel beta at /sdk/flutter'),
126126
ValidationMessage.error('version error'),
127127
]),
128128
));
@@ -152,7 +152,7 @@ void main() {
152152
fileSystem: fileSystem,
153153
processManager: FakeProcessManager.any(),
154154
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Windows'),
155-
flutterRoot: () => 'sdk/flutter'
155+
flutterRoot: () => '/sdk/flutter'
156156
);
157157

158158
expect(await flutterValidator.validate(), _matchDoctorValidation(
@@ -183,7 +183,7 @@ void main() {
183183
fileSystem: MemoryFileSystem.test(),
184184
processManager: FakeProcessManager.any(),
185185
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
186-
flutterRoot: () => 'sdk/flutter',
186+
flutterRoot: () => '/sdk/flutter',
187187
);
188188

189189
expect(await flutterValidator.validate(), _matchDoctorValidation(
@@ -213,15 +213,15 @@ void main() {
213213
fileSystem: MemoryFileSystem.test(),
214214
processManager: FakeProcessManager.any(),
215215
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
216-
flutterRoot: () => 'sdk/flutter',
216+
flutterRoot: () => '/sdk/flutter',
217217
);
218218

219219
expect(await flutterValidator.validate(), _matchDoctorValidation(
220220
validationType: ValidationType.partial,
221221
statusInfo: 'Channel unknown, 1.0.0, on Linux, locale en_US.UTF-8',
222222
messages: containsAll(<ValidationMessage>[
223223
const ValidationMessage.hint(
224-
'Flutter version 1.0.0 on channel unknown at sdk/flutter\n'
224+
'Flutter version 1.0.0 on channel unknown at /sdk/flutter\n'
225225
'Currently on an unknown channel. Run `flutter channel` to switch to an official channel.\n'
226226
"If that doesn't fix the issue, reinstall Flutter by following instructions at https://flutter.dev/docs/get-started/install."
227227
),
@@ -246,15 +246,15 @@ void main() {
246246
fileSystem: MemoryFileSystem.test(),
247247
processManager: FakeProcessManager.any(),
248248
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
249-
flutterRoot: () => 'sdk/flutter',
249+
flutterRoot: () => '/sdk/flutter',
250250
);
251251

252252
expect(await flutterValidator.validate(), _matchDoctorValidation(
253253
validationType: ValidationType.partial,
254254
statusInfo: 'Channel beta, 0.0.0-unknown, on Linux, locale en_US.UTF-8',
255255
messages: containsAll(<ValidationMessage>[
256256
const ValidationMessage.hint(
257-
'Flutter version 0.0.0-unknown on channel beta at sdk/flutter\n'
257+
'Flutter version 0.0.0-unknown on channel beta at /sdk/flutter\n'
258258
'Cannot resolve current version, possibly due to local changes.\n'
259259
'Reinstall Flutter by following instructions at https://flutter.dev/docs/get-started/install.'
260260
),
@@ -280,7 +280,7 @@ void main() {
280280
fileSystem: MemoryFileSystem.test(),
281281
processManager: FakeProcessManager.any(),
282282
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
283-
flutterRoot: () => 'sdk/flutter',
283+
flutterRoot: () => '/sdk/flutter',
284284
);
285285

286286
expect(await flutterValidator.validate(), _matchDoctorValidation(
@@ -371,7 +371,7 @@ void main() {
371371
fileSystem: MemoryFileSystem.test(),
372372
processManager: FakeProcessManager.any(),
373373
operatingSystemUtils: FakeOperatingSystemUtils(name: 'Linux'),
374-
flutterRoot: () => 'sdk/flutter',
374+
flutterRoot: () => '/sdk/flutter',
375375
);
376376

377377
expect(await flutterValidator.validate(), _matchDoctorValidation(
@@ -387,6 +387,7 @@ void main() {
387387
});
388388

389389
testWithoutContext('detects no flutter and dart on path', () async {
390+
const String flutterRoot = 'sdk/flutter';
390391
final FlutterValidator flutterValidator = FlutterValidator(
391392
platform: FakePlatform(localeName: 'en_US.UTF-8'),
392393
flutterVersion: () => FakeFlutterVersion(
@@ -402,14 +403,96 @@ void main() {
402403
name: 'Linux',
403404
whichLookup: const <String, File>{},
404405
),
405-
flutterRoot: () => 'sdk/flutter',
406+
flutterRoot: () => flutterRoot,
406407
);
407408

408409
expect(await flutterValidator.validate(), _matchDoctorValidation(
409410
validationType: ValidationType.partial,
410411
statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
411412
messages: contains(const ValidationMessage.hint(
412-
'The flutter binary is not on your path. Consider adding sdk/flutter/bin to your path.',
413+
'The flutter binary is not on your path. Consider adding $flutterRoot/bin to your path.',
414+
)),
415+
));
416+
});
417+
418+
testWithoutContext('allows case differences in paths on Windows', () async {
419+
const String flutterRoot = r'c:\path\to\flutter-sdk';
420+
const String osName = 'Microsoft Windows';
421+
final MemoryFileSystem fs = MemoryFileSystem.test(
422+
style: FileSystemStyle.windows,
423+
);
424+
// The windows' file system is not case sensitive, so changing the case
425+
// here should not matter.
426+
final File flutterBinary = fs.file('${flutterRoot.toUpperCase()}\\bin\\flutter')
427+
..createSync(recursive: true);
428+
final FlutterValidator flutterValidator = FlutterValidator(
429+
platform: FakePlatform(operatingSystem: 'windows', localeName: 'en_US.UTF-8'),
430+
flutterVersion: () => FakeFlutterVersion(
431+
frameworkVersion: '1.0.0',
432+
channel: 'beta'
433+
),
434+
devToolsVersion: () => '2.8.0',
435+
userMessages: UserMessages(),
436+
artifacts: Artifacts.test(),
437+
fileSystem: fs,
438+
processManager: FakeProcessManager.empty(),
439+
operatingSystemUtils: FakeOperatingSystemUtils(
440+
name: osName,
441+
whichLookup: <String, File>{
442+
'flutter': flutterBinary,
443+
},
444+
),
445+
flutterRoot: () => flutterRoot,
446+
);
447+
448+
expect(await flutterValidator.validate(), _matchDoctorValidation(
449+
validationType: ValidationType.partial,
450+
statusInfo: 'Channel beta, 1.0.0, on $osName, locale en_US.UTF-8',
451+
messages: everyElement(isA<ValidationMessage>().having(
452+
(ValidationMessage message) => message.message,
453+
'message',
454+
isNot(contains('Warning: `flutter` on your path resolves to')),
455+
)),
456+
));
457+
});
458+
459+
testWithoutContext('allows different separator types in paths on Windows', () async {
460+
const String flutterRoot = r'c:\path\to\flutter-sdk';
461+
const String osName = 'Microsoft Windows';
462+
final MemoryFileSystem fs = MemoryFileSystem.test(
463+
style: FileSystemStyle.windows,
464+
);
465+
const String filePath = '$flutterRoot\\bin\\flutter';
466+
// force posix style path separators
467+
final File flutterBinary = fs.file(filePath.replaceAll(r'\', '/'))
468+
..createSync(recursive: true);
469+
final FlutterValidator flutterValidator = FlutterValidator(
470+
platform: FakePlatform(operatingSystem: 'windows', localeName: 'en_US.UTF-8'),
471+
flutterVersion: () => FakeFlutterVersion(
472+
frameworkVersion: '1.0.0',
473+
channel: 'beta'
474+
),
475+
devToolsVersion: () => '2.8.0',
476+
userMessages: UserMessages(),
477+
artifacts: Artifacts.test(),
478+
fileSystem: fs,
479+
processManager: FakeProcessManager.empty(),
480+
operatingSystemUtils: FakeOperatingSystemUtils(
481+
name: osName,
482+
whichLookup: <String, File>{
483+
'flutter': flutterBinary,
484+
},
485+
),
486+
flutterRoot: () => flutterRoot,
487+
);
488+
489+
expect(await flutterValidator.validate(), _matchDoctorValidation(
490+
validationType: ValidationType.partial,
491+
statusInfo: 'Channel beta, 1.0.0, on $osName, locale en_US.UTF-8',
492+
messages: everyElement(isA<ValidationMessage>().having(
493+
(ValidationMessage message) => message.message,
494+
'message',
495+
isNot(contains('Warning: `flutter` on your path resolves to')),
413496
)),
414497
));
415498
});
@@ -430,20 +513,20 @@ void main() {
430513
operatingSystemUtils: FakeOperatingSystemUtils(
431514
name: 'Linux',
432515
whichLookup: <String, File>{
433-
'flutter': fs.file('/usr/bin/flutter')..createSync(recursive: true),
434-
'dart': fs.file('/usr/bin/dart')..createSync(recursive: true),
516+
'flutter': fs.file('/sdk/flutter-beta')..createSync(recursive: true),
517+
'dart': fs.file('/sdk/flutter-beta')..createSync(recursive: true),
435518
},
436519
),
437-
flutterRoot: () => 'sdk/flutter',
520+
flutterRoot: () => '/sdk/flutter',
438521
);
439522

440523
expect(await flutterValidator.validate(), _matchDoctorValidation(
441524
validationType: ValidationType.partial,
442525
statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
443526
messages: contains(const ValidationMessage.hint(
444-
'Warning: `flutter` on your path resolves to /usr/bin/flutter, which '
445-
'is not inside your current Flutter SDK checkout at sdk/flutter. '
446-
'Consider adding sdk/flutter/bin to the front of your path.',
527+
'Warning: `flutter` on your path resolves to /sdk/flutter-beta, which '
528+
'is not inside your current Flutter SDK checkout at /sdk/flutter. '
529+
'Consider adding /sdk/flutter/bin to the front of your path.',
447530
)),
448531
));
449532
});

0 commit comments

Comments
 (0)