Skip to content

Commit

Permalink
[7.1.0] The label API shakeup & docs cleanup (#20977)
Browse files Browse the repository at this point in the history
Been a long time coming.

* `native.repository_name()` is deprecated (has a spurious `@` prefix).
A new method `native.repo_name()` is introduced instead.
* `Label.workspace_name` is deprecated (misnomer). A new field
`Label.repo_name` is introduced instead.
* `Label.relative()` is deprecated (error-prone). A new method
`Label.local_target_label()` is introduced with narrowed focus,
alongside which `native.package_relative_label()` and `Label()` are
recommended as replacements.

And then a bunch of documentation cleanups, hopefully reducing any
potential confusion.

Fixes #16210.

RELNOTES:
Various methods and fields related to labels and repos are deprecated in
favor of new options with clearer naming and intent. The deprecated APIs
can be disabled by setting
`--noincompatible_enable_deprecated_label_apis`.
* `native.repository_name()` is deprecated in favor of the new
`native.repo_name()`.
* `Label.workspace_name` is deprecated in favor of the new
`Label.repo_name`.
* `Label.relative()` is deprecated in favor of the new
`Label.same_package_label()` alongside the existing
`native.package_relative_label()` and `Label()`.
  • Loading branch information
Wyverald committed Jan 23, 2024
1 parent 4d55e13 commit e0180e1
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 54 deletions.
93 changes: 59 additions & 34 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,10 @@ public RepositoryName getRepository() {
name = "package",
structField = true,
doc =
"The package part of this label. "
+ "For instance:<br>"
+ "<pre class=language-python>Label(\"//pkg/foo:abc\").package == \"pkg/foo\"</pre>")
"The name of the package containing the target referred to by this label, without the"
+ " repository name. For instance:<br><pre"
+ " class=language-python>Label(\"@@repo//pkg/foo:abc\").package =="
+ " \"pkg/foo\"</pre>")
public String getPackageName() {
return packageIdentifier.getPackageFragment().getPathString();
}
Expand All @@ -300,9 +301,9 @@ public String getPackageName() {
name = "workspace_root",
structField = true,
doc =
"Returns the execution root for the workspace of this label, relative to the execroot. "
+ "For instance:<br>"
+ "<pre class=language-python>Label(\"@repo//pkg/foo:abc\").workspace_root =="
"Returns the execution root for the repository containing the target referred to by this"
+ " label, relative to the execroot. For instance:<br><pre"
+ " class=language-python>Label(\"@repo//pkg/foo:abc\").workspace_root =="
+ " \"external/repo\"</pre>",
useStarlarkSemantics = true)
@Deprecated
Expand Down Expand Up @@ -351,9 +352,8 @@ public PathFragment toPathFragment() {
name = "name",
structField = true,
doc =
"The name of this label within the package. "
+ "For instance:<br>"
+ "<pre class=language-python>Label(\"//pkg/foo:abc\").name == \"abc\"</pre>")
"The name of the target referred to by this label. For instance:<br>"
+ "<pre class=language-python>Label(\"@@foo//pkg/foo:abc\").name == \"abc\"</pre>")
public String getName() {
return name;
}
Expand Down Expand Up @@ -433,20 +433,41 @@ public String getShorthandDisplayForm(RepositoryMapping mainRepositoryMapping) {
name = "workspace_name",
structField = true,
doc =
"The repository part of this label. For instance, "
+ "<pre class=language-python>Label(\"@foo//bar:baz\").workspace_name"
+ " == \"foo\"</pre>")
"<strong>Deprecated.</strong> The field name \"workspace name\" is a misnomer here; use"
+ " the identically-behaving <a href=\"#repo_name\"><code>Label.repo_name</code></a>"
+ " instead.<p>The canonical name of the repository containing the target referred to"
+ " by this label, without any leading at-signs (<code>@</code>). For instance, <pre"
+ " class=language-python>Label(\"@@foo//bar:baz\").workspace_name == \"foo\"</pre>",
enableOnlyWithFlag = BuildLanguageOptions.INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS)
@Deprecated
public String getWorkspaceName() throws EvalException {
checkRepoVisibilityForStarlark("workspace_name");
return packageIdentifier.getRepository().getName();
}

/** Return the name of the repository label refers to without the leading `at` symbol. */
@StarlarkMethod(
name = "repo_name",
structField = true,
doc =
"The canonical name of the repository containing the target referred to by this label,"
+ " without any leading at-signs (<code>@</code>). For instance, <pre"
+ " class=language-python>Label(\"@@foo//bar:baz\").repo_name == \"foo\"</pre>")
public String getRepoName() throws EvalException {
checkRepoVisibilityForStarlark("repo_name");
return packageIdentifier.getRepository().getName();
}

/**
* Returns a label in the same package as this label with the given target name.
*
* @throws LabelSyntaxException if {@code targetName} is not a valid target name
*/
public Label getLocalTargetLabel(String targetName) throws LabelSyntaxException {
@StarlarkMethod(
name = "same_package_label",
doc = "Creates a label in the same package as this label with the given target name.",
parameters = {@Param(name = "target_name", doc = "The target name of the new label.")})
public Label getSamePackageLabel(String targetName) throws LabelSyntaxException {
return create(packageIdentifier, targetName);
}

Expand All @@ -462,34 +483,38 @@ public Label getLocalTargetLabel(String targetName) throws LabelSyntaxException
@StarlarkMethod(
name = "relative",
doc =
// TODO(#14503): Fix the documentation.
"Resolves a label that is either absolute (starts with <code>//</code>) or relative to "
+ "the current package. If this label is in a remote repository, the argument will "
+ "be resolved relative to that repository. If the argument contains a repository "
+ "name, the current label is ignored and the argument is returned as-is, except "
+ "that the repository name is rewritten if it is in the current repository mapping. "
+ "Reserved labels will also be returned as-is.<br>"
+ "For example:<br>"
+ "<pre class=language-python>\n"
"<strong>Deprecated.</strong> This method behaves surprisingly when used with an argument"
+ " containing an apparent repo name. Prefer <a"
+ " href=\"#local_target_label\"><code>Label.same_package_label()</code></a>, <a"
+ " href=\"../toplevel/native#package_relative_label\"><code>native.package_relative_label()</code></a>,"
+ " or <a href=\"#Label\"><code>Label()</code></a> instead.<p>Resolves a label that"
+ " is either absolute (starts with <code>//</code>) or relative to the current"
+ " package. If this label is in a remote repository, the argument will be resolved"
+ " relative to that repository. If the argument contains a repository name, the"
+ " current label is ignored and the argument is returned as-is, except that the"
+ " repository name is rewritten if it is in the current repository mapping. Reserved"
+ " labels will also be returned as-is.<br>For example:<br><pre"
+ " class=language-python>\n"
+ "Label(\"//foo/bar:baz\").relative(\":quux\") == Label(\"//foo/bar:quux\")\n"
+ "Label(\"//foo/bar:baz\").relative(\"//wiz:quux\") == Label(\"//wiz:quux\")\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"//wiz:quux\") == "
+ "Label(\"@repo//wiz:quux\")\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"//visibility:public\") == "
+ "Label(\"//visibility:public\")\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"@other//wiz:quux\") == "
+ "Label(\"@other//wiz:quux\")\n"
+ "</pre>"
+ "<p>If the repository mapping passed in is <code>{'@other' : '@remapped'}</code>, "
+ "then the following remapping will take place:<br>"
+ "<pre class=language-python>\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"@other//wiz:quux\") == "
+ "Label(\"@remapped//wiz:quux\")\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"//wiz:quux\") =="
+ " Label(\"@repo//wiz:quux\")\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"//visibility:public\") =="
+ " Label(\"//visibility:public\")\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"@other//wiz:quux\") =="
+ " Label(\"@other//wiz:quux\")\n"
+ "</pre><p>If the repository mapping passed in is <code>{'@other' :"
+ " '@remapped'}</code>, then the following remapping will take place:<br><pre"
+ " class=language-python>\n"
+ "Label(\"@repo//foo/bar:baz\").relative(\"@other//wiz:quux\") =="
+ " Label(\"@remapped//wiz:quux\")\n"
+ "</pre>",
parameters = {
@Param(name = "relName", doc = "The label that will be resolved relative to this one.")
},
enableOnlyWithFlag = BuildLanguageOptions.INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS,
useStarlarkThread = true)
@Deprecated
public Label getRelative(String relName, StarlarkThread thread) throws LabelSyntaxException {
return parseWithPackageContext(
relName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,18 +608,21 @@ public NoneType exportsFiles(
@Override
public String packageName(StarlarkThread thread) throws EvalException {
BazelStarlarkContext.checkLoadingPhase(thread, "native.package_name");
PackageIdentifier packageId =
PackageFactory.getContext(thread).getBuilder().getPackageIdentifier();
PackageIdentifier packageId = getContext(thread).getBuilder().getPackageIdentifier();
return packageId.getPackageFragment().getPathString();
}

@Override
public String repositoryName(StarlarkThread thread) throws EvalException {
BazelStarlarkContext.checkLoadingPhase(thread, "native.repository_name");
PackageIdentifier packageId =
PackageFactory.getContext(thread).getBuilder().getPackageIdentifier();
// for legacy reasons, this is prefixed with a single '@'.
return '@' + packageId.getRepository().getName();
return '@' + repoName(thread);
}

@Override
public String repoName(StarlarkThread thread) throws EvalException {
BazelStarlarkContext.checkLoadingPhase(thread, "native.repo_name");
return getContext(thread).getBuilder().getPackageIdentifier().getRepository().getName();
}

@Override
Expand All @@ -630,7 +633,7 @@ public Label packageRelativeLabel(Object input, StarlarkThread thread) throws Ev
}
try {
String s = (String) input;
return PackageFactory.getContext(thread).getBuilder().getLabelConverter().convert(s);
return getContext(thread).getBuilder().getLabelConverter().convert(s);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("invalid label in native.package_relative_label: %s", e.getMessage());
}
Expand All @@ -640,14 +643,14 @@ public Label packageRelativeLabel(Object input, StarlarkThread thread) throws Ev
@Nullable
public String moduleName(StarlarkThread thread) throws EvalException {
BazelStarlarkContext.checkLoadingPhase(thread, "native.module_name");
return PackageFactory.getContext(thread).getBuilder().getAssociatedModuleName().orElse(null);
return getContext(thread).getBuilder().getAssociatedModuleName().orElse(null);
}

@Override
@Nullable
public String moduleVersion(StarlarkThread thread) throws EvalException {
BazelStarlarkContext.checkLoadingPhase(thread, "native.module_version");
return PackageFactory.getContext(thread).getBuilder().getAssociatedModuleVersion().orElse(null);
return getContext(thread).getBuilder().getAssociatedModuleVersion().orElse(null);
}

private static Dict<String, Object> getRuleDict(Rule rule, Mutability mu) throws EvalException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,16 @@ public final class BuildLanguageOptions extends OptionsBase {
help = "Enable experimental rule extension API and subrule APIs")
public boolean experimentalRuleExtensionApi;

@Option(
name = "incompatible_enable_deprecated_label_apis",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
help =
"If enabled, certain deprecated APIs (native.repository_name, Label.workspace_name,"
+ " Label.relative) can be used.")
public boolean enableDeprecatedLabelApis;

/**
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
* never accumulate a large number of these and being able to shortcut on object identity makes a
Expand Down Expand Up @@ -829,6 +839,7 @@ public StarlarkSemantics toStarlarkSemantics() {
INCOMPATIBLE_DISABLE_NON_EXECUTABLE_JAVA_BINARY,
incompatibleDisableNonExecutableJavaBinary)
.setBool(EXPERIMENTAL_RULE_EXTENSION_API, experimentalRuleExtensionApi)
.setBool(INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS, enableDeprecatedLabelApis)
.build();
return INTERNER.intern(semantics);
}
Expand Down Expand Up @@ -925,6 +936,8 @@ public StarlarkSemantics toStarlarkSemantics() {
public static final String INCOMPATIBLE_DISABLE_NON_EXECUTABLE_JAVA_BINARY =
"-incompatible_disable_non_executable_java_binary";
public static final String EXPERIMENTAL_RULE_EXTENSION_API = "-experimental_rule_extension_api";
public static final String INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS =
"+incompatible_enable_deprecated_label_apis";

// non-booleans
public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.devtools.build.docgen.annot.GlobalMethods;
import com.google.devtools.build.docgen.annot.GlobalMethods.Environment;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import javax.annotation.Nullable;
import net.starlark.java.annot.Param;
import net.starlark.java.annot.ParamType;
Expand Down Expand Up @@ -226,27 +227,38 @@ NoneType exportsFiles(Sequence<?> srcs, Object visibility, Object licenses, Star
@StarlarkMethod(
name = "package_name",
doc =
"The name of the package being evaluated. "
"The name of the package being evaluated, without the repository name. "
+ "For example, in the BUILD file <code>some/package/BUILD</code>, its value "
+ "will be <code>some/package</code>. "
+ "If the BUILD file calls a function defined in a .bzl file, "
+ "<code>package_name()</code> will match the caller BUILD file package. "
+ "This function is equivalent to the deprecated variable <code>PACKAGE_NAME</code>.",
+ "<code>package_name()</code> will match the caller BUILD file package.",
useStarlarkThread = true)
String packageName(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "repository_name",
doc =
"The name of the repository the rule or build extension is called from. "
+ "For example, in packages that are called into existence by the WORKSPACE stanza "
+ "<code>local_repository(name='local', path=...)</code> it will be set to "
+ "<code>@local</code>. In packages in the main repository, it will be set to "
+ "<code>@</code>. This function is equivalent to the deprecated variable "
+ "<code>REPOSITORY_NAME</code>.",
"<strong>Deprecated.</strong> Prefer to use <a"
+ " href=\"#repo_name\"><code>repo_name</code></a> instead, which doesn't contain the"
+ " spurious leading at-sign, but behaves identically otherwise.<p>The canonical name"
+ " of the repository containing the package currently being evaluated, with a single"
+ " at-sign (<code>@</code>) prefixed. For example, in packages that are called into"
+ " existence by the WORKSPACE stanza <code>local_repository(name='local',"
+ " path=...)</code> it will be set to <code>@local</code>. In packages in the main"
+ " repository, it will be set to <code>@</code>.",
enableOnlyWithFlag = BuildLanguageOptions.INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS,
useStarlarkThread = true)
@Deprecated
String repositoryName(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "repo_name",
doc =
"The canonical name of the repository containing the package currently being evaluated,"
+ " with no leading at-signs.",
useStarlarkThread = true)
String repoName(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "package_relative_label",
doc =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,11 @@ public interface StarlarkRuleContextApi<ConstraintValueT extends ConstraintValue
@StarlarkMethod(
name = "workspace_name",
structField = true,
doc = "The workspace name as defined in the WORKSPACE file.")
doc =
"The name of the workspace, which is effectively the execution root name and runfiles"
+ " prefix for the main repo. If <code>--enable_bzlmod</code> is on, this is the"
+ " fixed string <code>_main</code>. Otherwise, this is the workspace name as defined"
+ " in the WORKSPACE file.")
String getWorkspaceName() throws EvalException;

@StarlarkMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ public String repositoryName(StarlarkThread thread) {
return "";
}

@Override
public String repoName(StarlarkThread thread) {
return "";
}

@Override
public Label packageRelativeLabel(Object input, StarlarkThread thread) throws EvalException {
return Label.parseCanonicalUnchecked("//:fake_label");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep
"--incompatible_disallow_empty_glob=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean(),
"--incompatible_enable_deprecated_label_apis=" + rand.nextBoolean(),
"--incompatible_java_common_parameters=" + rand.nextBoolean(),
"--incompatible_merge_fixed_and_default_shell_env=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
Expand Down Expand Up @@ -193,6 +194,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
.setBool(
BuildLanguageOptions.INCOMPATIBLE_DISALLOW_STRUCT_PROVIDER_SYNTAX, rand.nextBoolean())
.setBool(BuildLanguageOptions.INCOMPATIBLE_DO_NOT_SPLIT_LINKING_CMDLINE, rand.nextBoolean())
.setBool(BuildLanguageOptions.INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS, rand.nextBoolean())
.setBool(BuildLanguageOptions.INCOMPATIBLE_JAVA_COMMON_PARAMETERS, rand.nextBoolean())
.setBool(
BuildLanguageOptions.INCOMPATIBLE_MERGE_FIXED_AND_DEFAULT_SHELL_ENV, rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public static void addMockK8Platform(MockToolsConfig mockToolsConfig, Label cros
"toolchain(",
" name = 'toolchain_cc-compiler-k8',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',",
" toolchain = '" + crosstoolLabel.getLocalTargetLabel("cc-compiler-k8-compiler") + "',",
" toolchain = '" + crosstoolLabel.getSamePackageLabel("cc-compiler-k8-compiler") + "',",
" target_compatible_with = [':mock_value'],",
")");
}
Expand All @@ -220,7 +220,7 @@ public static void addMockPPCPlatform(MockToolsConfig mockToolsConfig, Label cro
"toolchain(",
" name = 'toolchain_cc-compiler-ppc',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',",
" toolchain = '" + crosstoolLabel.getLocalTargetLabel("cc-compiler-ppc-compiler") + "',",
" toolchain = '" + crosstoolLabel.getSamePackageLabel("cc-compiler-ppc-compiler") + "',",
" target_compatible_with = [':mock_value'],",
")");
}
Expand Down

0 comments on commit e0180e1

Please sign in to comment.