From e460ffc96c570f416733f7a3405b6fd070f0396f Mon Sep 17 00:00:00 2001 From: "nweiz@google.com" <nweiz@google.com> Date: Thu, 22 Jan 2015 01:07:38 +0000 Subject: [PATCH] Regenerate pub's sources and remove workarounds for fixed issues. R=rnystrom@google.com Review URL: https://codereview.chromium.org//868463003 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@43057 260f80e4-7a28-3924-810f-c04153c831b5 --- lib/src/barback/transformer_loader.dart | 18 +++--- lib/src/entrypoint.dart | 76 ++++++++++++------------- lib/src/executable.dart | 52 ++++++++--------- lib/src/global_packages.dart | 76 ++++++++++++------------- lib/src/source/hosted.dart | 7 +-- lib/src/validator/dependency.dart | 7 +-- 6 files changed, 110 insertions(+), 126 deletions(-) diff --git a/lib/src/barback/transformer_loader.dart b/lib/src/barback/transformer_loader.dart index 2d9f3338..031c384c 100644 --- a/lib/src/barback/transformer_loader.dart +++ b/lib/src/barback/transformer_loader.dart @@ -97,17 +97,13 @@ class TransformerLoader { return new Future.value(new Set()); } - // TODO(nweiz): This is currently wrapped in a non-async closure to work - // around https://github.com/dart-lang/async_await/issues/51. When that - // issue is fixed, make this inline. - var transformer = () { - try { - return new Dart2JSTransformer.withSettings(_environment, - new BarbackSettings(config.configuration, _environment.mode)); - } on FormatException catch (error, stackTrace) { - fail(error.message, error, stackTrace); - } - }(); + var transformer; + try { + transformer = new Dart2JSTransformer.withSettings(_environment, + new BarbackSettings(config.configuration, _environment.mode)); + } on FormatException catch (error, stackTrace) { + fail(error.message, error, stackTrace); + } // Handle any exclusions. _transformers[config] = new Set.from( diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 811ae0bf..d207147a 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -143,15 +143,14 @@ class Entrypoint { var packageGraph = await loadPackageGraph(result); packageGraph.loadTransformerCache().clearIfOutdated(result.changedPackages); - // TODO(nweiz): Use await here when - // https://github.com/dart-lang/async_await/issues/51 is fixed. - return precompileDependencies(changed: result.changedPackages).then((_) { - return precompileExecutables(changed: result.changedPackages); - }).catchError((error, stackTrace) { + try { + await precompileDependencies(changed: result.changedPackages); + await precompileExecutables(changed: result.changedPackages); + } catch (error, stackTrace) { // Just log exceptions here. Since the method is just about acquiring // dependencies, it shouldn't fail unless that fails. log.exception(error, stackTrace); - }); + } } /// Precompile any transformed dependencies of the entrypoint. @@ -201,42 +200,43 @@ class Entrypoint { if (dependenciesToPrecompile.isEmpty) return; - await log.progress("Precompiling dependencies", () async { - var packagesToLoad = - unionAll(dependenciesToPrecompile.map(graph.transitiveDependencies)) - .map((package) => package.name).toSet(); - - var environment = await AssetEnvironment.create(this, BarbackMode.DEBUG, - packages: packagesToLoad, useDart2JS: false); - - /// Ignore barback errors since they'll be emitted via [getAllAssets] - /// below. - environment.barback.errors.listen((_) {}); - - // TODO(nweiz): only get assets from [dependenciesToPrecompile] so as not - // to trigger unnecessary lazy transformers. - var assets = await environment.barback.getAllAssets(); - await waitAndPrintErrors(assets.map((asset) async { - if (!dependenciesToPrecompile.contains(asset.id.package)) return; - - var destPath = path.join( - depsDir, asset.id.package, path.fromUri(asset.id.path)); - ensureDir(path.dirname(destPath)); - await createFileFromStream(asset.read(), destPath); - })); + try { + await log.progress("Precompiling dependencies", () async { + var packagesToLoad = + unionAll(dependenciesToPrecompile.map(graph.transitiveDependencies)) + .map((package) => package.name).toSet(); + + var environment = await AssetEnvironment.create(this, BarbackMode.DEBUG, + packages: packagesToLoad, useDart2JS: false); + + /// Ignore barback errors since they'll be emitted via [getAllAssets] + /// below. + environment.barback.errors.listen((_) {}); + + // TODO(nweiz): only get assets from [dependenciesToPrecompile] so as + // not to trigger unnecessary lazy transformers. + var assets = await environment.barback.getAllAssets(); + await waitAndPrintErrors(assets.map((asset) async { + if (!dependenciesToPrecompile.contains(asset.id.package)) return; + + var destPath = path.join( + depsDir, asset.id.package, path.fromUri(asset.id.path)); + ensureDir(path.dirname(destPath)); + await createFileFromStream(asset.read(), destPath); + })); - log.message("Precompiled " + - toSentence(ordered(dependenciesToPrecompile).map(log.bold)) + "."); - }).catchError((error) { - // TODO(nweiz): Do this in a catch clause when async_await supports - // "rethrow" (https://github.com/dart-lang/async_await/issues/46). + log.message("Precompiled " + + toSentence(ordered(dependenciesToPrecompile).map(log.bold)) + "."); + }); + } catch (_) { // TODO(nweiz): When barback does a better job of associating errors with // assets (issue 19491), catch and handle compilation errors on a // per-package basis. - dependenciesToPrecompile.forEach((package) => - deleteEntry(path.join(depsDir, package))); - throw error; - }); + for (var package in dependenciesToPrecompile) { + deleteEntry(path.join(depsDir, package)); + } + rethrow; + } } /// Precompiles all executables from dependencies that don't transitively diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 38105925..966b68f6 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -106,32 +106,9 @@ Future<int> runExecutable(Entrypoint entrypoint, String package, server = await environment.servePackageBinDirectory(package); } - // TODO(rnystrom): Use try/catch here when - // https://github.com/dart-lang/async_await/issues/4 is fixed. - return environment.barback.getAssetById(id).then((_) async { - var vmArgs = []; - - // Run in checked mode. - // TODO(rnystrom): Make this configurable. - vmArgs.add("--checked"); - - // Get the URL of the executable, relative to the server's root directory. - var relativePath = p.url.relative(assetPath, - from: p.url.joinAll(p.split(server.rootDirectory))); - vmArgs.add(server.url.resolve(relativePath).toString()); - vmArgs.addAll(args); - - var process = await Process.start(Platform.executable, vmArgs); - // Note: we're not using process.std___.pipe(std___) here because - // that prevents pub from also writing to the output streams. - process.stderr.listen(stderr.add); - process.stdout.listen(stdout.add); - stdin.listen(process.stdin.add); - - return process.exitCode; - }).catchError((error, stackTrace) { - if (error is! AssetNotFoundException) throw error; - + try { + await environment.barback.getAssetById(id); + } on AssetNotFoundException catch (error, stackTrace) { var message = "Could not find ${log.bold(executable + ".dart")}"; if (package != entrypoint.root.name) { message += " in package ${log.bold(server.package)}"; @@ -140,7 +117,28 @@ Future<int> runExecutable(Entrypoint entrypoint, String package, log.error("$message."); log.fine(new Chain.forTrace(stackTrace)); return exit_codes.NO_INPUT; - }); + } + + var vmArgs = []; + + // Run in checked mode. + // TODO(rnystrom): Make this configurable. + vmArgs.add("--checked"); + + // Get the URL of the executable, relative to the server's root directory. + var relativePath = p.url.relative(assetPath, + from: p.url.joinAll(p.split(server.rootDirectory))); + vmArgs.add(server.url.resolve(relativePath).toString()); + vmArgs.addAll(args); + + var process = await Process.start(Platform.executable, vmArgs); + // Note: we're not using process.std___.pipe(std___) here because + // that prevents pub from also writing to the output streams. + process.stderr.listen(stderr.add); + process.stdout.listen(stdout.add); + stdin.listen(process.stdin.add); + + return process.exitCode; } /// Runs the snapshot at [path] with [args] and hooks its stdout, stderr, and diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index 2a20acaf..6ce58a77 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart @@ -282,52 +282,46 @@ class GlobalPackages { /// Finds the active package with [name]. /// /// Returns an [Entrypoint] loaded with the active package if found. - Future<Entrypoint> find(String name) { - // TODO(rnystrom): Use async/await here when on __ catch is supported. - // See: https://github.com/dart-lang/async_await/issues/27 - return new Future.sync(() { - var lockFilePath = _getLockFilePath(name); - var lockFile; + Future<Entrypoint> find(String name) async { + var lockFilePath = _getLockFilePath(name); + var lockFile; + try { + lockFile = new LockFile.load(lockFilePath, cache.sources); + } on IOException catch (error) { + var oldLockFilePath = p.join(_directory, '$name.lock'); try { - lockFile = new LockFile.load(lockFilePath, cache.sources); + // TODO(nweiz): This looks for Dart 1.6's old lockfile location. + // Remove it when Dart 1.6 is old enough that we don't think anyone + // will have these lockfiles anymore (issue 20703). + lockFile = new LockFile.load(oldLockFilePath, cache.sources); } on IOException catch (error) { - var oldLockFilePath = p.join(_directory, '$name.lock'); - try { - // TODO(nweiz): This looks for Dart 1.6's old lockfile location. - // Remove it when Dart 1.6 is old enough that we don't think anyone - // will have these lockfiles anymore (issue 20703). - lockFile = new LockFile.load(oldLockFilePath, cache.sources); - } on IOException catch (error) { - // If we couldn't read the lock file, it's not activated. - dataError("No active package ${log.bold(name)}."); - } - - // Move the old lockfile to its new location. - ensureDir(p.dirname(lockFilePath)); - new File(oldLockFilePath).renameSync(lockFilePath); + // If we couldn't read the lock file, it's not activated. + dataError("No active package ${log.bold(name)}."); } - // Load the package from the cache. - var id = lockFile.packages[name]; - lockFile.packages.remove(name); - - var source = cache.sources[id.source]; - if (source is CachedSource) { - // For cached sources, the package itself is in the cache and the - // lockfile is the one we just loaded. - return cache.sources[id.source].getDirectory(id) - .then((dir) => new Package.load(name, dir, cache.sources)) - .then((package) { - return new Entrypoint.inMemory(package, lockFile, cache); - }); - } + // Move the old lockfile to its new location. + ensureDir(p.dirname(lockFilePath)); + new File(oldLockFilePath).renameSync(lockFilePath); + } - // For uncached sources (i.e. path), the ID just points to the real - // directory for the package. - assert(id.source == "path"); - return new Entrypoint(PathSource.pathFromDescription(id.description), - cache); - }); + // Load the package from the cache. + var id = lockFile.packages[name]; + lockFile.packages.remove(name); + + var source = cache.sources[id.source]; + if (source is CachedSource) { + // For cached sources, the package itself is in the cache and the + // lockfile is the one we just loaded. + var dir = await cache.sources[id.source].getDirectory(id); + var package = new Package.load(name, dir, cache.sources); + return new Entrypoint.inMemory(package, lockFile, cache); + } + + // For uncached sources (i.e. path), the ID just points to the real + // directory for the package. + assert(id.source == "path"); + return new Entrypoint(PathSource.pathFromDescription(id.description), + cache); } /// Runs [package]'s [executable] with [args]. diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index 593f9935..f1649d1c 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -134,9 +134,8 @@ class HostedSource extends CachedSource { var url = _directoryToUrl(path.basename(serverDir)); var packages = _getCachedPackagesInDirectory(path.basename(serverDir)); packages.sort(Package.orderByNameAndVersion); - // TODO(nweiz): Use a normal for loop here when - // https://github.com/dart-lang/async_await/issues/72 is fixed. - await Future.forEach(packages, (package) async { + + for (var package in packages) { try { await _download(url, package.name, package.version, package.dir); successes++; @@ -150,7 +149,7 @@ class HostedSource extends CachedSource { tryDeleteEntry(package.dir); } - }); + } } return new Pair(successes, failures); diff --git a/lib/src/validator/dependency.dart b/lib/src/validator/dependency.dart index 7e6a933b..37f55d79 100644 --- a/lib/src/validator/dependency.dart +++ b/lib/src/validator/dependency.dart @@ -37,10 +37,7 @@ class DependencyValidator extends Validator { Future validate() async { var caretDeps = []; - // TODO(nweiz): Replace this with a real for/in loop when we update - // async_await. - await Future.forEach(entrypoint.root.pubspec.dependencies, - (dependency) async { + for (var dependency in entrypoint.root.pubspec.dependencies) { if (dependency.source != "hosted") { await _warnAboutSource(dependency); } else if (dependency.constraint.isAny) { @@ -58,7 +55,7 @@ class DependencyValidator extends Validator { caretDeps.add(dependency); } } - }); + } if (caretDeps.isNotEmpty && !_caretAllowed) { _errorAboutCaretConstraints(caretDeps); -- GitLab