Skip to content

Commit

Permalink
Add an aspect to @bazel_tools for debugging bad Python dependencies
Browse files Browse the repository at this point in the history
We need this aspect because under the new Python version semantics, incompatibilities in srcs_version are caught by py_binary rather than at the py_library where the mismatch occurred. The workflow is for users to see the validation error and follow a URL to documentation (to come in a later CL) instructing them to run this aspect to locate the bad dependencies.

The aspect propagates to deps and uses both srcs_version and the py provider fields to try to determine the top-most targets that require Python 2 or Python 3. It's possible this idiom of "find the topmost dependencies satisfying property X" is reusable and could be factored into Skylib (perhaps once this code is moved out of core Bazel and into rules_python).

Incidentally removed python_version_srcs filegroup in favor of just using srcs.

Work towards #6583 and #1446.

RELNOTES: None
PiperOrigin-RevId: 230813240
  • Loading branch information
brandjon authored and Copybara-Service committed Jan 25, 2019
1 parent e42913a commit 029601e
Show file tree
Hide file tree
Showing 8 changed files with 778 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,7 @@ private boolean maybeCreateFailActionDueToTransitiveSourcesVersion() {
if (!ruleContext.getFragment(PythonConfiguration.class).useNewPyVersionSemantics()) {
return false;
}
// TODO(brandjon): Add hints to the error message about how to locate the offending
// dependencies.
// TODO(brandjon): Add link to documentation explaining the error and use of the aspect.
String error = null;
if (version == PythonVersion.PY2 && hasPy3OnlySources) {
error =
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ java_library(
resources = [
"packages/util/MOCK_ANDROID_CROSSTOOL",
"packages/util/MOCK_OSX_CROSSTOOL",
"//tools/python:python_version_srcs",
"//tools/python:srcs",
],
deps = [
":default_test_build_rules",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ public class BazelMockPythonSupport extends MockPythonSupport {

public static final BazelMockPythonSupport INSTANCE = new BazelMockPythonSupport();

private static void addTool(MockToolsConfig config, String toolRelativePath) throws IOException {
config.create(
TestConstants.TOOLS_REPOSITORY_SCRATCH + toolRelativePath,
ResourceLoader.readFromResources(TestConstants.BAZEL_REPO_PATH + toolRelativePath));
}

@Override
public void setup(MockToolsConfig config) throws IOException {
config.create(
TestConstants.TOOLS_REPOSITORY_SCRATCH + "tools/python/python_version.bzl",
ResourceLoader.readFromResources(
TestConstants.BAZEL_REPO_PATH + "tools/python/python_version.bzl"));
addTool(config, "tools/python/python_version.bzl");
addTool(config, "tools/python/srcs_version.bzl");

config.create(
TestConstants.TOOLS_REPOSITORY_SCRATCH + "tools/python/BUILD",
Expand Down
14 changes: 14 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/python/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ test_suite(
":PyStructUtilsTest",
":PyTestConfiguredTargetTest",
":PythonConfigurationTest",
":PythonSrcsVersionAspectTest",
":PythonStarlarkApiTest",
":PythonVersionSelectTest",
":PythonVersionTest",
Expand Down Expand Up @@ -156,6 +157,19 @@ java_test(
],
)

java_test(
name = "PythonSrcsVersionAspectTest",
srcs = ["PythonSrcsVersionAspectTest.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/test/java/com/google/devtools/build/lib:analysis_testutil",
"//src/test/java/com/google/devtools/build/lib:testutil",
"//third_party:junit4",
"//third_party:truth",
],
)

java_test(
name = "PyProviderUtilsTest",
srcs = ["PyProviderUtilsTest.java"],
Expand Down
Loading

0 comments on commit 029601e

Please sign in to comment.