Fix a bug when finding a package for a generated file

The recent change in `findFile` that uses the `bazel-bin`
symlinks was not supported by `findPackageFor` since the `blazeBins`
list would not necessarily contain the symlink path (we only added it
if there were no other paths). The user visible symptom of this was
that we would suggest importing packages with `package:blaze-bin.`
prefix.

I believe that we can simply always add the symlink as the last element
of `binPaths`, so that we recover the property one of its elements
corresponds to the generated file that we have found.

Bug: http://b/193636339
Change-Id: I818379b4770a477cbcb6b147a505f1f3c012bb6c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213046
Commit-Queue: Michal Terepeta <michalt@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Michal Terepeta 2021-09-14 17:53:01 +00:00 committed by commit-bot@chromium.org
parent 3860f4bf06
commit 5da6490b1b
2 changed files with 32 additions and 10 deletions

View file

@ -461,7 +461,7 @@ class BazelWorkspace extends Workspace
var binPaths = _findBinFolderPaths(folder);
String symlinkPrefix =
_findSymlinkPrefix(provider, root, binPaths: binPaths);
binPaths ??= [context.join(root, '$symlinkPrefix-bin')];
binPaths = binPaths..add(context.join(root, '$symlinkPrefix-bin'));
return BazelWorkspace._(provider, root, symlinkPrefix, readonlyRoot,
binPaths, context.join(root, '$symlinkPrefix-genfiles'),
lookForBuildFileSubstitutes: lookForBuildFileSubstitutes);
@ -474,7 +474,7 @@ class BazelWorkspace extends Workspace
var binPaths = _findBinFolderPaths(parent);
String symlinkPrefix =
_findSymlinkPrefix(provider, root, binPaths: binPaths);
binPaths ??= [context.join(root, '$symlinkPrefix-bin')];
binPaths = binPaths..add(context.join(root, '$symlinkPrefix-bin'));
return BazelWorkspace._(
provider,
root,
@ -491,7 +491,7 @@ class BazelWorkspace extends Workspace
var binPaths = _findBinFolderPaths(folder);
String symlinkPrefix =
_findSymlinkPrefix(provider, root, binPaths: binPaths);
binPaths ??= [context.join(root, '$symlinkPrefix-bin')];
binPaths = binPaths..add(context.join(root, '$symlinkPrefix-bin'));
return BazelWorkspace._(
provider,
root,
@ -516,11 +516,12 @@ class BazelWorkspace extends Workspace
/// the immediate folders found in `$root/blaze-out/` and `$root/bazel-out/`
/// for folders named "bin".
///
/// If no "bin" folder is found in any of those locations, `null` is returned.
static List<String>? _findBinFolderPaths(Folder root) {
/// If no "bin" folder is found in any of those locations, empty list is
/// returned.
static List<String> _findBinFolderPaths(Folder root) {
var out = _firstExistingFolder(root, ['blaze-out', 'bazel-out']);
if (out == null) {
return null;
return [];
}
List<String> binPaths = [];
@ -532,7 +533,7 @@ class BazelWorkspace extends Workspace
binPaths.add(possibleBin.path);
}
}
return binPaths.isEmpty ? null : binPaths;
return binPaths;
}
/// Return the symlink prefix, _X_, for folders `X-bin` or `X-genfiles`.

View file

@ -658,6 +658,26 @@ class BazelWorkspacePackageTest with ResourceProviderMixin {
expect(package?.workspace, equals(workspace));
}
void test_findPackageFor_generatedFileInBlazeOutAndBin() {
_addResources([
'/ws/blaze-out/host/bin/some/code/code.packages',
'/ws/blaze-out/host/bin/some/code/code.dart',
'/ws/blaze-bin/some/code/code.dart',
]);
workspace = BazelWorkspace.find(
resourceProvider,
convertPath('/ws/some/code/testing'),
)!;
// Make sure that we can find the package of the generated file.
var file = workspace.findFile(convertPath('/ws/some/code/code.dart'));
package = workspace.findPackageFor(file!.path);
expect(package, isNotNull);
expect(package?.root, convertPath('/ws/some/code'));
expect(package?.workspace, equals(workspace));
}
void test_findPackageFor_inBlazeOut_notPackage() {
var path =
convertPath('/ws/blaze-out/k8-opt/bin/news/lib/news_base.pb.dart');
@ -858,8 +878,8 @@ class BazelWorkspaceTest with ResourceProviderMixin {
resourceProvider, convertPath('/workspace/my/module'))!;
expect(workspace.root, convertPath('/workspace'));
expect(workspace.readonly, isNull);
expect(workspace.binPaths.single,
convertPath('/workspace/blaze-out/host/bin'));
expect(
workspace.binPaths.first, convertPath('/workspace/blaze-out/host/bin'));
expect(workspace.genfiles, convertPath('/workspace/blaze-genfiles'));
expect(
workspace
@ -893,11 +913,12 @@ class BazelWorkspaceTest with ResourceProviderMixin {
resourceProvider, convertPath('/workspace/my/module'))!;
expect(workspace.root, convertPath('/workspace'));
expect(workspace.readonly, isNull);
expect(workspace.binPaths, hasLength(2));
expect(workspace.binPaths, hasLength(3));
expect(workspace.binPaths,
contains(convertPath('/workspace/blaze-out/host/bin')));
expect(workspace.binPaths,
contains(convertPath('/workspace/blaze-out/k8-fastbuild/bin')));
expect(workspace.binPaths, contains(convertPath('/workspace/blaze-bin')));
expect(workspace.genfiles, convertPath('/workspace/blaze-genfiles'));
}