support file uris outside the root package (#3530)

Fixes #3528

This will allow us to look up asset ids for file uris in any package - which is needed for custom build scripts shipped as `bin` scripts in packages.

Author: jakemac53

build_runner_core/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 7.2.10
+
+- Fix build script invalidation check for custom build scripts from other
+  packages.
+
 ## 7.2.9
 
 - Fix compatibility with `package:logging` version `1.2.0`.

build_runner_core/lib/src/changes/build_script_updates.dart

@@ -8,6 +8,7 @@ import 'dart:mirrors';
 import 'package:build/build.dart';
 import 'package:collection/collection.dart';
 import 'package:logging/logging.dart';
+import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 
 import '../asset/reader.dart';
@@ -45,12 +46,11 @@ class _MirrorBuildScriptUpdates implements BuildScriptUpdates {
   static Future<BuildScriptUpdates> create(RunnerAssetReader reader,
       PackageGraph packageGraph, AssetGraph graph) async {
     var supportsIncrementalRebuilds = true;
-    var rootPackage = packageGraph.root.name;
     Set<AssetId> allSources;
     var logger = Logger('BuildScriptUpdates');
     try {
       allSources = _urisForThisScript
-          .map((id) => _idForUri(id, rootPackage))
+          .map((id) => idForUri(id, packageGraph))
           .whereNotNull()
           .toSet();
       var missing = allSources.firstWhereOrNull((id) => !graph.contains(id));
@@ -84,39 +84,6 @@ class _MirrorBuildScriptUpdates implements BuildScriptUpdates {
     if (!_supportsIncrementalRebuilds) return true;
     return updatedIds.intersection(_allSources).isNotEmpty;
   }
-
-  /// Attempts to return an [AssetId] for [uri].
-  ///
-  /// Returns `null` if the uri should be ignored, or throws an [ArgumentError]
-  /// if the [uri] is not recognized.
-  static AssetId? _idForUri(Uri uri, String rootPackage) {
-    switch (uri.scheme) {
-      case 'dart':
-        // TODO: check for sdk updates!
-        break;
-      case 'package':
-        var parts = uri.pathSegments;
-        return AssetId(parts[0],
-            p.url.joinAll(['lib', ...parts.getRange(1, parts.length)]));
-      case 'file':
-        var relativePath = p.relative(uri.toFilePath(), from: p.current);
-        return AssetId(rootPackage, relativePath);
-      case 'data':
-        // Test runner uses a `data` scheme, don't invalidate for those.
-        if (uri.path.contains('package:test')) break;
-        continue unsupported;
-      case 'http':
-        continue unsupported;
-      unsupported:
-      default:
-        throw ArgumentError('Unsupported uri scheme `${uri.scheme}` found for '
-            'library in build script.\n'
-            'This probably means you are running in an unsupported '
-            'context, such as in an isolate or via `dart run`.\n'
-            'Full uri was: $uri.');
-    }
-    return null;
-  }
 }
 
 /// Always returns false for [hasBeenUpdated], used when we want to skip
@@ -125,3 +92,47 @@ class _NoopBuildScriptUpdates implements BuildScriptUpdates {
   @override
   bool hasBeenUpdated(void _) => false;
 }
+
+/// Attempts to return an [AssetId] for [uri].
+///
+/// Returns `null` if the uri should be ignored, or throws an [ArgumentError]
+/// if the [uri] is not recognized.
+@visibleForTesting
+AssetId? idForUri(Uri uri, PackageGraph packageGraph) {
+  switch (uri.scheme) {
+    case 'dart':
+      // TODO: check for sdk updates!
+      break;
+    case 'package':
+      var parts = uri.pathSegments;
+      return AssetId(
+          parts[0], p.url.joinAll(['lib', ...parts.getRange(1, parts.length)]));
+    case 'file':
+      final package = packageGraph.asPackageConfig
+          .packageOf(Uri.file(p.canonicalize(uri.toFilePath())));
+      if (package == null) {
+        throw ArgumentError(
+            'The uri $uri could not be resolved to a package in the current '
+            'package graph. Do you have a dependency on the package '
+            'containing this uri?');
+      }
+      // The `AssetId` constructor normalizes this path to a URI style.
+      var relativePath =
+          p.relative(uri.toFilePath(), from: package.root.toFilePath());
+      return AssetId(package.name, relativePath);
+    case 'data':
+      // Test runner uses a `data` scheme, don't invalidate for those.
+      if (uri.path.contains('package:test')) break;
+      continue unsupported;
+    case 'http':
+      continue unsupported;
+    unsupported:
+    default:
+      throw ArgumentError('Unsupported uri scheme `${uri.scheme}` found for '
+          'library in build script.\n'
+          'This probably means you are running in an unsupported '
+          'context, such as in an isolate or via `dart run`.\n'
+          'Full uri was: $uri.');
+  }
+  return null;
+}

build_runner_core/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build_runner_core
-version: 7.2.9
+version: 7.2.10
 description: Core tools to organize the structure of a build and run Builders.
 repository: https://github.com/dart-lang/build/tree/master/build_runner_core
 

build_runner_core/test/changes/build_script_updates_test.dart

@@ -0,0 +1,55 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:build/build.dart';
+import 'package:build_runner_core/src/changes/build_script_updates.dart';
+import 'package:build_runner_core/src/package_graph/package_graph.dart';
+import 'package:package_config/package_config_types.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('idForUri', () {
+    late final PackageGraph packageGraph;
+    setUpAll(() async {
+      final rootPackage = PackageNode(
+          'a', '/a/', DependencyType.path, LanguageVersion(3, 0),
+          isRoot: true);
+      final dependency =
+          PackageNode('b', '/b/', DependencyType.path, LanguageVersion(3, 0));
+      rootPackage.dependencies.add(dependency);
+      packageGraph = PackageGraph.fromRoot(rootPackage);
+    });
+
+    test('dart: uris return null', () {
+      expect(idForUri(Uri.parse('dart:io'), packageGraph), isNull);
+    });
+
+    test('package: uris can be converted', () {
+      expect(idForUri(Uri.parse('package:a/a.dart'), packageGraph),
+          AssetId('a', 'lib/a.dart'));
+    });
+
+    test('file: uris can be looked up', () {
+      expect(idForUri(Uri.file('/a/lib/a.dart'), packageGraph),
+          AssetId('a', 'lib/a.dart'));
+      expect(idForUri(Uri.file('/b/b.dart'), packageGraph),
+          AssetId('b', 'b.dart'));
+    });
+    test('data: arent supported unless they are from the test runner', () {
+      expect(
+          () => idForUri(
+              Uri.parse('data:text/plain;charset=UTF-8,foo'), packageGraph),
+          throwsA(isA<ArgumentError>()));
+      expect(
+          idForUri(Uri.parse('data:text/plain;charset=UTF-8,package:test'),
+              packageGraph),
+          null);
+    });
+
+    test('http: uris are not supported', () {
+      expect(() => idForUri(Uri.parse('http://google.com'), packageGraph),
+          throwsA(isA<ArgumentError>()));
+    });
+  });
+}