dart-lang / package_config Goto Github PK

For analyzer (and server) integration we need a means to map a directory to it's subdirectories which have corresponding package maps (and so should be treated as analysis context roots).

Not a good name but this captures the basic idea.

/// Given this directory find all subdirectories for which there is a new
/// package map.
static Future<Map<Directory,Packages>> findAll(Directory dir);

I have this in my logs trying to build flutter engine:

Activated generate_package_config 0.0.0 at path "/tmp/clean_engine/src/flutter/tools/generate_package_config".
Unable to spawn isolate: src/flutter/tools/generate_package_config/bin/generate_from_legacy.dart:10:8: Error: Error when reading '../.pub-cache/hosted/pub.dartlang.org/package_config-2.0.0/lib/packages_file.dart': No such file or directory
import 'package:package_config/packages_file.dart'; // ignore: deprecated_member_use
       ^
src/flutter/tools/generate_package_config/bin/generate_from_legacy.dart:28:20: Error: Method not found: 'parse'.
  var packageMap = parse(await packagesFile.readAsBytes(), packagesFile.uri);
                   ^^^^^
Traceback (most recent call last):
  File "src/flutter/tools/run_third_party_dart.py", line 14, in <module>
    subprocess.check_call([os.path.join(leading, pub), "global", "run", "generate_package_config:generate_from_legacy", "src/flutter/flutter_frontend_server/.packages"])
  File "/usr/lib/python2.7/subprocess.py", line 190, in check_call
    raise CalledProcessError(retcode, cmd)

Likely packages_file.dart is needed back?

It would be great to use this library to try the various mechanisms to produce a package "mapping":

--package-root
--packages
look next to the entry point for a packages/ dir
look next to entry point for a .packages file
try a parent directory for a .packages file

checkValidPackageUri currently throws ArgumentErrors when the input is invalid (see https://github.com/dart-lang/package_config/blob/master/lib/src/util.dart#L46).

We need to wrap and catch these errors and are afraid that by catching ArgumentError we might be catching other errors that are not related to package validation.

Adding a new error class (even a subclass of ArgumentError) would let us filter out the relevant errors.

The behavior I expected is that all packages in the allow list would be opted in in this case, since they get the latest language version.

This behavior is not however fully specified, and making this change today would be very breaking. Primarily this is a question of how exactly things break when only a regular .packages file is present. Either some packages in the allow list that are not actually migrated yet would be opted in when they shouldn't be, or migrated versions will not be opted in (the current behavior).

An example repro is to check out the collection package, and add this file at lib/test.dart:

main() {
  int? x;
  print(x);
}

If you run that with dart lib/test.dart it works fine.

If you then add an empty .packages file at lib/.packages and re-run, it will fail saying the package is not opted in.

Related to #62 (and would likely be solved by solving that).

Assuming you have a package config file already loaded in memory you should be able to synchronously parse and load that config into a PackageConfig instance.

When trying to resolve "package:_internal/foo.dart" package_config complains that "_internal" is not a valid identifier.

The compiler crashed: Invalid argument (packageUri): Package names must be valid identifiers: Instance of 'Uri'
#0      checkValidPackageUri (package:package_config/src/util.dart:65)
#1      _PackagesBase.resolve (package:package_config/src/packages_impl.dart:37)
#2      Compiler.translatePackageUri (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/apiimpl.dart:343)
#3      Compiler.translateUri (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/apiimpl.dart:287)
#4      Compiler.readScript (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/apiimpl.dart:234)
#5      _LibraryLoaderTask.createLibrary.<anonymous closure> (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/library_loader.dart:550)
#6      Compiler.withCurrentElement (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/compiler.dart:908)
#7      _LibraryLoaderTask.createLibrary (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/library_loader.dart:549)
#8      _LibraryLoaderTask.registerLibraryFromTag (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/library_loader.dart:519)
#9      _LibraryLoaderTask.processLibraryTags.<anonymous closure>.<anonymous closure>.<anonymous closure> (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/library_loader.dart:427)
#10     Compiler.withCurrentElement (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/compiler.dart:908)
#11     _LibraryLoaderTask.processLibraryTags.<anonymous closure>.<anonymous closure> (file:///usr/local/google/home/het/code/dart/sdk/pkg/compiler/lib/src/library_loader.dart:426)
#12     Future.forEach.<anonymous closure>.<anonymous closure> (dart:async/future.dart:336)
#13     Future.Future.sync (dart:async/future.dart:168)
#14     Future.forEach.<anonymous closure> (dart:async/future.dart:336)
#15     Future.Future.sync (dart:async/future.dart:168)
#16     Future.doWhile.<anonymous closure> (dart:async/future.dart:361)
#17     _RootZone.runUnaryGuarded (dart:async/zone.dart:1104)
#18     _RootZone.bindUnaryCallback.<anonymous closure> (dart:async/zone.dart:1133)
#19     _RootZone.runUnary (dart:async/zone.dart:1166)
#20     _Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:494)
#21     _Future._propagateToListeners (dart:async/future_impl.dart:577)
#22     _Future._completeWithValue (dart:async/future_impl.dart:368)
#23     _Future._asyncComplete.<anonymous closure> (dart:async/future_impl.dart:422)
#24     _microtaskLoop (dart:async/schedule_microtask.dart:43)
#25     _microtaskLoopEntry (dart:async/schedule_microtask.dart:52)
#26     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:96)
#27     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:149)

It looks like a Uri needs to be encoded to a String before attempting to encode as JSON:

  Converting object to an encodable object failed: Instance of '_Uri'
  dart:convert                                                  JsonEncoder.convert
  package:package_config/src/package_config_json.dart 372:53    writePackageConfigJson
  package:package_config/package_config.dart 176:5              savePackageConfig
  test/runner/precompiled_test.dart 237:9                       _writePackagesFile

This is when depending on the 1.9 branch.

https://www.dartdocs.org/documentation/package_config/1.0.0/

I can find PackageContext:
https://www.dartdocs.org/documentation/package_config/1.0.0/package_config.discovery_analysis/PackageContext-class.html

But even there the Packages links don't work.

When pub creates a .dart_tool/package_config.json file the package entries look like:

    {
      "name": "_fe_analyzer_shared",
      "rootUri": "file:///Users/sigurdm/.pub-cache/hosted/pub.dartlang.org/_fe_analyzer_shared-11.0.0",
      "packageUri": "lib/",
      "languageVersion": "2.2"
    },

When flutter loads that package_config and adds a package and rewrites the package_config with an extra entry (https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/dart/pub.dart#L470), the entries end up like:

    {
      "name": "_fe_analyzer_shared",
      "rootUri": "../../../.pub-cache/hosted/pub.dartlang.org/_fe_analyzer_shared-12.0.0",
      "packageUri": "lib",
      "languageVersion": "2.2"
    },

Which is surprising (that you cannot round-trip without going to relative paths).

Also 'lib/' is changed to 'lib' - kind of unnecessarily.

Coming up in the context of flutter/flutter#73870

See comment around: (https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/dart/pub.dart#L470)

This functionality exists in src/ but is not exposed in the public interface.

Static methods like PackageConfig.find, PackageConfig.load would be more discoverable than the top level ones.

I believe we're failing - on validation - for URLs which are valid. Here's a failure I see:

Invalid argument (root): In package analyzer_plugin: Not an absolute URI with no query or fragment with a path ending in /: "/Users/devoncarew/projects/dart-lang/sdk/sdk/pkg/analyzer_plugin"
#0      throwError (package:package_config/src/errors.dart:32:35)
#1      SimplePackage.validate (package:package_config/src/package_config_impl.dart:234:14)
#2      new Package (package:package_config/src/package_config.dart:239:21)
#3      makePackageConfigs (file:///Users/devoncarew/projects/dart-lang/sdk/sdk/tools/generate_package_config.dart:129:11)
...

We're looking for a path ending in in /, but the path I'm passing in is one we see generated from most of our APIs (File.uri, File.path, package:path absolute(), ...).

It would be nice if tools could report where in the .packages file a parse error occurred.

the DEP now allows more than identifiers (any pchar with a couple restrictions), we just need to update the implementation to match.

Currently the changelog entry for version 1.1 just says "syntax updates", with no more context. This doesn't provide a user with useful information about what version to depend on. Please include more information: what part of the syntax was updated? In what ways is it still backwards-compatible? Can you link to a spec update the user can look at for more information?

The badge in the README is linking over to the linter project.

dart2js must be able to compile itself so it cannot use 'dart:io'. This means I can't use the discovery mechanism in this repo, but I would still like to use the Packages class. We should move things around a little to separate out the 'dart:io' dependent code from the platform agnostic code.

One thing the package_resolver package did (which we want to deprecate) is allow you to get a config from the current isolate easily. This is nice for doing package uri resolution which is identical to the current isolates resolution strategy while also having the flexibility to accept a custom package config (so just relying on the Isolate.resolvePackageUri isn't enough there).

The isolate class has a packageConfig getter, but that could presumably be either a .packages file or a .dart_tool/package_config.json file. Using the existing loadPackageConfigUri with that will throw if it is a .packages file with a format exception.

The readme.md file for this repo has a broken link (a 404) to the package config design doc:

https://github.com/dart-lang/language/blob/master/accepted/future-releases/language-versioning/package-config-file-v2.md

This package should make sure to use recursive: true when creating files.

This primarily causes problems in tests, typical use cases already would have a .dart_tool dir from a pub get/upgrade.

The doc comment says relative uris are allowed (which are then resolved relative to root). However passing a relative Uri gives this error: Invalid argument (packageUriRoot): In package example_a: Not an absolute URI with no query or fragment with a path ending in /: Instance of '_Uri'.

Either the docs or behavior should be updated.

Migrating from all of the deprecated APIs/libraries (like packages_file.parse) to supported ones (like package_config_discovery.loadPackageConfigUri) currently requires a change to asynchronous code.

I'd like to use the internal synchronous functions, like lib/src/package_config_io.dart's parseAnyConfigFile, as it is synchronous, and the public loadPackageConfigUri is more-or-less an asynchronous wrapper around it.

I wanted to get a "feel" for the API. Can you add a small snippet to the README?

Thanks!

Consider an entry like:

foo:../bar/lib

If I parse that with parse(), the URI I get back is a full file:// URI.

I can't seem to find a way to get the literal and original URI from the .packages file, after parsing.

Any thoughts? Thanks!

'''test_package:lib/''' parses

but:

'''test_package:lib/
'''

results in the error:

FormatException: No ':' on line (at offset 16)

Because the analyzer doesn't surface these errors I just wasted hours tracking this down.

1.2 is already in the changelog – but never shipped! (we should fix that either way)

Any reason to jump to 1.9?

CC @jakemac53

Hi there @kevmoo @lrhn,

We are seeing widespread failures across all of our Dart CI builds at Workiva as a result of the 1.9.0 release of this package.

Failures are of the form:

Failed to load "<path_to_a_test_entrypoint>.dart": Failed to load script at 
http://localhost:43031/DT4tNLNNdu4ivzkLd5RQn9vcGbvHr%2FRe/<path_to_a_test_entrypoint>.dart.browser_test.dart.js.

http depends on mirrors, which means that dart2js can't use it. We only use it for http.get, which can be written using just HttpClient without mirrors.

Paths are not case sensitive on windows, but this package treats them as if they are in the packageOf function.

In particular, this is annoying because:

• Pub will put paths with upper case in the package config
• The canonicalize function from package:path will lower case everything on windows.

These are essentially incompatible - a package config created from pub will not be able to find the containing package for any file URI whose path has been canonicalized.

Cross-posting https://github.com/dart-lang/build/issues/2687 here, as I'm not sure of the severity of the issue:

You have hit a bug in build_runner
Please file an issue with reproduction steps at https://github.com/dart-lang/build/issues
NoSuchMethodError: Closure call with mismatched arguments: function 'MutablePackageTree.allPackages'
Receiver: Closure: (_SyncIterator<Package>) => bool
Tried calling: MutablePackageTree.allPackages()
Found: MutablePackageTree.allPackages(_SyncIterator<Package>) => bool
dart:core                                                              _SyncIterable.iterator
package:package_config/src/package_config_impl.dart 26:36              new SimplePackageConfig
package:package_config/src/package_config_json.dart 213:10             parsePackageConfigJson
package:package_config/src/package_config_json.dart 45:10              parsePackageConfigBytes
package:package_config/src/package_config_io.dart 117:10               readPackageConfigJsonFile
package:package_config/src/discovery.dart 109:18                       findPackagConfigInDirectory
package:package_config/src/discovery.dart 44:31                        findPackageConfig
package:package_config/package_config.dart 114:5                       findPackageConfig
package:build_runner_core/src/package_graph/package_graph.dart 74:15   PackageGraph.forPath
package:build_runner_core/src/package_graph/package_graph.dart 110:20  PackageGraph.forThisPackage
package:build_runner/src/entrypoint/run.dart 22:64                     run
.dart_tool/build/entrypoint/build.dart 96:22                           main

That means this library cannot be in the transitive imports of any web application - which was previously not the case.

Previous versions of package_config only included these dependencies via a discovery.dart file, and a similar pattern likely should be used here.

This unfortunately kind of leaks all over the place in this package though right now.

Note that the test package bootstrapping currently uses package_resolver so it does require that we can compile and run code on the web.

I want to migrate the test package off of package_resolver and to package_config, but either way it ends up with package_config in the transitive imports.

The language versioning doc establishes that the fragment of the URL is url-encoded &/= key-values, similar to a querystring.

While version 1.4 can both parse and generate such .packages files, it leaves parsing of the fragment as querystring to the user. Maybe we should make some slightly higher abstractions that does this?

How much of the functionality in this package would we want to port forwards?

The concern is that someone using this library might parse the fragment with fragment.split('=') which works as long as:
A) there is no percent encoding,
B) there is only one key.

But the language versioning doc allows for both (A) and (B) in the future. At least as I understand it.

Lets start with a question:
Something like this does not give an error (copied from a test I've added which is why this looks sort of weird):

        'packages': [
          {'name': 'foo', 'rootUri': '/foo/', 'packageUri': 'lib/'},
          {'name': 'bar', 'rootUri': '/foo/lib/bar/', 'packageUri': 'lib/'},
        ]

but at the same time "bar" seems to not really have any effect:

      expect(config.packageOf(Uri.parse('file:///foo/lib/bar/lib/lala.dart'))!.name,
          'foo'); // why not bar?
      expect(config.toPackageUri(Uri.parse('file:///foo/lib/bar/lib/diz')),
          Uri.parse('package:foo/bar/lib/diz')); // why not package:bar/diz?

Is this how it's supposed to be?

Also, for the O(n^2) stuff: Adding to MutablePackageTree is O(n^2), i.e. super slow for big packages files. E.g. this "test":

diff --git a/test/parse_test.dart b/test/parse_test.dart
index d5c2e72..9b8214c 100644
--- a/test/parse_test.dart
+++ b/test/parse_test.dart
@@ -165,6 +165,37 @@ void main() {
       });
     });
 
+    test('big non-nested file', () {
+      var sb = StringBuffer();
+      sb.writeln('{');
+      sb.writeln('"configVersion": 2,');
+      sb.writeln('"packages": [');
+      for (var i = 0; i < 5000; i++) {
+        if (i != 0) {
+          sb.writeln(',');
+        }
+        sb.writeln('{');
+        sb.writeln('  "name": "p_$i",');
+        sb.writeln('  "rootUri": "file:///p_$i/",');
+        sb.writeln('  "packageUri": "lib/",');
+        sb.writeln('  "languageVersion": "2.5",');
+        sb.writeln('  "nonstandard": true');
+        sb.writeln('}');
+      }
+      sb.writeln('],');
+      sb.writeln('"generator": "pub",');
+      sb.writeln('"other": [42]');
+      sb.writeln('}');
+      var stopwatch = Stopwatch()..start();
+      var config = parsePackageConfigBytes(
+          // ignore: unnecessary_cast
+          utf8.encode(sb.toString()) as Uint8List,
+          Uri.parse('file:///tmp/.dart_tool/file.dart'),
+          throwError);
+      print('Read in ${stopwatch.elapsedMilliseconds} ms');
+      expect(config.version, 2);
+    });
+

where one can easily change the number of packages yields these numbers on my machine:

1,000 => 56 ms
5,000 => 494 ms
10,000 => 2,700 ms
50,000 => 81,380 ms

I've attempted another approach that passes all the tests and is a lot faster for big input:

diff --git a/lib/src/package_config_impl.dart b/lib/src/package_config_impl.dart
index 4167d35..6e22cca 100644
--- a/lib/src/package_config_impl.dart
+++ b/lib/src/package_config_impl.dart
@@ -54,7 +54,8 @@ class SimplePackageConfig implements PackageConfig {
   static PackageTree _validatePackages(Iterable<Package> originalPackages,
       List<Package> packages, void Function(Object error) onError) {
     var packageNames = <String>{};
-    var tree = MutablePackageTree();
+    var tree = TrielikePackageTree();
+    // var tree = MutablePackageTree();
     for (var originalPackage in packages) {
       SimplePackage? package;
       if (originalPackage is! SimplePackage) {
@@ -367,6 +368,116 @@ abstract class PackageTree {
   SimplePackage? packageOf(Uri file);
 }
 
+class _TrielikePackageTreeHelper {
+  SimplePackage? package;
+  Map<String, _TrielikePackageTreeHelper> map = {};
+}
+
+/// Packages of a package configuration ordered by root path.
+///
+/// A package has a root path and a package root path, where the latter
+/// contains the files exposed by `package:` URIs.
+///
+/// A package is said to be inside another package if the root path URI of
+/// the latter is a prefix of the root path URI of the former.
+///
+/// No two packages of a package may have the same root path, so this
+/// path prefix ordering defines a tree-like partial ordering on packages
+/// of a configuration.
+///
+/// The package root path of a package must not be inside another package's
+/// root path.
+/// Entire other packages are allowed inside a package's root or
+/// package root path.
+///
+/// The package tree contains an ordered mapping of unrelated packages
+/// (represented by their name) to their immediately nested packages' names.
+class TrielikePackageTree implements PackageTree {
+  Map<String, _TrielikePackageTreeHelper> _map = {};
+
+  /// A list of all packages.
+  final List<SimplePackage> _packages = [];
+
+  @override
+  Iterable<Package> get allPackages sync* {
+    for (var package in _packages) {
+      yield package;
+    }
+  }
+
+  /// Tries to (add) `package` to the tree.
+  ///
+  /// Reports a [ConflictException] if the added package conflicts with an
+  /// existing package.
+  /// It conflicts if its root or package root is the same as another
+  /// package's root or package root, or is between the two.
+  ///
+  /// If a conflict is detected between [package] and a previous package,
+  /// then [onError] is called with a [ConflictException] object
+  /// and the [package] is not added to the tree.
+  ///
+  /// The packages are added in order of their root path.
+  /// It is never necessary to insert a node between two existing levels.
+  void add(
+      int start, SimplePackage package, void Function(Object error) onError) {
+    var root = package.root;
+    var currentMapHelper = _map[root.scheme] ??= _TrielikePackageTreeHelper();
+    // For now assume we don't have stuff directly here.
+    var segments = root.pathSegments;
+    for (var i = 0; i < segments.length - 1; i++) {
+      var path = segments[i];
+      currentMapHelper = currentMapHelper.map[path] ??= _TrielikePackageTreeHelper();
+      if (currentMapHelper.package != null) {
+        // Trying to add package that is inside an already added package.
+        // 1) If it's an exact match it's not allowed.
+        if (i == segments.length - 2) {
+          onError(ConflictException.root(package, currentMapHelper.package!));
+          return;
+        }
+        // 2) The already added package has a packageUriRoot thats inside the
+        //    root of the package we're trying to add.
+        if (_beginsWith(0, package.root.toString(),
+            currentMapHelper.package!.packageUriRoot.toString())) {
+          onError(ConflictException.packageRoot(
+              package, currentMapHelper.package!));
+          return;
+        }
+      }
+    }
+    currentMapHelper.package = package;
+    _packages.add(package);
+  }
+
+  @override
+  SimplePackage? packageOf(Uri file) {
+    var currentMapHelper = _map[file.scheme];
+    if (currentMapHelper == null) return null;
+    // For now assume we don't have stuff directly here.
+    var segments = file.pathSegments;
+    var potential = [];
+    var path = file.toString();
+    for (var i = 0; i < segments.length - 1; i++) {
+      var segment = segments[i];
+      currentMapHelper = currentMapHelper!.map[segment];
+      if (currentMapHelper == null) break;
+      if (currentMapHelper.package != null) {
+        var currentPackage = currentMapHelper.package!;
+        var currentPackageRootLength = currentPackage.root.toString().length;
+        if (path.length == currentPackageRootLength) return currentPackage;
+        var currentPackageUriRoot = currentPackage.packageUriRoot.toString();
+        // Is [file] is inside the package root of [currentPackage]?
+        if (currentPackageUriRoot.length == currentPackageRootLength ||
+            _beginsWith(currentPackageRootLength, currentPackageUriRoot, path)) {
+          return currentPackage;
+        }
+        potential.add(currentPackage);
+      }
+    }
+    if (potential.isEmpty) return null;
+    return potential.last;
+  }
+}
+
 /// Packages of a package configuration ordered by root path.
 ///
 /// A package has a root path and a package root path, where the latter
diff --git a/test/parse_test.dart b/test/parse_test.dart
index d5c2e72..9b8214c 100644
--- a/test/parse_test.dart
+++ b/test/parse_test.dart
@@ -165,6 +165,37 @@ void main() {
       });
     });
 
+    test('big non-nested file', () {
+      var sb = StringBuffer();
+      sb.writeln('{');
+      sb.writeln('"configVersion": 2,');
+      sb.writeln('"packages": [');
+      for (var i = 0; i < 5000; i++) {
+        if (i != 0) {
+          sb.writeln(',');
+        }
+        sb.writeln('{');
+        sb.writeln('  "name": "p_$i",');
+        sb.writeln('  "rootUri": "file:///p_$i/",');
+        sb.writeln('  "packageUri": "lib/",');
+        sb.writeln('  "languageVersion": "2.5",');
+        sb.writeln('  "nonstandard": true');
+        sb.writeln('}');
+      }
+      sb.writeln('],');
+      sb.writeln('"generator": "pub",');
+      sb.writeln('"other": [42]');
+      sb.writeln('}');
+      var stopwatch = Stopwatch()..start();
+      var config = parsePackageConfigBytes(
+          // ignore: unnecessary_cast
+          utf8.encode(sb.toString()) as Uint8List,
+          Uri.parse('file:///tmp/.dart_tool/file.dart'),
+          throwError);
+      print('Read in ${stopwatch.elapsedMilliseconds} ms');
+      expect(config.version, 2);
+    });
+
     test('valid other order', () {
       // The ordering in the file is not important.
       var packageConfigFile = '''
@@ -272,6 +303,33 @@ void main() {
           Uri.parse('package:qux/diz'));
     });
 
+
+    test('nested packages 2', () {
+      var configBytes = utf8.encode(json.encode({
+        'configVersion': 2,
+        'packages': [
+          {'name': 'foo', 'rootUri': '/foo/', 'packageUri': 'lib/'},
+          {'name': 'bar', 'rootUri': '/foo/lib/bar/', 'packageUri': 'lib/'},
+        ]
+      }));
+      // ignore: unnecessary_cast
+      var config = parsePackageConfigBytes(configBytes as Uint8List,
+          Uri.parse('file:///tmp/.dart_tool/file.dart'), throwError);
+      expect(config.version, 2);
+      expect(config.packageOf(Uri.parse('file:///foo/lala/lala.dart'))!.name,
+          'foo');
+      expect(config.packageOf(Uri.parse('file:///foo/lib/lala.dart'))!.name,
+          'foo');
+      expect(config.packageOf(Uri.parse('file:///foo/lib/bar/lala.dart'))!.name,
+          'foo');
+      expect(config.packageOf(Uri.parse('file:///foo/lib/bar/lib/lala.dart'))!.name,
+          'foo'); // why not bar?
+      expect(config.toPackageUri(Uri.parse('file:///foo/lib/diz')),
+          Uri.parse('package:foo/diz'));
+      expect(config.toPackageUri(Uri.parse('file:///foo/lib/bar/lib/diz')),
+          Uri.parse('package:foo/bar/lib/diz')); // why not package:bar/diz?
+    });
+
     group('invalid', () {
       void testThrows(String name, String source) {
         test(name, () {
@@ -375,8 +433,6 @@ void main() {
       });
       testThrows('duplicate package name',
           '{$cfg,"packages":[{$name,$root},{$name,"rootUri":"/other/"}]}');
-      testThrows('same roots',
-          '{$cfg,"packages":[{$name,$root},{"name":"bar",$root}]}');
       testThrows(
           // The roots of foo and bar are the same.
           'same roots',

giving these savings:

1,000 => 56 ms --- now 38 ms
5,000 => 494 ms --- now 135 ms
10,000 => 2,700 ms --- now 168 ms
50,000 => 81,380 ms --- now 492 ms

It probably needs some polish --- but are there any pitfalls I'm overlooking?

/cc @lrhn

Once #96 lands, are we good to publish?

What do you think @lrhn?

This is when depending on the 1.9-allow-overlap branch.

If you call PackageConfig.parse with invalid config (I tried passing a .packages file), then you get this stack trace instead of the desired parse error:

  NoSuchMethodError: The method 'call' was called on null.
  Receiver: null
  Tried calling: call()
  package:package_config/src/package_config_json.dart 54:12  parsePackageConfigString
  package:package_config/src/package_config.dart 94:7        PackageConfig.parseString
  package:coverage/src/resolver.dart 93:34                   Resolver._parsePackages
  package:coverage/src/resolver.dart 15:44                   new Resolver
  test/lcov_test.dart 42:24                                  main.<fn>.<fn>

This is because it calls onError(PackageConfigFormatException(e.message, e.source, e.offset)); without checking if onError is null first.

We should likely throw the new exception instead in that case to get a better message.

The test_core package currently sends a .packages file to the browser for stack trace mapping purposes. It sends the same one that is used to compile with dart2js, and so it needs to be able to serialize/deserialize it.

This functionality exists but is hidden. Deserializing can be done but is cumbersome (using a loader or fake file), but serialization cannot be done today as the only exposed api just takes a directory.

This also makes testing anything that takes a package config in its api more difficult than it should be (a PackageConfig.fromJson or equivalent api would be easier to use).

See also #59 which is related - exposing the dart:io types causes other problems and should really be hidden behind a different library entrypoint. The main one should only deal in terms of Strings or bytes imo and be cross platform.

@lrhn

This is to support imports like package:foo.bar.baz/src/quox.dart.

We could cleanup packagemap.dart and get away from defining our own stand-in:

static Uri _normalizePath(Uri existingUri) => new Uri().resolveUri(existingUri);

and migrate to using Uri.normalizePath().

If we do this, we'll need to up our SDK contsraint to be >=1.11.

This will allow workflows in the flutter repo to continue producing their .packages files, but also get a valid .dart_tool/package_config.json file from that.

Generally it is much easier for users to produce the .packages files as opposed to implementing the reading of pubspecs etc themselves.

Currently, parse() always returns absolute URIs. While this resolution is usually useful, sometimes it's necessary for the caller to have the extra context of whether the URI was originally relative; for example, pub wants to use this to test whether it generates the correct properly relative URIs in its package specs.

In theory you could work around this by passing in . as baseLocation, but dart-lang/sdk#23809 prevents that.