Skip to content

Commit

Permalink
Flip default Python version to PY3
Browse files Browse the repository at this point in the history
This flips --incompatible_py3_is_default and --incompatible_py2_outputs_are_suffixed. We opt out of this change for our own analysis and shell integration tests to avoid migrating them at the moment.

Fixes #7359, fixes #7593, fixes #6647.

RELNOTES[INC]: Python 3 is now the default Python version (for `py_binary` and `py_test` targets that don't specify the `python_version` attribute). Targets that are built for Python 3 will no longer have their output put in a separate `-py3` directory; instead there is now a separate `-py2` directory for Python 2 targets. See #7359 and #7593 for more information.

PiperOrigin-RevId: 241142275
  • Loading branch information
brandjon authored and copybara-github committed Mar 30, 2019
1 parent ad64a8d commit 5f5e6ad
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public String getTypeDescription() {
*/
@Option(
name = "incompatible_py3_is_default",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
effectTags = {
OptionEffectTag.LOADING_AND_ANALYSIS,
Expand All @@ -125,7 +125,7 @@ public String getTypeDescription() {

@Option(
name = "incompatible_py2_outputs_are_suffixed",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ protected BuildConfigurationCollection createCollection(String... args) throws E
.addAll(buildOptionClasses)
.add(TestOptions.class)
.build());
parser.parse(args);
parser.parse(TestConstants.PRODUCT_SPECIFIC_FLAGS);
parser.parse(args);

ImmutableSortedSet<String> multiCpu = ImmutableSortedSet.copyOf(
parser.getOptions(TestOptions.class).multiCpus);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ test_suite(
java_library(
name = "PythonTestUtils",
srcs = ["PythonTestUtils.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib:python-rules",
"//src/main/java/com/google/devtools/common/options",
"//third_party:junit4",
"//third_party:truth",
],
)

java_test(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package com.google.devtools.build.lib.rules.python;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.ensureDefaultIsPY2;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.assumesDefaultIsPY2;

import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
Expand Down Expand Up @@ -90,7 +90,7 @@ public void srcsVersionClashesWithVersionFlagUnderOldSemantics() throws Exceptio

@Test
public void versionIs2IfUnspecified() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file(
"pkg/BUILD", //
ruleName + "(",
Expand All @@ -106,7 +106,7 @@ public void versionIs3IfForcedByFlagUnderOldSemantics() throws Exception {
// py_binary, and py_test. Under the new semantics the rule attribute takes precedence, so this
// would only make sense for py_library; see PyLibraryConfiguredTargetTest for the analogous
// test.
ensureDefaultIsPY2();
assumesDefaultIsPY2();
useConfiguration(
"--incompatible_allow_python_version_transitions=false", "--python_version=PY3");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.ensureDefaultIsPY2;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.assumesDefaultIsPY2;

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -298,7 +298,7 @@ public void newVersionAttrTakesPrecedenceOverOld() throws Exception {

@Test
public void versionAttrWorksUnderOldAndNewSemantics_WhenNotDefaultValue() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3"));

assertPythonVersionIs_UnderNewConfigs(
Expand All @@ -310,7 +310,7 @@ public void versionAttrWorksUnderOldAndNewSemantics_WhenNotDefaultValue() throws

@Test
public void versionAttrWorksUnderOldAndNewSemantics_WhenSameAsDefaultValue() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2"));

assertPythonVersionIs_UnderNewConfigs(
Expand All @@ -322,7 +322,7 @@ public void versionAttrWorksUnderOldAndNewSemantics_WhenSameAsDefaultValue() thr

@Test
public void flagTakesPrecedenceUnderOldSemantics_NonDefaultValue() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2"));
assertPythonVersionIs_UnderNewConfig(
"//pkg:foo",
Expand All @@ -333,7 +333,7 @@ public void flagTakesPrecedenceUnderOldSemantics_NonDefaultValue() throws Except

@Test
public void flagTakesPrecedenceUnderOldSemantics_DefaultValue() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3"));
assertPythonVersionIs_UnderNewConfig(
"//pkg:foo",
Expand All @@ -344,7 +344,7 @@ public void flagTakesPrecedenceUnderOldSemantics_DefaultValue() throws Exception

@Test
public void versionAttrTakesPrecedenceUnderNewSemantics_NonDefaultValue() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3"));

// Test against both flags.
Expand All @@ -363,7 +363,7 @@ public void versionAttrTakesPrecedenceUnderNewSemantics_NonDefaultValue() throws

@Test
public void versionAttrTakesPrecedenceUnderNewSemantics_DefaultValue() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2"));

// Test against both flags.
Expand Down Expand Up @@ -402,7 +402,7 @@ public void canBuildWithDifferentVersionAttrs_UnderOldAndNewSemantics() throws E
@Test
public void canBuildWithDifferentVersionAttrs_UnderOldSemantics_FlagSetToDefault()
throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file(
"pkg/BUILD",
ruleDeclWithPyVersionAttr("foo_v2", "PY2"),
Expand All @@ -423,7 +423,7 @@ public void canBuildWithDifferentVersionAttrs_UnderOldSemantics_FlagSetToDefault
@Test
public void canBuildWithDifferentVersionAttrs_UnderOldSemantics_FlagSetToNonDefault()
throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
scratch.file(
"pkg/BUILD",
ruleDeclWithPyVersionAttr("foo_v2", "PY2"),
Expand Down Expand Up @@ -478,7 +478,7 @@ public void incompatibleSrcsVersion_NewSemantics() throws Exception {

@Test
public void incompatibleSrcsVersion_DueToVersionAttrDefault() throws Exception {
ensureDefaultIsPY2(); // When changed to PY3, flip srcs_version below to be PY2ONLY.
assumesDefaultIsPY2(); // When changed to PY3, flip srcs_version below to be PY2ONLY.

// This test doesn't care whether we use old and new semantics, but it affects how we assert.
useConfiguration("--incompatible_allow_python_version_transitions=false");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package com.google.devtools.build.lib.rules.python;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.ensureDefaultIsPY2;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.assumesDefaultIsPY2;

import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
Expand Down Expand Up @@ -54,7 +54,7 @@ public void canBuildWithIncompatibleSrcsVersionUnderNewSemantics() throws Except
public void versionIs3IfSetByFlagUnderNewSemantics() throws Exception {
// See PyBaseConfiguredTargetTestBase for the analogous test under the old semantics, which
// applies not just to py_library but also to py_binary and py_test.
ensureDefaultIsPY2();
assumesDefaultIsPY2();
useConfiguration(
"--incompatible_allow_python_version_transitions=true", "--python_version=PY3");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package com.google.devtools.build.lib.rules.python;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.ensureDefaultIsPY2;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.assumesDefaultIsPY2;

import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
Expand Down Expand Up @@ -72,7 +72,7 @@ public void nonhermeticRuntime() throws Exception {

@Test
public void pythonVersionDefault() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
// When using toolchains, the python_version attribute is mandatory.
useConfiguration("--incompatible_use_python_toolchains=false");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package com.google.devtools.build.lib.rules.python;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.ensureDefaultIsPY2;
import static com.google.devtools.build.lib.rules.python.PythonTestUtils.assumesDefaultIsPY2;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;

import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
Expand Down Expand Up @@ -103,7 +103,7 @@ public void getPythonVersion_FallBackOnDefaultPythonVersion() throws Exception {

@Test
public void getPythonVersion_NewFlagTakesPrecedence() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
// --force_python is superseded by --python_version.
PythonOptions opts =
parsePythonOptions(
Expand All @@ -115,7 +115,7 @@ public void getPythonVersion_NewFlagTakesPrecedence() throws Exception {

@Test
public void getPythonVersion_FallBackOnOldFlag() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
// --force_python is used because --python_version is absent.
PythonOptions opts =
parsePythonOptions(
Expand All @@ -125,15 +125,15 @@ public void getPythonVersion_FallBackOnOldFlag() throws Exception {

@Test
public void canTransitionPythonVersion_OldSemantics_Yes() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
PythonOptions opts =
parsePythonOptions("--incompatible_allow_python_version_transitions=false");
assertThat(opts.canTransitionPythonVersion(PythonVersion.PY3)).isTrue();
}

@Test
public void canTransitionPythonVersion_OldSemantics_NoBecauseAlreadySet() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
PythonOptions optsWithOldFlag =
parsePythonOptions(
"--incompatible_allow_python_version_transitions=false",
Expand All @@ -149,7 +149,7 @@ public void canTransitionPythonVersion_OldSemantics_NoBecauseAlreadySet() throws
@Test
public void canTransitionPythonVersion_OldSemantics_NoBecauseNewValueSameAsDefault()
throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
PythonOptions opts =
parsePythonOptions("--incompatible_allow_python_version_transitions=false");
assertThat(opts.canTransitionPythonVersion(PythonVersion.PY2)).isFalse();
Expand Down Expand Up @@ -234,7 +234,7 @@ public void getHost_CopiesMostValues() throws Exception {

@Test
public void getHost_AppliesHostForcePython() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
PythonOptions optsWithForcePythonFlag =
parsePythonOptions(
"--incompatible_remove_old_python_version_api=false",
Expand All @@ -261,7 +261,7 @@ public void getHost_AppliesHostForcePython() throws Exception {

@Test
public void getHost_Py3IsDefaultFlagChangesHost() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
PythonOptions opts =
// --incompatible_py3_is_default requires --incompatible_allow_python_version_transitions
parsePythonOptions(
Expand All @@ -281,7 +281,7 @@ public void getNormalized_OldSemantics() throws Exception {

@Test
public void getNormalized_NewSemantics() throws Exception {
ensureDefaultIsPY2();
assumesDefaultIsPY2();
PythonOptions opts = parsePythonOptions("--incompatible_allow_python_version_transitions=true");
PythonOptions normalizedOpts = (PythonOptions) opts.getNormalized();
assertThat(normalizedOpts.pythonVersion).isEqualTo(PythonVersion.PY2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@

package com.google.devtools.build.lib.rules.python;

import static com.google.common.truth.Truth.assertWithMessage;

import com.google.devtools.common.options.Options;

/** Helpers for Python tests. */
public class PythonTestUtils {
Expand All @@ -25,19 +23,23 @@ public class PythonTestUtils {
private PythonTestUtils() {}

/**
* Assert that the default Python version (for flagless builds) hasn't changed.
* Stub method that is used to annotate that the calling test case assumes the default Python
* version is PY2.
*
* <p>Use this to indicate that the PY2 and PY3 values of your test should be flipped if this
* default value is changed. In general, it is useful to write tests with expected values that
* differ from the default, so that they don't spuriously succeed if the default is erroneously
* returned.
* <p>Marking test cases that depend on the default Python version helps to diagnose failures. It
* also helps guard against accidentally making the test spuriously pass, e.g. if the expected
* value becomes the same as the default value..
*
* <p>Although the hard-coded default in {@link PythonOptions} has been flipped to PY3, we
* override this back to PY2 in our analysis-time tests and some of our integration tests. These
* tests will need to be ported in the future.
*/
public static void ensureDefaultIsPY2() {
assertWithMessage(
"This test case is written with the assumption that the default is Python 2. When "
+ "updating the default to Python 3, flip all the PY2/PY3 constants in the test "
+ "case and this helper function.")
.that(Options.getDefaults(PythonOptions.class).getDefaultPythonVersion())
.isEqualTo(PythonVersion.PY2);
public static void assumesDefaultIsPY2() {
// No-op.
}

/** Same as {@link #assumesDefaultIsPY2}, but for PY3. */
public static void assumesDefaultIsPY3() {
// No-op.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ public static void processSkyframeExecutorForTesting(SkyframeExecutor skyframeEx
* some reason.
*/
public static final ImmutableList<String> PRODUCT_SPECIFIC_FLAGS =
ImmutableList.of();
ImmutableList.of(
// TODO(#7903): Remove once our own tests are migrated.
"--incompatible_py3_is_default=false",
"--incompatible_py2_outputs_are_suffixed=false");

public static final BuilderFactoryForTesting PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING =
PackageFactoryBuilderFactoryForBazelUnitTests.INSTANCE;
Expand Down
20 changes: 15 additions & 5 deletions src/test/shell/bazel/python_version_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,9 @@ EOF
sed s/py3bin/py2bin/ test/py2bin_calling_py3bin.py > test/py3bin_calling_py2bin.py
chmod u+x test/py2bin_calling_py3bin.py test/py3bin_calling_py2bin.py

EXPFLAG="--incompatible_allow_python_version_transitions=true"
EXPFLAG="--incompatible_allow_python_version_transitions=true \
--incompatible_py3_is_default=false \
--incompatible_py2_outputs_are_suffixed=false"

bazel build $EXPFLAG //test:py2bin_calling_py3bin //test:py3bin_calling_py2bin \
|| fail "bazel build failed"
Expand Down Expand Up @@ -319,7 +321,9 @@ EOF

chmod u+x test/shbin_calling_py23bins.sh

EXPFLAG="--incompatible_allow_python_version_transitions=true"
EXPFLAG="--incompatible_allow_python_version_transitions=true \
--incompatible_py3_is_default=false \
--incompatible_py2_outputs_are_suffixed=false"

bazel build $EXPFLAG //test:shbin_calling_py23bins \
|| fail "bazel build failed"
Expand Down Expand Up @@ -359,8 +363,12 @@ EOF

# Run under both old and new semantics.
for EXPFLAG in \
"--incompatible_allow_python_version_transitions=true" \
"--incompatible_allow_python_version_transitions=false"; do
"--incompatible_allow_python_version_transitions=true \
--incompatible_py3_is_default=false \
--incompatible_py2_outputs_are_suffixed=false" \
"--incompatible_allow_python_version_transitions=false \
--incompatible_py3_is_default=false \
--incompatible_py2_outputs_are_suffixed=false"; do
echo "Using $EXPFLAG" > $TEST_log
bazel build $EXPFLAG --host_force_python=PY2 //test:genrule_calling_pybin \
|| fail "bazel build failed"
Expand Down Expand Up @@ -449,7 +457,9 @@ $(rlocation {{WORKSPACE_NAME}}/test/py3bin)
EOF
chmod u+x test/shbin.sh

EXPFLAG="--incompatible_allow_python_version_transitions=true"
EXPFLAG="--incompatible_allow_python_version_transitions=true \
--incompatible_py3_is_default=false \
--incompatible_py2_outputs_are_suffixed=false"

bazel build $EXPFLAG //test:shbin \
|| fail "bazel build failed"
Expand Down
4 changes: 3 additions & 1 deletion src/test/shell/integration/python_stub_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ EOF
expect_log "I am Python 3"

# These assertions try to override the version, which is legacy semantics.
FLAG=--incompatible_allow_python_version_transitions=false
FLAG="--incompatible_allow_python_version_transitions=false \
--incompatible_py3_is_default=false \
--incompatible_py2_outputs_are_suffixed=false"

# Force to Python 2.
bazel run //test:main2 $FLAG --python_version=PY2 \
Expand Down

0 comments on commit 5f5e6ad

Please sign in to comment.