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

Author: jcollins-g

lib/src/dartdoc_options.dart

@@ -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;

lib/src/model/library.dart

@@ -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;
   }

lib/src/model/package_builder.dart

@@ -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);

lib/src/model/package_graph.dart

@@ -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);
@@ -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;

lib/src/package_meta.dart

@@ -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);
@@ -173,7 +170,7 @@ abstract class PackageMeta {
 
   bool get requiresFlutter;
 
-  void runPubGet();
+  void runPubGet(String flutterRoot);
 
   String get name;
 
@@ -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) {
@@ -385,7 +390,7 @@ class _SdkMeta extends PackageMeta {
   bool get isSdk => true;
 
   @override
-  void runPubGet() {
+  void runPubGet(String flutterRoot) {
     throw 'unsupported operation';
   }
 

test/dartdoc_integration_test.dart

@@ -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'),

tool/grind.dart

@@ -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));
@@ -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>()
@@ -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();
 }