GN: change the way we specify dependencies for the patched_sdk target.

tools/patch_sdk.dart

Index: tools/patch_sdk.dart
diff --git a/tools/patch_sdk.dart b/tools/patch_sdk.dart
index 33a06fb7c7e61d566e7825388e9759472ed8a863..722effb1be13f2516bb86b037f64375347d0af2b 100644
--- a/tools/patch_sdk.dart
+++ b/tools/patch_sdk.dart
@@ -16,6 +16,41 @@ import 'package:path/path.dart' as path;
 import 'package:front_end/src/fasta/compile_platform.dart' as
     compile_platform;
 
+import 'package:front_end/src/fasta/outline.dart' show CompileTask;
+
+import 'package:front_end/src/fasta/compiler_command_line.dart' show CompilerCommandLine;

[kustermann, 2017/03/03 13:23:08]

nit: long line

[Vyacheslav Egorov (Google), 2017/03/03 13:49:15]

Done.

+
+import 'package:front_end/src/fasta/compiler_context.dart' show CompilerContext;
+
+import 'package:front_end/src/fasta/ticker.dart' show Ticker;
+
+/// Set of input files that were read by this script to generate patched SDK.
+/// We will dump it out into the depfile for ninja to use.
+///
+/// For more information see GN and Ninja references:
+///    https://chromium.googlesource.com/chromium/src/+/56807c6cb383140af0c03da8f6731d77785d7160/tools/gn/docs/reference.md#depfile_string_File-name-for-input-dependencies-for-actions
+///    https://ninja-build.org/manual.html#_depfile
+///
+final deps = new Set<String>();
+
+/// Create [File] object from the given path and register it as a dependency.
+File getInputFile(String path, {canBeMissing: true}) {
+  final file = new File(path);
+  if (!file.existsSync()) {
+    if (!canBeMissing)
+      throw "patch_sdk.dart expects all inputs to exist";
+    return null;
+  }
+  deps.add(new File(path).absolute.path);

[kustermann, 2017/03/03 13:23:08]

deps.add(file.absolute.path);

[Vyacheslav Egorov (Google), 2017/03/03 13:49:15]

Done.

+  return file;
+}
+
+
+/// Read the given file synchronously as a string and register this path as
+/// a dependency.
+String readInputFile(String path, {canBeMissing: true}) =>
+  getInputFile(path, canBeMissing: canBeMissing)?.readAsStringSync();
+
 Future main(List<String> argv) async {
   var base = path.fromUri(Platform.script);
   var dartDir = path.dirname(path.dirname(path.absolute(base)));
@@ -37,6 +72,7 @@ Future main(List<String> argv) async {
   }
 
   var mode = argv[0];
+  assert(mode == "vm");
   var input = argv[1];
   var sdkLibIn = path.join(input, 'lib');
   var patchIn = argv[2];
@@ -48,10 +84,9 @@ Future main(List<String> argv) async {
   var INTERNAL_PATH = '_internal/compiler/js_lib/';
 
   // Copy and patch libraries.dart and version
-  var libContents = new File(path.join(sdkLibIn, '_internal',
-      'sdk_library_metadata', 'lib', 'libraries.dart')).readAsStringSync();
-  if (mode == 'vm') {
-    libContents = libContents.replaceAll(
+  var libContents = readInputFile(path.join(sdkLibIn, '_internal',
+      'sdk_library_metadata', 'lib', 'libraries.dart'));
+  libContents = libContents.replaceAll(

[kustermann, 2017/03/03 13:23:08]

readInputFile has canBeMissing: true so we could do a null dereference. Maybe
set it to `true`.

[Vyacheslav Egorov (Google), 2017/03/03 13:49:15]

canBeMissing should be false by default

         ' libraries = const {',
         ''' libraries = const {
 
@@ -80,39 +115,25 @@ Future main(List<String> argv) async {
       platforms: VM_PLATFORM),
 
 ''');
-  }
   _writeSync(
       path.join(
           sdkOut, '_internal', 'sdk_library_metadata', 'lib', 'libraries.dart'),
       libContents);
-  if (mode == 'ddc') {
-    _writeSync(path.join(sdkOut, '..', 'version'),
-        new File(path.join(sdkLibIn, '..', 'version')).readAsStringSync());
-  }
 
   // Parse libraries.dart
   var sdkLibraries = _getSdkLibraries(libContents);
 
   // Enumerate core libraries and apply patches
   for (SdkLibrary library in sdkLibraries) {
-    // TODO(jmesserly): analyzer does not handle the default case of
-    // "both platforms" correctly, and treats it as being supported on neither.
-    // So instead we skip explicitly marked as either VM or dart2js libs.
-    if (mode == 'ddc' ? library.isVmLibrary : library.isDart2JsLibrary) {
+    if (library.isDart2JsLibrary) {
       continue;
     }
 
     var libraryOut = path.join(sdkLibIn, library.path);
-    var libraryIn;
-    if (mode == 'ddc' && library.path.contains(INTERNAL_PATH)) {
-      libraryIn =
-          path.join(privateIn, library.path.replaceAll(INTERNAL_PATH, ''));
-    } else {
-      libraryIn = libraryOut;
-    }
+    var libraryIn = libraryOut;
 
-    var libraryFile = new File(libraryIn);
-    if (libraryFile.existsSync()) {
+    var libraryFile = getInputFile(libraryIn, canBeMissing: true);
+    if (libraryFile != null) {
       var outPaths = <String>[libraryOut];
       var libraryContents = libraryFile.readAsStringSync();
 
@@ -124,7 +145,7 @@ Future main(List<String> argv) async {
           var partPath = part.uri.stringValue;
           outPaths.add(path.join(path.dirname(libraryOut), partPath));
 
-          var partFile = new File(path.join(path.dirname(libraryIn), partPath));
+          var partFile = getInputFile(path.join(path.dirname(libraryIn), partPath));

[kustermann, 2017/03/03 13:23:08]

run dartfmt?

[Vyacheslav Egorov (Google), 2017/03/03 13:49:15]

Done.

           partFiles.add(partFile);
           inputModifyTime = math.max(inputModifyTime,
               partFile.lastModifiedSync().millisecondsSinceEpoch);
@@ -135,9 +156,8 @@ Future main(List<String> argv) async {
       var patchPath = path.join(
           patchIn, path.basenameWithoutExtension(libraryIn) + '_patch.dart');
 
-      var patchFile = new File(patchPath);
-      bool patchExists = patchFile.existsSync();
-      if (patchExists) {
+      var patchFile = getInputFile(patchPath, canBeMissing: true);
+      if (patchFile != null) {
         inputModifyTime = math.max(inputModifyTime,
             patchFile.lastModifiedSync().millisecondsSinceEpoch);
       }
@@ -162,7 +182,7 @@ Future main(List<String> argv) async {
       if (needsUpdate) {
         var contents = <String>[libraryContents];
         contents.addAll(partFiles.map((f) => f.readAsStringSync()));
-        if (patchExists) {
+        if (patchFile != null) {
           var patchContents = patchFile.readAsStringSync();
           contents = _patchLibrary(
               patchFile.path, contents, patchContents);
@@ -175,22 +195,20 @@ Future main(List<String> argv) async {
     }
   }
 
-  if (mode == 'vm') {
-    for (var tuple in [['_builtin', 'builtin.dart']]) {
-      var vmLibrary = tuple[0];
-      var dartFile = tuple[1];
+  for (var tuple in [['_builtin', 'builtin.dart']]) {
+    var vmLibrary = tuple[0];
+    var dartFile = tuple[1];
 
-      // The "dart:_builtin" library is only available for the DartVM.
-      var builtinLibraryIn  = path.join(dartDir, 'runtime', 'bin', dartFile);
-      var builtinLibraryOut = path.join(sdkOut, vmLibrary, '${vmLibrary}.dart');
-      _writeSync(builtinLibraryOut, new File(builtinLibraryIn).readAsStringSync());
-    }
+    // The "dart:_builtin" library is only available for the DartVM.
+    var builtinLibraryIn  = path.join(dartDir, 'runtime', 'bin', dartFile);
+    var builtinLibraryOut = path.join(sdkOut, vmLibrary, '${vmLibrary}.dart');
+    _writeSync(builtinLibraryOut, readInputFile(builtinLibraryIn));
+  }
 
-    for (var file in ['loader.dart', 'server.dart', 'vmservice_io.dart']) {
-      var libraryIn  = path.join(dartDir, 'runtime', 'bin', 'vmservice', file);
-      var libraryOut = path.join(sdkOut, 'vmservice_io', file);
-      _writeSync(libraryOut, new File(libraryIn).readAsStringSync());
-    }
+  for (var file in ['loader.dart', 'server.dart', 'vmservice_io.dart']) {
+    var libraryIn  = path.join(dartDir, 'runtime', 'bin', 'vmservice', file);
+    var libraryOut = path.join(sdkOut, 'vmservice_io', file);
+    _writeSync(libraryOut, readInputFile(libraryIn));
   }
 
   // TODO(kustermann): We suppress compiler hints/warnings/errors temporarily
@@ -199,13 +217,45 @@ Future main(List<String> argv) async {
   // been fixed (either in fasta or the dart files in the patched_sdk).
   final capturedLines = <String>[];
   try {
+    final platform = path.join(outDir, 'platform.dill');
+
     await runZoned(() async {
       await compile_platform.mainEntryPoint(<String>[
         '--packages',
         new Uri.file(packagesFile).toString(),
         new Uri.directory(outDir).toString(),
-        path.join(outDir, 'platform.dill')
+        platform,
       ]);
+
+      // platform.dill was generated, now generate platform.dill.d depfile
+      // that captures all dependencies that participated in the generation.
+      // There are two types of dependencies:
+      //   (1) all Dart sources that constitute this tool itself
+      //   (2) Dart SDK and patch sources.
+      // We already collected all inputs from the second category in the deps
+      // set. To collect inputs from the first category we actually use Fasta:
+      // we ask Fasta to outline patch_sdk.dart and generate a depfile which
+      // would list all the sources.
+      final depfile = "${outDir}.d";
+      await CompilerCommandLine.withGlobalOptions("outline", [
+        '--packages',
+        new Uri.file(packagesFile).toString(),
+        '--platform',
+        platform,  // platform.dill that was just generated
+        Platform.script.toString()  // patch_sdk.dart
+      ], (CompilerContext c) async {
+        CompileTask task =
+            new CompileTask(c, new Ticker(isVerbose: c.options.verbose));
+        KernelTarget kernelTarget = await task.buildOutline(null);
+        await kernelTarget.writeDepsFile(new Uri.file(platform), new Uri.file(depfile));
+      });
+
+      // Read depfile generated by Fasta and append deps that we have collected
+      // during generation of patched_sdk to it.
+      final list = new File(depfile).readAsStringSync().split(':');
+      assert(list.length == 2);
+      deps.addAll(list[1].split(' ').where((str) => str.isNotEmpty));

[kustermann, 2017/03/03 13:23:08]

maybe assert list[0] == 'platform.dill'

Could you ensure we run this file in checked mode?

+      new File(depfile).writeAsStringSync("${list[0]}: ${deps.join(' ')}\n");
     }, zoneSpecification: new ZoneSpecification(print: (_, _2, _3, line) {
       capturedLines.add(line);
     }));