Skip to content
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

Run flutter pub get when appropriate instead of normal pub get #2190

Merged
merged 4 commits into from
Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions lib/src/dartdoc_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1583,10 +1583,6 @@ Future<List<DartdocOption>> createDartdocOptions() async {
(option.root['topLevelPackageMeta'].valueAt(dir) as PackageMeta)
.requiresFlutter) {
String flutterRoot = option.root['flutterRoot'].valueAt(dir);
if (flutterRoot == null) {
throw DartdocOptionError(
'Top level package requires Flutter but FLUTTER_ROOT environment variable not set');
}
return p.join(flutterRoot, 'bin', 'cache', 'dart-sdk');
}
return defaultSdkDir.absolute.path;
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/library.dart
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ class Library extends ModelElement with Categorization, TopLevelContainer {

PackageMeta get packageMeta {
if (_packageMeta == null) {
_packageMeta = PackageMeta.fromElement(element, config);
_packageMeta = PackageMeta.fromElement(element, config.sdkDir);
}
return _packageMeta;
}
Expand Down
12 changes: 10 additions & 2 deletions lib/src/model/package_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,16 @@ class PackageBuilder {
PackageBuilder(this.config);

Future<PackageGraph> buildPackageGraph() async {
if (config.topLevelPackageMeta.needsPubGet) {
config.topLevelPackageMeta.runPubGet();
if (!config.sdkDocs) {
if (config.topLevelPackageMeta.needsPubGet &&
config.topLevelPackageMeta.requiresFlutter &&
config.flutterRoot == null) {
throw DartdocOptionError(
'Top level package requires Flutter but FLUTTER_ROOT environment variable not set');
}
if (config.topLevelPackageMeta.needsPubGet) {
config.topLevelPackageMeta.runPubGet(config.flutterRoot);
}
}

RendererFactory rendererFactory = RendererFactory.forFormat(config.format);
Expand Down
4 changes: 2 additions & 2 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class PackageGraph {
void addLibraryToGraph(ResolvedLibraryResult result) {
assert(!allLibrariesAdded);
LibraryElement element = result.element;
var packageMeta = PackageMeta.fromElement(element, config);
var packageMeta = PackageMeta.fromElement(element, config.sdkDir);
var lib = Library.fromLibraryResult(
result, this, Package.fromPackageMeta(packageMeta, this));
packageMap[packageMeta.name].libraries.add(lib);
Expand Down Expand Up @@ -850,7 +850,7 @@ class PackageGraph {
result,
this,
Package.fromPackageMeta(
PackageMeta.fromElement(result.element.library, config),
PackageMeta.fromElement(result.element.library, config.sdkDir),
packageGraph));
allLibraries[result.element.library] = foundLibrary;
return foundLibrary;
Expand Down
31 changes: 18 additions & 13 deletions lib/src/package_meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,9 @@ abstract class PackageMeta {

/// Use this instead of fromDir where possible.
factory PackageMeta.fromElement(
LibraryElement libraryElement, DartdocOptionContext config) {
// [config] is only here for sdkDir, and it's OK that it is the wrong
// context since sdkDir is argOnly and this is supposed to be a temporary
// workaround.
LibraryElement libraryElement, String sdkDir) {
if (libraryElement.isInSdk) {
return PackageMeta.fromDir(Directory(config.sdkDir));
return PackageMeta.fromDir(Directory(sdkDir));
}
return PackageMeta.fromDir(
File(path.canonicalize(libraryElement.source.fullName)).parent);
Expand Down Expand Up @@ -173,7 +170,7 @@ abstract class PackageMeta {

bool get requiresFlutter;

void runPubGet();
void runPubGet(String flutterRoot);

String get name;

Expand Down Expand Up @@ -279,16 +276,24 @@ class _FilePackageMeta extends PackageMeta {

@override
bool get needsPubGet =>
!(File(path.join(dir.path, '.packages')).existsSync());
!(File(path.join(dir.path, '.dart_tool', 'package_config.json'))
.existsSync());

@override
void runPubGet() {
String pubPath =
path.join(path.dirname(Platform.resolvedExecutable), 'pub');
if (Platform.isWindows) pubPath += '.bat';
void runPubGet(String flutterRoot) {
String binPath;
List<String> parameters;
if (requiresFlutter) {
binPath = path.join(flutterRoot, 'bin', 'flutter');
parameters = ['pub', 'get'];
} else {
binPath = path.join(path.dirname(Platform.resolvedExecutable), 'pub');
parameters = ['get'];
}
if (Platform.isWindows) binPath += '.bat';

ProcessResult result =
Process.runSync(pubPath, ['get'], workingDirectory: dir.path);
Process.runSync(binPath, parameters, workingDirectory: dir.path);

var trimmedStdout = (result.stdout as String).trim();
if (trimmedStdout.isNotEmpty) {
Expand Down Expand Up @@ -385,7 +390,7 @@ class _SdkMeta extends PackageMeta {
bool get isSdk => true;

@override
void runPubGet() {
void runPubGet(String flutterRoot) {
throw 'unsupported operation';
}

Expand Down
3 changes: 3 additions & 0 deletions test/dartdoc_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ void main() {
test('Validate missing FLUTTER_ROOT exception is clean', () async {
StringBuffer output = StringBuffer();
var args = <String>[dartdocPath];
var dart_tool =
Directory(path.join(_testPackageFlutterPluginPath, '.dart_tool'));
if (dart_tool.existsSync()) dart_tool.deleteSync(recursive: true);
Future run = subprocessLauncher.runStreamed(
Platform.resolvedExecutable, args,
environment: Map.from(Platform.environment)..remove('FLUTTER_ROOT'),
Expand Down
23 changes: 23 additions & 0 deletions tool/grind.dart
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ Future<List<Map>> _buildTestPackageDocs(
}

@Task('Build generated test package docs (with inherited docs and source code)')
@Depends(clean)
Future<void> buildTestPackageDocs() async {
await _buildTestPackageDocs(
testPackageDocsDir.absolute.path, Future.value(Directory.current.path));
Expand Down Expand Up @@ -903,23 +904,44 @@ Future<void> tryPublish() async {
}

@Task('Run a smoke test, only')
@Depends(clean)
Future<void> smokeTest() async {
await testDart2(smokeTestFiles);
await testFutures.wait();
}

@Task('Run non-smoke tests, only')
@Depends(clean)
Future<void> longTest() async {
await testDart2(testFiles);
await testFutures.wait();
}

@Task('Run all the tests.')
@Depends(clean)
Future<void> test() async {
await testDart2(smokeTestFiles.followedBy(testFiles));
await testFutures.wait();
}

@Task('Clean up pub data from test directories')
Future<void> clean() async {
var toDelete = nonRootPubData;
toDelete.forEach((e) => e.deleteSync(recursive: true));
}

Iterable<FileSystemEntity> get nonRootPubData {
// This involves deleting things, so be careful.
if (!File(path.join('tool', 'grind.dart')).existsSync()) {
throw FileSystemException('wrong CWD, run from root of dartdoc package');
}
return Directory('.')
.listSync(recursive: true)
.where((e) => path.dirname(e.path) != '.')
.where((e) => <String>['.dart_tool', '.packages', 'pubspec.lock']
.contains(path.basename(e.path)));
}

List<File> get smokeTestFiles => Directory('test')
.listSync(recursive: true)
.whereType<File>()
Expand Down Expand Up @@ -1017,6 +1039,7 @@ Future<WarningsCollection> _buildDartdocFlutterPluginDocs() async {
}

@Task('Build docs for a package that requires flutter with remote linking')
@Depends(clean)
Future<void> buildDartdocFlutterPluginDocs() async {
await _buildDartdocFlutterPluginDocs();
}
Expand Down