Skip to content

Commit

Permalink
Validate bins on path in doctor (flutter#113106)
Browse files Browse the repository at this point in the history
  • Loading branch information
@christopherfujino
christopherfujino authored Oct 20, 2022
1 parent 482466e commit 483e1d9
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 4 deletions.
41 changes: 38 additions & 3 deletions packages/flutter_tools/lib/src/doctor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,10 @@ class FlutterValidator extends DoctorValidator {
versionChannel = version.channel;
frameworkVersion = version.frameworkVersion;

messages.add(_getFlutterVersionMessage(frameworkVersion, versionChannel));
final String flutterRoot = _flutterRoot();
messages.add(_getFlutterVersionMessage(frameworkVersion, versionChannel, flutterRoot));

_validateRequiredBinaries(flutterRoot).forEach(messages.add);
messages.add(_getFlutterUpstreamMessage(version));
if (gitUrl != null) {
messages.add(ValidationMessage(_userMessages.flutterGitUrl(gitUrl)));
Expand Down Expand Up @@ -575,8 +578,8 @@ class FlutterValidator extends DoctorValidator {
);
}

ValidationMessage _getFlutterVersionMessage(String frameworkVersion, String versionChannel) {
String flutterVersionMessage = _userMessages.flutterVersion(frameworkVersion, versionChannel, _flutterRoot());
ValidationMessage _getFlutterVersionMessage(String frameworkVersion, String versionChannel, String flutterRoot) {
String flutterVersionMessage = _userMessages.flutterVersion(frameworkVersion, versionChannel, flutterRoot);

// The tool sets the channel as "unknown", if the current branch is on a
// "detached HEAD" state or doesn't have an upstream, and sets the
Expand All @@ -594,6 +597,38 @@ class FlutterValidator extends DoctorValidator {
return ValidationMessage.hint(flutterVersionMessage);
}

List<ValidationMessage> _validateRequiredBinaries(String flutterRoot) {
final ValidationMessage? flutterWarning = _validateSdkBinary('flutter', flutterRoot);
final ValidationMessage? dartWarning = _validateSdkBinary('dart', flutterRoot);
return <ValidationMessage>[
if (flutterWarning != null) flutterWarning,
if (dartWarning != null) dartWarning,
];
}

/// Return a warning if the provided [binary] on the user's path does not
/// resolve within the Flutter SDK.
ValidationMessage? _validateSdkBinary(String binary, String flutterRoot) {
final String flutterBinDir = _fileSystem.path.join(flutterRoot, 'bin');

final File? flutterBin = _operatingSystemUtils.which(binary);
if (flutterBin == null) {
return ValidationMessage.hint(
'The $binary binary is not on your path. Consider adding '
'$flutterBinDir to your path.',
);
}
final String resolvedFlutterPath = flutterBin.resolveSymbolicLinksSync();
if (!resolvedFlutterPath.contains(flutterRoot)) {
final String hint = 'Warning: `$binary` on your path resolves to '
'$resolvedFlutterPath, which is not inside your current Flutter '
'SDK checkout at $flutterRoot. Consider adding $flutterBinDir to '
'the front of your path.';
return ValidationMessage.hint(hint);
}
return null;
}

ValidationMessage _getFlutterUpstreamMessage(FlutterVersion version) {
final String? repositoryUrl = version.repositoryUrl;
final VersionCheckError? upstreamValidationError = VersionUpstreamValidator(version: version, platform: _platform).run();
Expand Down
115 changes: 114 additions & 1 deletion packages/flutter_tools/test/general.shard/flutter_validator_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/os.dart';
Expand Down Expand Up @@ -384,10 +385,122 @@ void main() {
),
));
});

testWithoutContext('detects no flutter and dart on path', () async {
final FlutterValidator flutterValidator = FlutterValidator(
platform: FakePlatform(localeName: 'en_US.UTF-8'),
flutterVersion: () => FakeFlutterVersion(
frameworkVersion: '1.0.0',
channel: 'beta'
),
devToolsVersion: () => '2.8.0',
userMessages: UserMessages(),
artifacts: Artifacts.test(),
fileSystem: MemoryFileSystem.test(),
processManager: FakeProcessManager.any(),
operatingSystemUtils: FakeOperatingSystemUtils(
name: 'Linux',
whichLookup: const <String, File>{},
),
flutterRoot: () => 'sdk/flutter',
);

expect(await flutterValidator.validate(), _matchDoctorValidation(
validationType: ValidationType.partial,
statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
messages: contains(const ValidationMessage.hint(
'The flutter binary is not on your path. Consider adding sdk/flutter/bin to your path.',
)),
));
});

testWithoutContext('detects flutter and dart from outside flutter sdk', () async {
final FileSystem fs = MemoryFileSystem.test();
final FlutterValidator flutterValidator = FlutterValidator(
platform: FakePlatform(localeName: 'en_US.UTF-8'),
flutterVersion: () => FakeFlutterVersion(
frameworkVersion: '1.0.0',
channel: 'beta'
),
devToolsVersion: () => '2.8.0',
userMessages: UserMessages(),
artifacts: Artifacts.test(),
fileSystem: fs,
processManager: FakeProcessManager.any(),
operatingSystemUtils: FakeOperatingSystemUtils(
name: 'Linux',
whichLookup: <String, File>{
'flutter': fs.file('/usr/bin/flutter')..createSync(recursive: true),
'dart': fs.file('/usr/bin/dart')..createSync(recursive: true),
},
),
flutterRoot: () => 'sdk/flutter',
);

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

testWithoutContext('no warnings if flutter & dart binaries are inside the Flutter SDK', () async {
final FileSystem fs = MemoryFileSystem.test();
final FlutterValidator flutterValidator = FlutterValidator(
platform: FakePlatform(localeName: 'en_US.UTF-8'),
flutterVersion: () => FakeFlutterVersion(
frameworkVersion: '1.0.0',
channel: 'beta'
),
devToolsVersion: () => '2.8.0',
userMessages: UserMessages(),
artifacts: Artifacts.test(),
fileSystem: fs,
processManager: FakeProcessManager.any(),
operatingSystemUtils: FakeOperatingSystemUtils(
name: 'Linux',
whichLookup: <String, File>{
'flutter': fs.file('/sdk/flutter/bin/flutter')..createSync(recursive: true),
'dart': fs.file('/sdk/flutter/bin/dart')..createSync(recursive: true),
},
),
flutterRoot: () => '/sdk/flutter',
);

expect(await flutterValidator.validate(), _matchDoctorValidation(
validationType: ValidationType.installed,
statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
messages: isNot(contains(isA<ValidationMessage>().having(
(ValidationMessage message) => message.message,
'message',
contains('Consider adding /sdk/flutter/bin to the front of your path'),
))),
));
});
}

class FakeOperatingSystemUtils extends Fake implements OperatingSystemUtils {
FakeOperatingSystemUtils({required this.name});
FakeOperatingSystemUtils({
required this.name,
this.whichLookup,
FileSystem? fs,
}) {
fs ??= MemoryFileSystem.test();
whichLookup ??= <String, File>{
'flutter': fs.file('/sdk/flutter/bin/flutter')..createSync(recursive: true),
'dart': fs.file('/sdk/flutter/bin/dart')..createSync(recursive: true),
};
}

/// A map of [File]s that calls to [which] will return.
Map<String, File>? whichLookup;

@override
File? which(String execName) => whichLookup![execName];

@override
final String name;
Expand Down

0 comments on commit 483e1d9

Please sign in to comment.