From ac47d0a1fafb26ccc627876a51bf462c5db94b9a Mon Sep 17 00:00:00 2001 From: Jacob MacDonald <jakemac@google.com> Date: Thu, 4 May 2017 13:56:54 -0700 Subject: [PATCH] Some windows fixes (#1585) Also delete temp_environment which snuck back in during a bad merge --- .../dartdevc/linked_summary_transformer.dart | 23 +++--- lib/src/barback/dartdevc/scratch_space.dart | 4 +- .../barback/dartdevc/temp_environment.dart | 65 ----------------- lib/src/barback/dartdevc/workers.dart | 9 ++- lib/src/io.dart | 2 +- .../dartdevc/temp_environment_test.dart | 70 ------------------- 6 files changed, 22 insertions(+), 151 deletions(-) delete mode 100644 lib/src/barback/dartdevc/temp_environment.dart delete mode 100644 test/barback/dartdevc/temp_environment_test.dart diff --git a/lib/src/barback/dartdevc/linked_summary_transformer.dart b/lib/src/barback/dartdevc/linked_summary_transformer.dart index 4ba00bf4..32798227 100644 --- a/lib/src/barback/dartdevc/linked_summary_transformer.dart +++ b/lib/src/barback/dartdevc/linked_summary_transformer.dart @@ -7,10 +7,10 @@ import 'dart:async'; import 'package:barback/barback.dart'; import 'package:bazel_worker/bazel_worker.dart'; -import 'workers.dart'; import 'module.dart'; import 'module_reader.dart'; -import 'temp_environment.dart'; +import 'scratch_space.dart'; +import 'workers.dart'; final String linkedSummaryExtension = '.linked.sum'; @@ -30,7 +30,7 @@ class LinkedSummaryTransformer extends Transformer { var reader = new ModuleReader(transform.readInputAsString); var configId = transform.primaryInput.id; var modules = await reader.readModules(configId); - TempEnvironment tempEnv; + ScratchSpace scratchSpace; try { var allAssetIds = new Set<AssetId>(); var summariesForModule = <ModuleId, Set<AssetId>>{}; @@ -43,11 +43,12 @@ class LinkedSummaryTransformer extends Transformer { allAssetIds..addAll(module.assetIds)..addAll(unlinkedSummaryIds); } // Create a single temp environment for all the modules in this package. - tempEnv = await TempEnvironment.create(allAssetIds, transform.readInput); + scratchSpace = + await ScratchSpace.create(allAssetIds, transform.readInput); await Future.wait(modules.map((m) => _createLinkedSummaryForModule( - m, summariesForModule[m.id], tempEnv, transform))); + m, summariesForModule[m.id], scratchSpace, transform))); } finally { - tempEnv?.delete(); + scratchSpace?.delete(); } } } @@ -55,9 +56,9 @@ class LinkedSummaryTransformer extends Transformer { Future _createLinkedSummaryForModule( Module module, Set<AssetId> unlinkedSummaryIds, - TempEnvironment tempEnv, + ScratchSpace scratchSpace, Transform transform) async { - var summaryOutputFile = tempEnv.fileFor(module.id.linkedSummaryId); + var summaryOutputFile = scratchSpace.fileFor(module.id.linkedSummaryId); var request = new WorkRequest(); // TODO(jakemac53): Diet parsing results in erroneous errors in later steps, // but ideally we would do that (pass '--build-summary-only-diet'). @@ -67,15 +68,15 @@ Future _createLinkedSummaryForModule( '--strong', ]); // Add all the unlinked summaries as build summary inputs. - request.arguments.addAll(unlinkedSummaryIds.map( - (id) => '--build-summary-unlinked-input=${tempEnv.fileFor(id).path}')); + request.arguments.addAll(unlinkedSummaryIds.map((id) => + '--build-summary-unlinked-input=${scratchSpace.fileFor(id).path}')); // Add all the files to include in the linked summary bundle. request.arguments.addAll(module.assetIds.map((id) { var uri = canonicalUriFor(id); if (!uri.startsWith('package:')) { uri = 'file://$uri'; } - return '$uri|${tempEnv.fileFor(id).path}'; + return '$uri|${scratchSpace.fileFor(id).path}'; })); var response = await analyzerDriver.doWork(request); if (response.exitCode == EXIT_CODE_ERROR) { diff --git a/lib/src/barback/dartdevc/scratch_space.dart b/lib/src/barback/dartdevc/scratch_space.dart index bedc7333..2c039866 100644 --- a/lib/src/barback/dartdevc/scratch_space.dart +++ b/lib/src/barback/dartdevc/scratch_space.dart @@ -54,7 +54,9 @@ class ScratchSpace { /// otherwise it just returns [id.path]. String canonicalUriFor(AssetId id) { if (topLevelDir(id.path) == 'lib') { - return 'package:${p.join(id.package, p.joinAll(p.split(id.path).skip(1)))}'; + var packagePath = + p.url.join(id.package, p.url.joinAll(p.url.split(id.path).skip(1))); + return 'package:$packagePath'; } else { return id.path; } diff --git a/lib/src/barback/dartdevc/temp_environment.dart b/lib/src/barback/dartdevc/temp_environment.dart deleted file mode 100644 index 62dccd41..00000000 --- a/lib/src/barback/dartdevc/temp_environment.dart +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) 2017, 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 'dart:async'; -import 'dart:io'; - -import 'package:barback/barback.dart'; -import 'package:path/path.dart' as p; - -import '../../io.dart'; - -/// An on-disk temporary environment for running executables that don't have -/// a standard dart library api. -class TempEnvironment { - final Directory tempDir; - final Directory packagesDir; - - TempEnvironment._(Directory tempDir) - : packagesDir = new Directory(p.join(tempDir.path, 'packages')), - this.tempDir = tempDir; - - /// Creates a new [TempEnvironment] containing [assetIds]. - /// - /// Any [Asset] that is under a `lib` dir will be output under a `packages` - /// directory corresponding to it's package, and any other assets are output - /// directly under the temp dir using their unmodified path. - static Future<TempEnvironment> create( - Iterable<AssetId> assetIds, Stream<List<int>> readAsset(AssetId)) async { - var tempDir = new Directory(createSystemTempDir()); - var futures = <Future>[]; - for (var id in assetIds) { - var filePath = p.join(tempDir.path, _relativePathFor(id)); - ensureDir(p.dirname(filePath)); - futures.add(createFileFromStream(readAsset(id), filePath)); - } - await Future.wait(futures); - return new TempEnvironment._(tempDir); - } - - /// Deletes the temp directory for this environment. - Future delete() => tempDir.delete(recursive: true); - - /// Returns the actual [File] in this environment corresponding to [id]. - /// - /// The returned [File] may or may not already exist. - File fileFor(AssetId id) => - new File(p.join(tempDir.path, _relativePathFor(id))); -} - -/// Returns a canonical uri for [id]. -/// -/// If [id] is under a `lib` directory then this returns a `package:` uri, -/// otherwise it just returns [id.path]. -String canonicalUriFor(AssetId id) { - if (topLevelDir(id.path) == 'lib') { - return 'package:${p.join(id.package, p.joinAll(p.split(id.path).skip(1)))}'; - } else { - return id.path; - } -} - -/// The path relative to the root of the environment for a given [id]. -String _relativePathFor(AssetId id) => - canonicalUriFor(id).replaceFirst('package:', 'packages/'); diff --git a/lib/src/barback/dartdevc/workers.dart b/lib/src/barback/dartdevc/workers.dart index 587f10a7..6b1c061e 100644 --- a/lib/src/barback/dartdevc/workers.dart +++ b/lib/src/barback/dartdevc/workers.dart @@ -8,13 +8,16 @@ import 'package:bazel_worker/driver.dart'; import 'package:cli_util/cli_util.dart' as cli_util; import 'package:path/path.dart' as p; +String get _scriptExtension => Platform.isWindows ? '.bat' : ''; + /// Manages a shared set of persistent analyzer workers. final analyzerDriver = new BazelWorkerDriver(() => Process.start( - p.join(sdkDir.path, 'bin', 'dartanalyzer'), + p.join(sdkDir.path, 'bin', 'dartanalyzer$_scriptExtension'), ['--build-mode', '--persistent_worker'])); /// Manages a shared set of persistent dartdevc workers. -final dartdevcDriver = new BazelWorkerDriver(() => Process - .start(p.join(sdkDir.path, 'bin', 'dartdevc'), ['--persistent_worker'])); +final dartdevcDriver = new BazelWorkerDriver(() => Process.start( + p.join(sdkDir.path, 'bin', 'dartdevc$_scriptExtension'), + ['--persistent_worker'])); final sdkDir = cli_util.getSdkDir(); diff --git a/lib/src/io.dart b/lib/src/io.dart index 3b171123..b1f3b0bf 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -1137,7 +1137,7 @@ class PubProcessResult { /// /// Throws an [ArgumentError] if [uri] is just a filename with no directory. String topLevelDir(String uri) { - var parts = path.url.split(path.normalize(uri)); + var parts = path.url.split(path.url.normalize(uri)); String error; if (parts.length == 1) { error = 'The uri `$uri` does not contain a directory.'; diff --git a/test/barback/dartdevc/temp_environment_test.dart b/test/barback/dartdevc/temp_environment_test.dart deleted file mode 100644 index 0200adcf..00000000 --- a/test/barback/dartdevc/temp_environment_test.dart +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2017, 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 'dart:async'; -import 'dart:io'; - -import 'package:barback/barback.dart'; -import 'package:path/path.dart' as p; -import 'package:test/test.dart'; - -import 'package:pub/src/barback/dartdevc/temp_environment.dart'; -import 'package:pub/src/io.dart'; - -void main() { - test('Can create and delete a temp environment', () async { - Map<AssetId, List<int>> allAssets = [ - 'dep|lib/dep.dart', - 'myapp|lib/myapp.dart', - 'myapp|web/main.dart', - ].fold({}, (assets, serializedId) { - assets[new AssetId.parse(serializedId)] = serializedId.codeUnits; - return assets; - }); - - var tempEnv = await TempEnvironment.create( - allAssets.keys, (id) => new Stream.fromIterable([allAssets[id]])); - - expect(p.isWithin(Directory.systemTemp.path, tempEnv.tempDir.path), isTrue); - - for (var id in allAssets.keys) { - var file = tempEnv.fileFor(id); - expect(file.existsSync(), isTrue); - expect(file.readAsStringSync(), equals('$id')); - - var relativeFilePath = p.relative(file.path, from: tempEnv.tempDir.path); - if (topLevelDir(id.path) == 'lib') { - var packagesPath = - p.join('packages', id.package, p.relative(id.path, from: 'lib')); - expect(relativeFilePath, equals(packagesPath)); - } else { - expect(relativeFilePath, equals(id.path)); - } - } - - await tempEnv.delete(); - - for (var id in allAssets.keys) { - var file = tempEnv.fileFor(id); - expect(file.existsSync(), isFalse); - } - expect(tempEnv.tempDir.existsSync(), isFalse); - }); - - test('canonicalUriFor', () { - expect(canonicalUriFor(new AssetId('a', 'lib/a.dart')), - equals('package:a/a.dart')); - expect(canonicalUriFor(new AssetId('a', 'lib/src/a.dart')), - equals('package:a/src/a.dart')); - expect( - canonicalUriFor(new AssetId('a', 'web/a.dart')), equals('web/a.dart')); - - expect( - () => canonicalUriFor(new AssetId('a', 'a.dart')), throwsArgumentError); - expect(() => canonicalUriFor(new AssetId('a', 'lib/../a.dart')), - throwsArgumentError); - expect(() => canonicalUriFor(new AssetId('a', 'web/../a.dart')), - throwsArgumentError); - }); -} -- GitLab