Skip to content

Commit

Permalink
Adding Starlark dependencies to the package //external
Browse files Browse the repository at this point in the history
This is an alternative approach to fix bazelbuild#14280. It adds transitive closure of Starlark dependencies to `//external` package when loading `WORKSPACE` file, so it can be processed in the same way as `BUILD` files during query execution. Comparing to the approach taken in bazelbuild#14497, this approach is less intrusive, but not able to distinguish the extension files needed by `//external:foo` and `//external:bar`, meaning `buildfiles(//external:foo)` returns the same result as `buildfiles(//external:*)`. However, this behavior is consistent with other packages. For example, `buildfiles(//foo:bar)` has the same result as `buildfiles(//foo:*)`.

Closes bazelbuild#14630.

PiperOrigin-RevId: 424092916
(cherry picked from commit a6c3f23)
  • Loading branch information
linzhp authored and brentleyjones committed Mar 7, 2022
1 parent 61cfa1d commit 52442ad
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ FailureDetail getFailureDetail() {
return null;
}

Builder setStarlarkFileDependencies(ImmutableList<Label> starlarkFileDependencies) {
public Builder setStarlarkFileDependencies(ImmutableList<Label> starlarkFileDependencies) {
this.starlarkFileDependencies = starlarkFileDependencies;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ private static ImmutableList<Label> transitiveClosureOfLabels(
return ImmutableList.copyOf(set);
}

private static void transitiveClosureOfLabelsRec(
public static void transitiveClosureOfLabelsRec(
Set<Label> set, ImmutableMap<String, Module> loads) {
for (Module m : loads.values()) {
BazelModuleContext ctx = BazelModuleContext.of(m);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -58,6 +59,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import net.starlark.java.eval.Module;
import net.starlark.java.eval.Mutability;
import net.starlark.java.eval.Starlark;
Expand Down Expand Up @@ -325,14 +327,21 @@ public SkyValue compute(SkyKey skyKey, Environment env)
directories.getWorkspace(),
directories.getLocalJavabase(),
starlarkSemantics);
Set<Label> starlarkFileDependencies;
if (prevValue != null) {
starlarkFileDependencies =
Sets.newLinkedHashSet(prevValue.getPackage().getStarlarkFileDependencies());
try {
parser.setParent(
prevValue.getPackage(), prevValue.getLoadedModules(), prevValue.getBindings());
} catch (NameConflictException e) {
throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT);
}
} else {
starlarkFileDependencies = Sets.newLinkedHashSet();
}
PackageFactory.transitiveClosureOfLabelsRec(starlarkFileDependencies, loadedModules);
builder.setStarlarkFileDependencies(ImmutableList.copyOf(starlarkFileDependencies));
// Execute the partial files that comprise this chunk.
for (StarlarkFile partialFile : chunk) {
parser.execute(partialFile, loadedModules, key);
Expand Down
89 changes: 89 additions & 0 deletions src/test/py/bazel/query_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,87 @@ def testSimpleQuery(self):
self._AssertQueryOutput('deps(//foo:top-rule, 1)', '//foo:top-rule',
'//foo:dep-rule')

def testBuildFilesForExternalRepos_Simple(self):
self.ScratchFile('WORKSPACE', [
'load("//:deps.bzl", "repos")',
'repos()',
])
self.ScratchFile('BUILD.bazel')
self.ScratchFile('deps.bzl', [
'def repos():',
' native.new_local_repository(',
' name = "io_bazel_rules_go",',
' path = ".",',
""" build_file_content = "exports_files(glob(['*.go']))",""",
' )',
])
self._AssertQueryOutputContains('buildfiles(//external:io_bazel_rules_go)',
'//external:WORKSPACE', '//:deps.bzl',
'//:BUILD.bazel')

def testBuildFilesForExternalRepos_IndirectLoads(self):
self.ScratchFile('WORKSPACE', [
'load("//:deps.bzl", "repos")',
'repos()',
])
self.ScratchFile('BUILD.bazel')
self.ScratchFile('deps.bzl', [
'load("//:private_deps.bzl", "other_repos")',
'def repos():',
' native.new_local_repository(',
' name = "io_bazel_rules_go",',
' path = ".",',
""" build_file_content = "exports_files(glob(['*.go']))",""",
' )',
' other_repos()',
'',
])
self.ScratchFile('private_deps.bzl', [
'def other_repos():',
' native.new_local_repository(',
' name = "io_bazel_rules_python",',
' path = ".",',
""" build_file_content = "exports_files(glob(['*.py']))",""",
' )',
])

self._AssertQueryOutputContains(
'buildfiles(//external:io_bazel_rules_python)', '//external:WORKSPACE',
'//:deps.bzl', '//:private_deps.bzl', '//:BUILD.bazel')

def testBuildFilesForExternalRepos_NoDuplicates(self):
self.ScratchFile('WORKSPACE', [
'load("//:deps.bzl", "repos")',
'repos()',
])
self.ScratchFile('BUILD.bazel')
self.ScratchFile('deps.bzl', [
'def repos():',
' native.new_local_repository(',
' name = "io_bazel_rules_go",',
' path = ".",',
""" build_file_content = "exports_files(glob(['*.go']))",""",
' )',
' other_repos()',
'',
'def other_repos():',
' native.new_local_repository(',
' name = "io_bazel_rules_python",',
' path = ".",',
""" build_file_content = "exports_files(glob(['*.py']))",""",
' )',
])

exit_code, stdout, stderr = self.RunBazel(
['query', 'buildfiles(//external:io_bazel_rules_python)'])
self.AssertExitCode(exit_code, 0, stderr)
result = set()
for item in stdout:
if not item:
continue
self.assertNotIn(item, result)
result.add(item)

def _AssertQueryOutput(self, query_expr, *expected_results):
exit_code, stdout, stderr = self.RunBazel(['query', query_expr])
self.AssertExitCode(exit_code, 0, stderr)
Expand All @@ -46,6 +127,14 @@ def _AssertQueryOutput(self, query_expr, *expected_results):
self.assertEqual(len(stdout), len(expected_results))
self.assertListEqual(stdout, sorted(expected_results))

def _AssertQueryOutputContains(self, query_expr, *expected_content):
exit_code, stdout, stderr = self.RunBazel(['query', query_expr])
self.AssertExitCode(exit_code, 0, stderr)

stdout = {x for x in stdout if x}
for item in expected_content:
self.assertIn(item, stdout)


if __name__ == '__main__':
unittest.main()

0 comments on commit 52442ad

Please sign in to comment.