fix listFiles and relative for precompiled package dependencies (#1588)

Author: jakemac53

lib/src/cached_package.dart

@@ -44,7 +44,9 @@ class CachedPackage extends Package {
   }
 
   String relative(String path) {
-    if (p.isWithin(path, _cacheDir)) return p.relative(path, from: _cacheDir);
+    if (p.isWithin(_cacheDir, path)) {
+      return p.relative(path, from: _cacheDir);
+    }
     return super.relative(path);
   }
 
@@ -56,14 +58,18 @@ class CachedPackage extends Package {
       return super.listFiles(recursive: recursive, useGitIgnore: useGitIgnore);
     }
 
-    if (_pathInCache(beneath)) return listDir(p.join(_cacheDir, beneath));
+    if (_pathInCache(beneath)) {
+      return listDir(p.join(_cacheDir, beneath),
+          includeDirs: false, recursive: recursive);
+    }
     return super.listFiles(
         beneath: beneath, recursive: recursive, useGitIgnore: useGitIgnore);
   }
 
   /// Returns whether [relativePath], a path relative to the package's root,
   /// is in a cached directory.
-  bool _pathInCache(String relativePath) => p.isWithin('lib', relativePath);
+  bool _pathInCache(String relativePath) =>
+      relativePath == 'lib' || p.isWithin('lib', relativePath);
 }
 
 /// A pubspec wrapper that reports no transformers.

test/cached_package_test.dart

@@ -0,0 +1,49 @@
+// 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 'package:path/path.dart' as p;
+import 'package:scheduled_test/scheduled_test.dart';
+
+import 'package:pub/src/cached_package.dart';
+import 'package:pub/src/package.dart';
+import 'package:pub/src/pubspec.dart';
+
+import 'descriptor.dart' as d;
+import 'test_pub.dart';
+
+main() {
+  // Regression test for https://github.com/dart-lang/pub/issues/1586.
+  integration('Can list the cached lib dir and compute relative paths', () {
+    d.dir('cache', [
+      d.dir('app', [
+        d.dir('lib', [
+          d.file('cached.txt', 'hello'),
+          d.file('original.txt', 'world'),
+        ]),
+      ]),
+    ]).create();
+    d.dir('app', [
+      d.dir('lib', [
+        d.file('original.txt'),
+      ])
+    ]).create();
+
+    var cachedPackage = new CachedPackage(
+        new Package(new Pubspec('a'), p.join(d.defaultRoot, 'app')),
+        p.join(d.defaultRoot, 'cache', 'app'));
+
+    schedule(() {
+      var paths = cachedPackage.listFiles(beneath: 'lib');
+      expect(
+          paths,
+          unorderedMatches([
+            endsWith('cached.txt'),
+            endsWith('original.txt'),
+          ]));
+      for (var path in paths) {
+        expect(cachedPackage.relative(path), startsWith('lib'));
+      }
+    });
+  });
+}

test/get/cache_transformed_dependency_test.dart

@@ -55,6 +55,44 @@ class HasInputTransformer extends Transformer {
 }
 """;
 
+const COPY_TRANSFORMER = """
+import 'dart:async';
+
+import 'package:barback/barback.dart';
+
+class CopyTransformer extends Transformer {
+  CopyTransformer.asPlugin();
+
+  bool isPrimary(AssetId id) => true;
+
+  Future apply(Transform transform) async {
+    transform.addOutput(new Asset.fromString(
+      transform.primaryInput.id.addExtension('.copy'),
+      await transform.primaryInput.readAsString()));
+  }
+}
+""";
+
+const LIST_INPUTS_TRANSFORMER = """
+import 'dart:async';
+
+import 'package:barback/barback.dart';
+
+class ListInputsTransformer extends AggregateTransformer {
+  ListInputsTransformer.asPlugin();
+
+  String classifyPrimary(AssetId id) => '';
+
+  Future apply(AggregateTransform transform) async {
+    var inputs = await transform.primaryInputs.toList();
+    var names = inputs.map((asset) => asset.id.toString()).toList();
+    names.sort();
+    transform.addOutput(new Asset.fromString(
+      new AssetId(transform.package, 'lib/inputs.txt'), names.join('\\n')));
+  }
+}
+""";
+
 main() {
   integration("caches a transformed dependency", () {
     servePackages((builder) {
@@ -414,6 +452,48 @@ main() {
     d.dir(appPath, [d.nothing(".pub/deps/debug/foo")]).validate();
   });
 
+  // Regression test for https://github.com/dart-lang/pub/issues/1586.
+  integration(
+      "AggregateTransformers can read generated inputs from cached packages",
+      () {
+    servePackages((builder) {
+      builder.serveRealPackage('barback');
+
+      builder.serve("foo", "1.2.3", deps: {
+        'barback': 'any'
+      }, pubspec: {
+        'transformers': ['foo/copy_transformer', 'foo/list_transformer'],
+      }, contents: [
+        d.dir("lib", [
+          d.file("hello.dart", "String get hello => 'hello';"),
+          d.file("copy_transformer.dart", COPY_TRANSFORMER),
+          d.file("list_transformer.dart", LIST_INPUTS_TRANSFORMER),
+        ])
+      ]);
+    });
+
+    d.appDir({"foo": "1.2.3"}).create();
+
+    pubGet(output: contains("Precompiled foo"));
+
+    d.dir(appPath, [
+      d.matcherFile(
+          ".pub/deps/debug/foo/lib/inputs.txt", contains('hello.dart.copy'))
+    ]).validate();
+
+    pubServe();
+    requestShouldSucceed(
+        "packages/foo/inputs.txt",
+        """
+foo|lib/copy_transformer.dart
+foo|lib/copy_transformer.dart.copy
+foo|lib/hello.dart
+foo|lib/hello.dart.copy
+foo|lib/list_transformer.dart
+foo|lib/list_transformer.dart.copy""");
+    endPubServe();
+  });
+
   group("with --no-precompile", () {
     integration("doesn't cache a transformed dependency", () {
       servePackages((builder) {