Update documentation for Python version mechanisms

This isn't perfect in terms of linkifying names of rules/attrs, but it's a vast improvement and needed to get this change migration-ready. Ideally we'd have a dedicated Python concepts page.

Also did a little drive-by cleanup of unrelated attributes' docs and a few redundancies in attribute declarations.

Work toward #6583, #6442.

RELNOTES: None
PiperOrigin-RevId: 231966448

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyLibraryRule.java

@@ -31,30 +31,15 @@ public final class BazelPyLibraryRule implements RuleDefinition {
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
     return builder
         .requiresConfigurationFragments(PythonConfiguration.class)
-        /* <!-- #BLAZE_RULE(py_library).ATTRIBUTE(deps) -->
-        The list of other libraries to be linked in to the library target.
-        See general comments about <code>deps</code> at
-        <a href="${link common-definitions#common-attributes}">
-        Attributes common to all build rules</a>.
-        In practice, these arguments are treated like those in <code>srcs</code>;
-        you may move items between these lists willy-nilly.  It's probably more
-        readable to keep your <code>.py</code> files in your <code>srcs</code>.
-        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-
-        /* <!-- #BLAZE_RULE(py_library).ATTRIBUTE(data) -->
-        The list of files needed by this library at runtime.
-        See general comments about <code>data</code> at
-        <a href="${link common-definitions#common-attributes}">
-        Attributes common to all build rules</a>.
-        <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
 
         /* <!-- #BLAZE_RULE(py_library).ATTRIBUTE(srcs) -->
-        The list of source files that are processed to create the target.
+        The list of source (<code>.py</code>) files that are processed to create the target.
         This includes all your checked-in code and any generated source files.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("srcs", LABEL_LIST)
-            .direct_compile_time_input()
-            .allowedFileTypes(BazelPyRuleClasses.PYTHON_SOURCE))
+        .add(
+            attr("srcs", LABEL_LIST)
+                .direct_compile_time_input()
+                .allowedFileTypes(BazelPyRuleClasses.PYTHON_SOURCE))
         .build();
   }
 

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java

@@ -65,8 +65,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
           See general comments about <code>deps</code> at
           <a href="${link common-definitions#common-attributes}">
           Attributes common to all build rules</a>.
-          These can be
-          <a href="${link py_binary}"><code>py_binary</code></a> rules,
+          These are generally
           <a href="${link py_library}"><code>py_library</code></a> rules.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .override(
@@ -80,7 +79,6 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
                               SkylarkProviderIdentifier.forLegacy(PyStructUtils.PROVIDER_NAME)),
                           // Modern provider.
                           ImmutableList.of(PyInfo.PROVIDER.id())))
-                  .legacyMandatoryProviders(PyStructUtils.PROVIDER_NAME)
                   .allowedFileTypes())
           /* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(imports) -->
           List of import directories to be added to the <code>PYTHONPATH</code>.
@@ -97,26 +95,35 @@ Absolute paths (paths that start with <code>/</code>) and paths that references
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("imports", STRING_LIST).value(ImmutableList.<String>of()))
           /* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(srcs_version) -->
-          The value set here is for documentation purpose, and it will NOT determine which version
-          of python interpreter to use. Starting with 0.5.3 this attribute has been deprecated and
-          no longer has effect.
-          A string specifying the Python major version(s) that the <code>.py</code> source
-          files listed in the <code>srcs</code> of this rule are compatible with.
-          Please reference to <a href="${link py_runtime}"><code>py_runtime</code></a> rules for
-          determining the python version.
-          Valid values are:<br/>
-          <code>"PY2ONLY"</code> -
-            Python 2 code that is <b>not</b> suitable for <code>2to3</code> conversion.<br/>
-          <code>"PY2"</code> -
-            Python 2 code that is expected to work when run through <code>2to3</code>.<br/>
-          <code>"PY2AND3"</code> -
-            Code that is compatible with both Python 2 and 3 without
-            <code>2to3</code> conversion.<br/>
-          <code>"PY3ONLY"</code> -
-            Python 3 code that will not run on Python 2.<br/>
-          <code>"PY3"</code> -
-            A synonym for PY3ONLY.<br/>
-          <br/>
+          This attribute declares the target's <code>srcs</code> to be compatible with either Python
+          2, Python 3, or both. To actually set the Python runtime version, use the
+          <a href="${link py_binary.python_version}"><code>python_version</code></a> attribute of an
+          executable Python rule (<code>py_binary</code> or <code>py_test</code>).
+
+          <p>Allowed values are: <code>"PY2AND3"</code>, <code>"PY2"</code>, and <code>"PY3"</code>.
+          The values <code>"PY2ONLY"</code> and <code>"PY3ONLY"</code> are also allowed for historic
+          reasons, but they are essentially the same as <code>"PY2"</code> and <code>"PY3"</code>
+          and should be avoided.
+
+          <p>Under the old semantics
+          (<code>--experimental_allow_python_version_transitions=false</code>), it is an error to
+          build any Python target for a version disallowed by its <code>srcs_version</code>
+          attribute. Under the new semantics
+          (<code>--experimental_allow_python_version_transitions=true</code>), this check is
+          deferred to the executable rule: You can build a <code>srcs_version = "PY3"</code>
+          <code>py_library</code> target for Python 2, but you cannot actually depend on it via
+          <code>deps</code> from a Python 3 <code>py_binary</code>.
+
+          <p>To get diagnostic information about which dependencies introduce version requirements,
+          you can run the <code>find_requirements</code> aspect on your target:
+          <pre>
+          bazel build &lt;your target&gt; \
+              --aspects=@bazel_tools//tools/python:srcs_version.bzl%find_requirements \
+              --output_groups=pyversioninfo
+          </pre>
+          This will build a file with the suffix <code>-pyversioninfo.txt</code> giving information
+          about why your target requires one Python version or another. Note that it works even if
+          the given target failed to build due to a version conflict.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(
               attr("srcs_version", STRING)
@@ -154,15 +161,6 @@ public static final class PyBinaryBaseRule implements RuleDefinition {
     @Override
     public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironment env) {
       return builder
-          /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(data) -->
-          The list of files needed by this binary at runtime.
-          See general comments about <code>data</code> at
-          <a href="${link common-definitions#common-attributes}">
-          Attributes common to all build rules</a>.
-          Also see the <a href="${link py_library.data}"><code>data</code></a> argument of
-          the <a href="${link py_library}"><code>py_library</code></a> rule for details.
-          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(main) -->
           The name of the source file that is the main entry point of the application.
           This file must also be listed in <code>srcs</code>. If left unspecified,
@@ -171,12 +169,10 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("main", LABEL).allowedFileTypes(PYTHON_SOURCE))
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(default_python_version) -->
-          A string specifying the default Python major version to use when building this binary and
-          all of its <code>deps</code>.
-          Valid values are <code>"PY2"</code> (default) or <code>"PY3"</code>.
-          Python 3 support is experimental.
-          <p>If both this attribute and <code>python_version</code> are supplied,
-          <code>default_python_version</code> will be ignored.
+          A deprecated alias for <code>python_version</code>; use that instead. This attribute is
+          disabled under <code>--experimental_remove_old_python_version_api</code>. For migration
+          purposes, if <code>python_version</code> is given then the value of
+          <code>default_python_version</code> is ignored.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(
               attr(PyCommon.DEFAULT_PYTHON_VERSION_ATTRIBUTE, STRING)
@@ -186,8 +182,25 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
                       "read by PyRuleClasses.PYTHON_VERSION_TRANSITION, which doesn't have access"
                           + " to the configuration"))
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(python_version) -->
-          A replacement for <code>default_python_version</code>. If both this and
-          <code>default_python_version</code> are supplied, the latter will be ignored.
+          Whether to build this target (and its transitive <code>deps</code>) for Python 2 or Python
+          3. Valid values are <code>"PY2"</code> (the default) and <code>"PY3"</code>.
+
+          <p>Under the old semantics
+          (<code>--experimental_allow_python_version_transitions=false</code>), the Python version
+          generally cannot be changed once set. This means that the <code>--python_version</code>
+          flag overrides this attribute, and other Python binaries in the <code>data</code> deps of
+          this target are forced to use the same version as this target.
+
+          <p>Under the new semantics
+          (<code>--experimental_allow_python_version_transitions=true</code>), the Python version
+          is always set (possibly by default) to whatever version is specified by this attribute,
+          regardless of the version specified on the command line or by other targets that depend on
+          this one.
+
+          <p>If you want to <code>select()</code> on the current Python version, you can inspect the
+          value of <code>@bazel_tools//tools/python:python_version</code>. See
+          <a href="https://github.com/bazelbuild/bazel/blob/4b74ea9a3f81b7ed30562f1689827b5488884c86/tools/python/BUILD#L33">here</a>
+          for more information.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(
               attr(PyCommon.PYTHON_VERSION_ATTRIBUTE, STRING)
@@ -197,26 +210,23 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
                       "read by PyRuleClasses.PYTHON_VERSION_TRANSITION, which doesn't have access"
                           + " to the configuration"))
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(srcs) -->
-          The list of source files that are processed to create the target.
-          This includes all your checked-in code and any
-          generated source files.  The line between <code>srcs</code> and
-          <code>deps</code> is loose. The <code>.py</code> files
-          probably belong in <code>srcs</code> and library targets probably belong
-          in <code>deps</code>, but don't worry about it too much.
+          The list of source (<code>.py</code>) files that are processed to create the target.
+          This includes all your checked-in code and any generated source files. Library targets
+          belong in <code>deps</code> instead, while other binary files needed at runtime belong in
+          <code>data</code>.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(
               attr("srcs", LABEL_LIST)
                   .mandatory()
                   .allowedFileTypes(PYTHON_SOURCE)
-                  .direct_compile_time_input()
-                  .allowedFileTypes(BazelPyRuleClasses.PYTHON_SOURCE))
+                  .direct_compile_time_input())
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(legacy_create_init) -->
           Whether to implicitly create empty __init__.py files in the runfiles tree.
           These are created in every directory containing Python source code or
-          shared libraries, and every parent directory of those directories.
-          Default is true for backward compatibility.  If false, the user is responsible
-          for creating __init__.py files (empty or not) and adding them to `srcs` or `deps`
-          of Python targets as required.
+          shared libraries, and every parent directory of those directories, excluding the repo root
+          directory. The default is true for backward compatibility. If false, the user is
+          responsible for creating (possibly empty) __init__.py files and adding them to the
+          <code>srcs</code> of Python targets as required.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("legacy_create_init", BOOLEAN).value(true))
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(stamp) -->

src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java

@@ -483,14 +483,16 @@ private boolean maybeCreateFailActionDueToTransitiveSourcesVersion() {
     if (!ruleContext.getFragment(PythonConfiguration.class).useNewPyVersionSemantics()) {
       return false;
     }
-    // TODO(brandjon): Add link to documentation explaining the error and use of the aspect.
+    String errorTemplate =
+        "This target is being built for Python %s but (transitively) includes Python %s-only "
+            + "sources. You can get diagnostic information about which dependencies introduce this "
+            + "version requirement by running the `find_requirements` aspect. For more info see "
+            + "the documentation for the `srcs_version` attribute.";
     String error = null;
     if (version == PythonVersion.PY2 && hasPy3OnlySources) {
-      error =
-          "target is being built for Python 2 but (transitively) includes Python 3-only sources";
+      error = String.format(errorTemplate, "2", "3");
     } else if (version == PythonVersion.PY3 && hasPy2OnlySources) {
-      error =
-          "target is being built for Python 3 but (transitively) includes Python 2-only sources";
+      error = String.format(errorTemplate, "3", "2");
     }
     if (error == null) {
       return false;

src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java

@@ -69,30 +69,28 @@ public String getTypeDescription() {
       help = "Build python executable zip; on on Windows, off on other platforms")
   public TriState buildPythonZip;
 
-  // TODO(brandjon): For both experimental options below, add documentation and add a link to it
-  // from the help text, then change the documentationCategory to SKYLARK_SEMANTICS.
-
   @Option(
       name = "experimental_remove_old_python_version_api",
-      // TODO(brandjon): Do not flip until we have an answer for how to disallow the
-      // "default_python_version" attribute without hacking up native.existing_rules(). See
-      // #7071 and b/122596733.
       defaultValue = "false",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {OptionMetadataTag.EXPERIMENTAL},
       help =
           "If true, disables use of the `--force_python` flag and the `default_python_version` "
-              + "attribute for `py_binary` and `py_test`.")
+              + "attribute for `py_binary` and `py_test`. Use the `--python_version` flag and "
+              + "`python_version` attribute instead, which have exactly the same meaning. This "
+              + "flag also disables `select()`-ing over `--host_force_python`.")
   public boolean experimentalRemoveOldPythonVersionApi;
 
   @Option(
       name = "experimental_allow_python_version_transitions",
       defaultValue = "false",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {OptionMetadataTag.EXPERIMENTAL},
-      help = "If true, Python rules use the new PY2/PY3 version semantics.")
+      help =
+          "If true, Python rules use the new PY2/PY3 version semantics. For more information, see "
+              + "the documentation for `py_binary`'s `python_version` attribute.")
   public boolean experimentalAllowPythonVersionTransitions;
 
   /**
@@ -107,17 +105,16 @@ public String getTypeDescription() {
       name = "python_version",
       defaultValue = "null",
       converter = TargetPythonVersionConverter.class,
-      // TODO(brandjon): Change to OptionDocumentationCategory.GENERIC_INPUTS when this is
-      // sufficiently implemented/documented.
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
       effectTags = {
         OptionEffectTag.LOADING_AND_ANALYSIS,
         OptionEffectTag.AFFECTS_OUTPUTS // because of "-py3" output root
       },
       help =
-          "The Python major version mode, either `PY2` or `PY3`. Note that this is overridden by "
-              + "`py_binary` and `py_test` targets (whether or not they specify their version "
-              + "explicitly), so there is usually not much reason to supply this flag.")
+          "The Python major version mode, either `PY2` or `PY3`. Note that under the new version "
+              + "semantics (`--experimental_allow_python_version_transitions`) this is overridden "
+              + "by `py_binary` and `py_test` targets (even if they don't explicitly specify a "
+              + "version) so there is usually not much reason to supply this flag.")
   public PythonVersion pythonVersion;
 
   private static final OptionDefinition PYTHON_VERSION_DEFINITION =
@@ -140,7 +137,9 @@ public String getTypeDescription() {
       converter = TargetPythonVersionConverter.class,
       documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      help = "Overrides default_python_version attribute. Can be \"PY2\" or \"PY3\".")
+      help =
+          "Deprecated alias for `--python_version`. Disabled by "
+              + "`--experimental_remove_old_python_version_api`.")
   public PythonVersion forcePython;
 
   private static final OptionDefinition FORCE_PYTHON_DEFINITION =
@@ -186,19 +185,25 @@ public String getTypeDescription() {
 
   @Override
   public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
-    // TODO(brandjon): Add an error string that references documentation explaining to use
-    // @bazel_tools//tools/python:python_version instead.
+    // TODO(brandjon): Instead of referencing the python_version target, whose path depends on the
+    // tools repo name, reference a standalone documentation page instead.
     ImmutableMap.Builder<OptionDefinition, SelectRestriction> restrictions = ImmutableMap.builder();
     restrictions.put(
         PYTHON_VERSION_DEFINITION,
-        new SelectRestriction(/*visibleWithinToolsPackage=*/ true, /*errorMessage=*/ null));
+        new SelectRestriction(
+            /*visibleWithinToolsPackage=*/ true,
+            "Use @bazel_tools//python/tools:python_version instead."));
     if (experimentalRemoveOldPythonVersionApi) {
       restrictions.put(
           FORCE_PYTHON_DEFINITION,
-          new SelectRestriction(/*visibleWithinToolsPackage=*/ true, /*errorMessage=*/ null));
+          new SelectRestriction(
+              /*visibleWithinToolsPackage=*/ true,
+              "Use @bazel_tools//python/tools:python_version instead."));
       restrictions.put(
           HOST_FORCE_PYTHON_DEFINITION,
-          new SelectRestriction(/*visibleWithinToolsPackage=*/ false, /*errorMessage=*/ null));
+          new SelectRestriction(
+              /*visibleWithinToolsPackage=*/ false,
+              "Use @bazel_tools//python/tools:python_version instead."));
     }
     return restrictions.build();
   }