Skip to content

Commit

Permalink
Automated rollback of commit bf66dc7.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Breaks a large number of downstream projects: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/946

Also breaks //src/test/py/bazel:runfiles_test on Bazel CI.

*** Original change description ***

Enable Python toolchains by default

This flips --incompatible_use_python_toolchains, which deprecates --python_top (and for the most part, --python_path). See #7899 for more on the change and migration procedure.

RELNOTES[INC]: Python rules now determine the Python runtime using toolchains rather than `--python_top` and `--python_path`, which are deprecated. See #7899 for information on declaring Python toolchains and migrating your code. As a side-benefit, this addresses #4815 (incorrect interpreter version used) on non-Windows platforms. You can temporarily opt out of this change with `--incompatible_use_python_toolchains=false`.

Fixes #7899, fixes #7375, significant progress on #4815.

PiperOrigin-RevId: 246324971
  • Loading branch information
laszlocsomor authored and copybara-github committed May 2, 2019
1 parent 27fad93 commit 4837e11
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public String getTypeDescription() {

@Option(
name = "incompatible_use_python_toolchains",
defaultValue = "true",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ private String getInterpreterPathFromStub(ConfiguredTarget pyExecutableTarget) {
"Failed to find the '%python_binary%' key in the stub script's template expansion action");
}

// TODO(#8169): Delete tests of the legacy --python_top / --python_path behavior.

@Test
public void runtimeSetByPythonTop() throws Exception {
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ public void pythonTop() throws Exception {
scratch.file(
"a/BUILD",
"py_runtime(name='b', files=[], interpreter='c')");
BazelPythonConfiguration config =
create("--incompatible_use_python_toolchains=false", "--python_top=//a:b")
.getFragment(BazelPythonConfiguration.class);
BazelPythonConfiguration config = create("--python_top=//a:b")
.getFragment(BazelPythonConfiguration.class);
assertThat(config.getPythonTop()).isNotNull();
}

Expand All @@ -44,7 +43,7 @@ public void pythonTop_malformedLabel() {
OptionsParsingException expected =
assertThrows(
OptionsParsingException.class,
() -> create("--incompatible_use_python_toolchains=false", "--python_top=//a:!b:"));
() -> create("--python_top=//a:!b:"));
assertThat(expected).hasMessageThat().contains("While parsing option --python_top");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,10 @@ public void setup(MockToolsConfig config) throws IOException {
"toolchain_type(name = 'toolchain_type')",
"constraint_setting(name = 'py2_interpreter_path')",
"constraint_setting(name = 'py3_interpreter_path')",
"py_runtime(",
" name = 'py2_interpreter',",
" interpreter_path = '/usr/bin/mockpython2',",
" python_version = 'PY2',",
")",
"py_runtime(",
" name = 'py3_interpreter',",
" interpreter_path = '/usr/bin/mockpython3',",
" python_version = 'PY3',",
")",
"py_runtime_pair(",
" name = 'default_py_runtime_pair',",
" py2_runtime = ':py2_interpreter',",
" py3_runtime = ':py3_interpreter',",
")",
"py_runtime_pair(name = 'dummy_py_runtime_pair')",
"toolchain(",
" name = 'default_python_toolchain',",
" toolchain = ':default_py_runtime_pair',",
" name = 'dummy_toolchain',",
" toolchain = ':dummy_py_runtime_pair',",
" toolchain_type = ':toolchain_type',",
")",
"exports_files(['precompile.py'])",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ java_test(
"//src/main/java/com/google/devtools/build/lib:python-rules",
"//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",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.testutil.TestConstants;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -175,10 +174,6 @@ public void runtimeSandwich() throws Exception {
")");
scratch.file(
"pkg/BUILD",
"load('"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/python:toolchain.bzl', "
+ "'py_runtime_pair')",
"load(':rules.bzl', 'userruntime')",
"py_runtime(",
" name = 'pyruntime',",
Expand All @@ -192,22 +187,13 @@ public void runtimeSandwich() throws Exception {
" interpreter = ':userintr',",
" files = ['userdata.txt'],",
")",
"py_runtime_pair(",
" name = 'userruntime_pair',",
" py2_runtime = 'userruntime',",
")",
"toolchain(",
" name = 'usertoolchain',",
" toolchain = ':userruntime_pair',",
" toolchain_type = '"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/python:toolchain_type',",
")",
"py_binary(",
" name = 'pybin',",
" srcs = ['pybin.py'],",
")");
useConfiguration("--extra_toolchains=//pkg:usertoolchain");
String pythonTopLabel =
analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:userruntime");
useConfiguration("--python_top=" + pythonTopLabel);
ConfiguredTarget target = getConfiguredTarget("//pkg:pybin");
assertThat(collectRunfiles(target))
.containsAtLeast(getSourceArtifact("pkg/data.txt"), getSourceArtifact("pkg/userdata.txt"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ fail_if_no_android_sdk
source "${CURRENT_DIR}/../../integration_test_setup.sh" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }

# TODO(#8169): Make this test compatible with Python toolchains. Blocked on the
# fact that there's no PY3 environment on our Mac workers
# (bazelbuild/continuous-integration#578).
add_to_bazelrc "build --incompatible_use_python_toolchains=false"

function setup_android_instrumentation_test_env() {
mkdir -p java/com/bin/res/values
mkdir -p javatests/com/bin
Expand Down
69 changes: 48 additions & 21 deletions src/test/shell/bazel/python_version_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ fi
source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }

# TODO(bazelbuild/continuous-integration#578): Enable this test for Mac and
# Windows.

# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
# `tr` converts all upper case letters to lower case.
# `case` matches the result if the `uname | tr` expression to string prefixes
Expand All @@ -52,15 +55,15 @@ case "$(uname -s | tr [:upper:] [:lower:])" in
msys*)
# As of 2018-08-14, Bazel on Windows only supports MSYS Bash.
declare -r is_windows=true
# As of 2019-04-26, this test is disabled on Windows (via "no_windows" tag),
# so this code shouldn't even have run.
# TODO(bazelbuild/continuous-integration#578): Enable this test for Windows.
# As of 2018-12-17, this test is disabled on windows (via "no_windows" tag),
# so this code shouldn't even have run. See the TODO at
# use_system_python_2_3_runtimes.
fail "This test does not run on Windows."
;;
darwin*)
# As of 2019-04-26, this test is disabled on mac, but there's no "no_mac" tag
# so we just have to trivially succeed.
# TODO(bazelbuild/continuous-integration#578): Enable this test for Mac.
# As of 2018-12-17, this test is disabled on mac, but there's no "no_mac" tag
# so we just have to trivially succeed. See the TODO at
# use_system_python_2_3_runtimes.
echo "This test does not run on Mac; exiting early." >&2
exit 0
;;
Expand All @@ -76,6 +79,44 @@ if "$is_windows"; then
export MSYS2_ARG_CONV_EXCL="*"
fi

# Use a py_runtime that invokes either the system's Python 2 or Python 3
# interpreter based on the Python mode. On Unix this is a workaround for #4815.
#
# TODO(brandjon): Replace this with the autodetecting Python toolchain.
function use_system_python_2_3_runtimes() {
PYTHON2_BIN=$(which python2 || echo "")
PYTHON3_BIN=$(which python3 || echo "")
# Debug output.
echo "Python 2 interpreter: ${PYTHON2_BIN:-"Not found"}"
echo "Python 3 interpreter: ${PYTHON3_BIN:-"Not found"}"
# Fail if either isn't present.
if [[ -z "${PYTHON2_BIN:-}" || -z "${PYTHON3_BIN:-}" ]]; then
fail "Can't use system interpreter: Could not find one or both of \
'python2', 'python3'"
fi

# Point Python builds at a py_runtime target defined in a //tools package of
# the main repo. This is not related to @bazel_tools//tools/python.
add_to_bazelrc "build --python_top=//tools/python:default_runtime"

mkdir -p tools/python

cat > tools/python/BUILD << EOF
package(default_visibility=["//visibility:public"])
py_runtime(
name = "default_runtime",
files = [],
interpreter_path = select({
"@bazel_tools//tools/python:PY2": "${PYTHON2_BIN}",
"@bazel_tools//tools/python:PY3": "${PYTHON3_BIN}",
}),
)
EOF
}

use_system_python_2_3_runtimes

#### TESTS #############################################################

# Sanity test that our environment setup above works.
Expand Down Expand Up @@ -161,8 +202,6 @@ function test_build_python_zip_works_with_py_runtime() {
mkdir -p test

cat > test/BUILD << EOF
load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")
py_binary(
name = "pybin",
srcs = ["pybin.py"],
Expand All @@ -173,17 +212,6 @@ py_runtime(
interpreter = ":mockpy.sh",
python_version = "PY3",
)
py_runtime_pair(
name = "mock_runtime_pair",
py3_runtime = ":mock_runtime",
)
toolchain(
name = "mock_toolchain",
toolchain = ":mock_runtime_pair",
toolchain_type = "@bazel_tools//tools/python:toolchain_type",
)
EOF
cat > test/pybin.py << EOF
# This doesn't actually run because we use a mock Python runtime that never
Expand All @@ -196,8 +224,7 @@ echo "I am mockpy!"
EOF
chmod u+x test/mockpy.sh

bazel run //test:pybin \
--extra_toolchains=//test:mock_toolchain --build_python_zip \
bazel run //test:pybin --python_top=//test:mock_runtime --build_python_zip \
&> $TEST_log || fail "bazel run failed"
expect_log "I am mockpy!"
}
Expand Down
8 changes: 2 additions & 6 deletions src/test/shell/integration/runfiles_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,14 @@ msys*|mingw*|cygwin*)
;;
esac

# We disable Python toolchains in EXTRA_BUILD_FLAGS because it throws off the
# counts and manifest checks in test_foo_runfiles.
# TODO(#8169): Update this test and remove the toolchain opt-out.
if "$is_windows"; then
export MSYS_NO_PATHCONV=1
export MSYS2_ARG_CONV_EXCL="*"
export EXT=".exe"
export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false \
--enable_runfiles --build_python_zip=0"
export EXTRA_BUILD_FLAGS="--enable_runfiles --build_python_zip=0"
else
export EXT=""
export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false"
export EXTRA_BUILD_FLAGS=""
fi

#### SETUP #############################################################
Expand Down
34 changes: 10 additions & 24 deletions src/test/shell/testenv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -569,13 +569,11 @@ function use_fake_python_runtimes_for_testsuite() {
PYTHON3_FILENAME="python3.sh"
fi

add_to_bazelrc "build --extra_toolchains=//tools/python:fake_python_toolchain"
add_to_bazelrc "build --python_top=//tools/python:default_runtime"

mkdir -p tools/python

cat > tools/python/BUILD << EOF
load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")
package(default_visibility=["//visibility:public"])
sh_binary(
Expand All @@ -584,27 +582,15 @@ sh_binary(
)
py_runtime(
name = "fake_py2_interpreter",
interpreter = ":${PYTHON2_FILENAME}",
python_version = "PY2",
)
py_runtime(
name = "fake_py3_interpreter",
interpreter = ":${PYTHON3_FILENAME}",
python_version = "PY3",
)
py_runtime_pair(
name = "fake_py_runtime_pair",
py2_runtime = ":fake_py2_interpreter",
py3_runtime = ":fake_py3_interpreter",
)
toolchain(
name = "fake_python_toolchain",
toolchain = ":fake_py_runtime_pair",
toolchain_type = "@bazel_tools//tools/python:toolchain_type",
name = "default_runtime",
files = select({
"@bazel_tools//tools/python:PY3": [":${PYTHON3_FILENAME}"],
"//conditions:default": [":${PYTHON2_FILENAME}"],
}),
interpreter = select({
"@bazel_tools//tools/python:PY3": ":${PYTHON3_FILENAME}",
"//conditions:default": ":${PYTHON2_FILENAME}",
}),
)
EOF

Expand Down

0 comments on commit 4837e11

Please sign in to comment.