From fcd407b0cf78472ae64e092eca9a56f322448cea Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 4 Jun 2015 15:55:13 -0700 Subject: [PATCH] Revert "Support Package Resolution Configuration files." This reverts commit 36c29d05bcc1dfe43a2f95698df5b41857f93221. BUG= Review URL: https://codereview.chromium.org//1165943005 --- DEPS | 2 +- pkg/compiler/lib/compiler.dart | 17 +--- pkg/compiler/lib/src/apiimpl.dart | 91 +++++-------------- pkg/compiler/lib/src/dart2js.dart | 28 ++---- pkg/compiler/lib/src/mirrors/analyze.dart | 11 +-- .../lib/caching_compiler.dart | 4 +- .../compiler/dart2js/bad_output_io_test.dart | 3 +- tests/compiler/dart2js/exit_code_test.dart | 10 +- .../dart2js/expect_annotations_test.dart | 2 +- .../compiler/dart2js/import_mirrors_test.dart | 32 +++---- tests/compiler/dart2js/memory_compiler.dart | 2 +- 11 files changed, 60 insertions(+), 142 deletions(-) diff --git a/DEPS b/DEPS index 256a53ee0b39..1b10899b847a 100644 --- a/DEPS +++ b/DEPS @@ -76,7 +76,7 @@ vars = { "oauth2_rev": "@1bff41f4d54505c36f2d1a001b83b8b745c452f5", "observe_rev": "@eee2b8ec34236fa46982575fbccff84f61202ac6", "observatory_pub_packages_rev": "@45565", - "package_config_tag": "@0.0.3+1", + "package_config_tag": "@0.0.2+4", "path_rev": "@93b3e2aa1db0ac0c8bab9d341588d77acda60320", "petitparser_rev" : "@37878", "ply_rev": "@604b32590ffad5cbb82e4afef1d305512d06ae93", diff --git a/pkg/compiler/lib/compiler.dart b/pkg/compiler/lib/compiler.dart index 710ad444d61a..3db762d3e780 100644 --- a/pkg/compiler/lib/compiler.dart +++ b/pkg/compiler/lib/compiler.dart @@ -5,7 +5,6 @@ library compiler; import 'dart:async'; -import 'package:package_config/packages.dart'; import 'src/apiimpl.dart'; // Unless explicitly allowed, passing [:null:] for any argument to the @@ -59,19 +58,13 @@ typedef EventSink CompilerOutputProvider(String name, * [:null:]. If [uri] is not [:null:], neither are [begin] and * [end]. [uri] indicates the compilation unit from where the * diagnostic originates. [begin] and [end] are zero-based character - * offsets from the beginning of the compilation unit. [message] is the + * offsets from the beginning of the compilaton unit. [message] is the * diagnostic message, and [kind] indicates indicates what kind of * diagnostic it is. */ typedef void DiagnosticHandler(Uri uri, int begin, int end, String message, Diagnostic kind); -/** - * Provides a package lookup mechanism in the case that no package root or - * package resolution configuration file are explicitly specified. - */ -typedef Future PackagesDiscoveryProvider(Uri uri); - /// Information resulting from the compilation. class CompilationResult { /// `true` if the compilation succeeded, that is, compilation didn't fail due @@ -110,9 +103,7 @@ Future compile( DiagnosticHandler handler, [List options = const [], CompilerOutputProvider outputProvider, - Map environment = const {}, - Uri packageConfig, - PackagesDiscoveryProvider packagesDiscoveryProvider]) { + Map environment = const {}]) { if (!libraryRoot.path.endsWith("/")) { throw new ArgumentError("libraryRoot must end with a /"); } @@ -127,9 +118,7 @@ Future compile( libraryRoot, packageRoot, options, - environment, - packageConfig, - packagesDiscoveryProvider); + environment); return compiler.run(script).then((bool success) { return new CompilationResult(compiler, isSuccess: success); }); diff --git a/pkg/compiler/lib/src/apiimpl.dart b/pkg/compiler/lib/src/apiimpl.dart index 4b560733c8be..a9579f042a48 100644 --- a/pkg/compiler/lib/src/apiimpl.dart +++ b/pkg/compiler/lib/src/apiimpl.dart @@ -5,7 +5,6 @@ library leg_apiimpl; import 'dart:async'; -import 'dart:convert'; import '../compiler.dart' as api; import 'dart2jslib.dart' as leg; @@ -14,10 +13,6 @@ import 'elements/elements.dart' as elements; import 'package:_internal/libraries.dart' hide LIBRARIES; import 'package:_internal/libraries.dart' as library_info show LIBRARIES; import 'io/source_file.dart'; -import 'package:package_config/packages.dart'; -import 'package:package_config/packages_file.dart' as pkgs; -import 'package:package_config/src/packages_impl.dart' - show NonFilePackagesDirectoryPackages, MapPackages; const bool forceIncrementalSupport = const bool.fromEnvironment('DART2JS_EXPERIMENTAL_INCREMENTAL_SUPPORT'); @@ -26,10 +21,7 @@ class Compiler extends leg.Compiler { api.CompilerInputProvider provider; api.DiagnosticHandler handler; final Uri libraryRoot; - final Uri packageConfig; final Uri packageRoot; - final api.PackagesDiscoveryProvider packagesDiscoveryProvider; - Packages packages; List options; Map environment; bool mockableLibraryUsed = false; @@ -37,7 +29,6 @@ class Compiler extends leg.Compiler { leg.GenericTask userHandlerTask; leg.GenericTask userProviderTask; - leg.GenericTask userPackagesDiscoveryTask; Compiler(this.provider, api.CompilerOutputProvider outputProvider, @@ -45,9 +36,7 @@ class Compiler extends leg.Compiler { this.libraryRoot, this.packageRoot, List options, - this.environment, - [this.packageConfig, - this.packagesDiscoveryProvider]) + this.environment) : this.options = options, this.allowedLibraryCategories = getAllowedLibraryCategories(options), super( @@ -106,8 +95,6 @@ class Compiler extends leg.Compiler { tasks.addAll([ userHandlerTask = new leg.GenericTask('Diagnostic handler', this), userProviderTask = new leg.GenericTask('Input provider', this), - userPackagesDiscoveryTask = - new leg.GenericTask('Package discovery', this), ]); if (libraryRoot == null) { throw new ArgumentError("[libraryRoot] is null."); @@ -115,11 +102,10 @@ class Compiler extends leg.Compiler { if (!libraryRoot.path.endsWith("/")) { throw new ArgumentError("[libraryRoot] must end with a /."); } - if (packageRoot != null && packageConfig != null) { - throw new ArgumentError("Only one of [packageRoot] or [packageConfig] " - "may be given."); + if (packageRoot == null) { + throw new ArgumentError("[packageRoot] is null."); } - if (packageRoot != null && !packageRoot.path.endsWith("/")) { + if (!packageRoot.path.endsWith("/")) { throw new ArgumentError("[packageRoot] must end with a /."); } if (!analyzeOnly) { @@ -173,7 +159,8 @@ class Compiler extends leg.Compiler { // TODO(johnniwinther): Merge better with [translateDartUri] when // [scanBuiltinLibrary] is removed. - String lookupLibraryPath(LibraryInfo info) { + String lookupLibraryPath(String dartLibraryName) { + LibraryInfo info = lookupLibraryInfo(dartLibraryName); if (info == null) return null; if (!info.isDart2jsLibrary) return null; if (!allowedLibraryCategories.contains(info.category)) return null; @@ -297,7 +284,7 @@ class Compiler extends leg.Compiler { Uri translateDartUri(elements.LibraryElement importingLibrary, Uri resolvedUri, tree.Node node) { LibraryInfo libraryInfo = lookupLibraryInfo(resolvedUri.path); - String path = lookupLibraryPath(libraryInfo); + String path = lookupLibraryPath(resolvedUri.path); if (libraryInfo != null && libraryInfo.category == "Internal") { bool allowInternalLibraryAccess = false; @@ -345,49 +332,25 @@ class Compiler extends leg.Compiler { } Uri translatePackageUri(leg.Spannable node, Uri uri) { - return packages.resolve(uri); - } - - Future setupPackages(Uri uri) async { - if (packageRoot != null) { - // Use "non-file" packages because the file version requires a [Directory] - // and we can't depend on 'dart:io' classes. - packages = new NonFilePackagesDirectoryPackages(packageRoot); - } else if (packageConfig != null) { - var packageConfigContents = await provider(packageConfig); - if (packageConfigContents is String) { - packageConfigContents = UTF8.encode(packageConfigContents); - } - packages = - new MapPackages(pkgs.parse(packageConfigContents, packageConfig)); - } else { - if (packagesDiscoveryProvider == null) { - packages = Packages.noPackages; - } else { - packages = await callUserPackagesDiscovery(uri); - } - } + return packageRoot.resolve(uri.path); } - Future run(Uri uri) async { + Future run(Uri uri) { log('Allowed library categories: $allowedLibraryCategories'); - - await setupPackages(uri); - assert(packages != null); - - bool success = await super.run(uri); - int cumulated = 0; - for (final task in tasks) { - int elapsed = task.timing; - if (elapsed != 0) { - cumulated += elapsed; - log('${task.name} took ${elapsed}msec'); + return super.run(uri).then((bool success) { + int cumulated = 0; + for (final task in tasks) { + int elapsed = task.timing; + if (elapsed != 0) { + cumulated += elapsed; + log('${task.name} took ${elapsed}msec'); + } } - } - int total = totalCompileTime.elapsedMilliseconds; - log('Total compile-time ${total}msec;' - ' unaccounted ${total - cumulated}msec'); - return success; + int total = totalCompileTime.elapsedMilliseconds; + log('Total compile-time ${total}msec;' + ' unaccounted ${total - cumulated}msec'); + return success; + }); } void reportDiagnostic(leg.Spannable node, @@ -436,16 +399,6 @@ class Compiler extends leg.Compiler { } } - Future callUserPackagesDiscovery(Uri uri) { - try { - return userPackagesDiscoveryTask.measure( - () => packagesDiscoveryProvider(uri)); - } catch (ex, s) { - diagnoseCrashInUserCode('Uncaught exception in package discovery', ex, s); - rethrow; - } - } - void diagnoseCrashInUserCode(String message, exception, stackTrace) { hasCrashed = true; print('$message: ${tryToString(exception)}'); diff --git a/pkg/compiler/lib/src/dart2js.dart b/pkg/compiler/lib/src/dart2js.dart index 098e09ccfbdf..3f92322cc26f 100644 --- a/pkg/compiler/lib/src/dart2js.dart +++ b/pkg/compiler/lib/src/dart2js.dart @@ -19,7 +19,6 @@ import 'util/uri_extras.dart'; import 'util/util.dart' show stackTraceFilePrefix; import 'util/command_line.dart'; import 'package:_internal/libraries.dart'; -import 'package:package_config/discovery.dart' show findPackages; const String LIBRARY_ROOT = '../../../../../sdk'; const String OUTPUT_LANGUAGE_DART = 'Dart'; @@ -106,7 +105,6 @@ Future compile(List argv) { Uri libraryRoot = currentDirectory; Uri out = currentDirectory.resolve('out.js'); Uri sourceMapOut = currentDirectory.resolve('out.js.map'); - Uri packageConfig = null; Uri packageRoot = null; List options = new List(); bool explicitOut = false; @@ -142,10 +140,6 @@ Future compile(List argv) { packageRoot = currentDirectory.resolve(extractPath(argument)); } - setPackageConfig(String argument) { - packageConfig = currentDirectory.resolve(extractPath(argument)); - } - setOutput(Iterator arguments) { optionsImplyCompilation.add(arguments.current); String path; @@ -335,7 +329,6 @@ Future compile(List argv) { (_) => setTrustPrimitives( '--trust-primitives')), new OptionHandler(r'--help|/\?|/h', (_) => wantHelp = true), - new OptionHandler('--packages=.+', setPackageConfig), new OptionHandler('--package-root=.+|-p.+', setPackageRoot), new OptionHandler('--analyze-all', setAnalyzeAll), new OptionHandler('--analyze-only', setAnalyzeOnly), @@ -410,8 +403,9 @@ Future compile(List argv) { "checked mode."); } - if (packageRoot != null && packageConfig != null) { - helpAndFail("Cannot specify both '--package-root' and '--packages."); + Uri uri = currentDirectory.resolve(arguments[0]); + if (packageRoot == null) { + packageRoot = uri.resolve('./packages/'); } if ((analyzeOnly || analyzeAll) && !optionsImplyCompilation.isEmpty) { @@ -437,6 +431,8 @@ Future compile(List argv) { "combination with the '--output-type=dart' option."); } + diagnosticHandler.info('Package root is $packageRoot'); + options.add('--out=$out'); options.add('--source-map=$sourceMapOut'); @@ -471,10 +467,9 @@ Future compile(List argv) { return result; } - Uri uri = currentDirectory.resolve(arguments[0]); - return compileFunc(uri, libraryRoot, packageRoot, inputProvider, - diagnosticHandler, options, outputProvider, environment, - packageConfig, findPackages) + return compileFunc(uri, libraryRoot, packageRoot, + inputProvider, diagnosticHandler, + options, outputProvider, environment) .then(compilationDone); } @@ -556,12 +551,7 @@ Supported options: Display version information. -p, --package-root= - Where to find packages, that is, "package:..." imports. This option cannot - be used with --packages. - - --packages= - Path to the package resolution configuration file, which supplies a mapping - of package names to paths. This option cannot be used with --package-root. + Where to find packages, that is, "package:..." imports. --analyze-all Analyze all code. Without this option, the compiler only analyzes diff --git a/pkg/compiler/lib/src/mirrors/analyze.dart b/pkg/compiler/lib/src/mirrors/analyze.dart index 8932fff4d8e7..44bd859ed234 100644 --- a/pkg/compiler/lib/src/mirrors/analyze.dart +++ b/pkg/compiler/lib/src/mirrors/analyze.dart @@ -26,9 +26,7 @@ Future analyze(List libraries, Uri packageRoot, api.CompilerInputProvider inputProvider, api.DiagnosticHandler diagnosticHandler, - [List options = const [], - Uri packageConfig, - api.PackagesDiscoveryProvider findPackages]) { + [List options = const []]) { if (!libraryRoot.path.endsWith("/")) { throw new ArgumentError("libraryRoot must end with a /"); } @@ -56,12 +54,9 @@ Future analyze(List libraries, Compiler compiler = new apiimpl.Compiler(inputProvider, null, internalDiagnosticHandler, - libraryRoot, - packageRoot, + libraryRoot, packageRoot, options, - const {}, - packageConfig, - findPackages); + const {}); compiler.librariesToAnalyzeWhenRun = libraries; return compiler.run(null).then((bool success) { if (success && !compilationFailed) { diff --git a/pkg/dart2js_incremental/lib/caching_compiler.dart b/pkg/dart2js_incremental/lib/caching_compiler.dart index 0050ab069ace..06445cbec200 100644 --- a/pkg/dart2js_incremental/lib/caching_compiler.dart +++ b/pkg/dart2js_incremental/lib/caching_compiler.dart @@ -60,9 +60,7 @@ Future reuseCompiler( libraryRoot, packageRoot, options, - environment, - null, - null); + environment); JavaScriptBackend backend = compiler.backend; // Much like a scout, an incremental compiler is always prepared. For diff --git a/tests/compiler/dart2js/bad_output_io_test.dart b/tests/compiler/dart2js/bad_output_io_test.dart index d2429f03b78e..868d53cf28e9 100644 --- a/tests/compiler/dart2js/bad_output_io_test.dart +++ b/tests/compiler/dart2js/bad_output_io_test.dart @@ -55,8 +55,7 @@ class CollectingFormattingDiagnosticHandler } testOutputProvider(script, libraryRoot, packageRoot, inputProvider, handler, - [options, outputProvider, environment, packageConfig, - findPackages]) { + [options, outputProvider, environment]) { diagnosticHandler = new CollectingFormattingDiagnosticHandler(); outputProvider("/non/existing/directory/should/fail/file", "js"); } diff --git a/tests/compiler/dart2js/exit_code_test.dart b/tests/compiler/dart2js/exit_code_test.dart index f69a271fabf0..82473cb81cc0 100644 --- a/tests/compiler/dart2js/exit_code_test.dart +++ b/tests/compiler/dart2js/exit_code_test.dart @@ -34,13 +34,11 @@ class TestCompiler extends apiimpl.Compiler { Uri packageRoot, List options, Map environment, - Uri packageConfig, - api.PackagesDiscoveryProvider findPackages, String this.testMarker, String this.testType, Function this.onTest) : super(inputProvider, outputProvider, handler, libraryRoot, - packageRoot, options, environment, packageConfig, findPackages) { + packageRoot, options, environment) { scanner = new TestScanner(this); resolver = new TestResolver(this, backend.constantCompilerTask); test('Compiler'); @@ -158,9 +156,7 @@ Future testExitCode( api.DiagnosticHandler handler, [List options = const [], api.CompilerOutputProvider outputProvider, - Map environment = const {}, - Uri packageConfig, - api.PackagesDiscoveryProvider findPackages]) { + Map environment = const {}]) { libraryRoot = Platform.script.resolve('../../../sdk/'); outputProvider = NullSink.outputProvider; // Use this to silence the test when debugging: @@ -172,8 +168,6 @@ Future testExitCode( packageRoot, options, environment, - packageConfig, - findPackages, marker, type, onTest); diff --git a/tests/compiler/dart2js/expect_annotations_test.dart b/tests/compiler/dart2js/expect_annotations_test.dart index 8613107bef1a..6669d32bf1aa 100644 --- a/tests/compiler/dart2js/expect_annotations_test.dart +++ b/tests/compiler/dart2js/expect_annotations_test.dart @@ -49,7 +49,7 @@ void main(List args) { main() { Compiler compiler = compilerFor(MEMORY_SOURCE_FILES); - asyncTest(() => compiler.run(Uri.parse('memory:main.dart')).then((_) { + asyncTest(() => compiler.runCompiler(Uri.parse('memory:main.dart')).then((_) { Expect.isFalse(compiler.compilationFailed, 'Unsuccessful compilation'); JavaScriptBackend backend = compiler.backend; Expect.isNotNull(backend.annotations.expectNoInlineClass, diff --git a/tests/compiler/dart2js/import_mirrors_test.dart b/tests/compiler/dart2js/import_mirrors_test.dart index f5f799be74a3..cbe044bb33a9 100644 --- a/tests/compiler/dart2js/import_mirrors_test.dart +++ b/tests/compiler/dart2js/import_mirrors_test.dart @@ -66,16 +66,16 @@ import 'first.dart'; main() {} ''', '/first.dart': ''' -import 'package:second/second.dart'; +import 'package:second.dart'; ''', - '/pkg/second/second.dart': ''' + '/pkg/second.dart': ''' import 'dart:mirrors'; ''', 'paths': - "first.dart => package:second => dart:mirrors", + "first.dart => package:second.dart => dart:mirrors", 'verbosePaths': - "main.dart => first.dart => package:second/second.dart => dart:mirrors", + "main.dart => first.dart => package:second.dart => dart:mirrors", }; const INDIRECT_PACKAGE_IMPORT2 = const { @@ -85,16 +85,16 @@ import 'first.dart'; main() {} ''', '/first.dart': ''' -import 'package:packagename/second.dart'; +import 'package:package-name/second.dart'; ''', - '/pkg/packagename/second.dart': ''' + '/pkg/package-name/second.dart': ''' import 'dart:mirrors'; ''', 'paths': - "first.dart => package:packagename => dart:mirrors", + "first.dart => package:package-name => dart:mirrors", 'verbosePaths': - "main.dart => first.dart => package:packagename/second.dart " + "main.dart => first.dart => package:package-name/second.dart " "=> dart:mirrors", }; @@ -306,16 +306,16 @@ import 'first.dart'; main() {} ''', '/first.dart': ''' -import 'package:packagename/second.dart'; +import 'package:package-name/second.dart'; ''', - '/pkg/packagename/second.dart': ''' + '/pkg/package-name/second.dart': ''' export 'dart:mirrors'; ''', 'paths': - "first.dart => package:packagename => dart:mirrors", + "first.dart => package:package-name => dart:mirrors", 'verbosePaths': - "main.dart => first.dart => package:packagename/second.dart " + "main.dart => first.dart => package:package-name/second.dart " "=> dart:mirrors", }; @@ -326,16 +326,16 @@ import 'first.dart'; main() {} ''', '/first.dart': ''' -export 'package:packagename/second.dart'; +export 'package:package-name/second.dart'; ''', - '/pkg/packagename/second.dart': ''' + '/pkg/package-name/second.dart': ''' import 'dart:mirrors'; ''', 'paths': - "first.dart => package:packagename => dart:mirrors", + "first.dart => package:package-name => dart:mirrors", 'verbosePaths': - "main.dart => first.dart => package:packagename/second.dart " + "main.dart => first.dart => package:package-name/second.dart " "=> dart:mirrors", }; diff --git a/tests/compiler/dart2js/memory_compiler.dart b/tests/compiler/dart2js/memory_compiler.dart index b9de4fea6585..482bb7c78520 100644 --- a/tests/compiler/dart2js/memory_compiler.dart +++ b/tests/compiler/dart2js/memory_compiler.dart @@ -110,7 +110,7 @@ Compiler compilerFor(Map memorySourceFiles, expando[readStringFromUri] = provider; } else { // When using a cached compiler, it has read a number of files from disk - // already (and will not attempt to read them again due to caching). These + // already (and will not attemp to read them again due to caching). These // files must be available to the new diagnostic handler. provider = expando[cachedCompiler.provider]; readStringFromUri = cachedCompiler.provider;