Skip to content

Commit

Permalink
Flip incompatible_disallow_struct_provider_syntax
Browse files Browse the repository at this point in the history
Downstream tests: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4163

rules_jvm_external, rules_android_ndk, rules_python are failing because of Bazel 7 builtins/common/python/py_executable_bazel.bzl:103
All these failures are fixed by consistently using rules_python.

Needs: #23829
PiperOrigin-RevId: 681450758
Change-Id: I7580968b2d3b1460cc6a610a93110eaea2fb4f6c
  • Loading branch information
comius authored and copybara-github committed Oct 2, 2024
1 parent 401ed58 commit b9999f4
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ public final class BuildLanguageOptions extends OptionsBase {

@Option(
name = "incompatible_disallow_struct_provider_syntax",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
Expand Down Expand Up @@ -960,7 +960,7 @@ public StarlarkSemantics toStarlarkSemantics() {
"-incompatible_disable_target_provider_fields";
public static final String INCOMPATIBLE_DISALLOW_EMPTY_GLOB = "-incompatible_disallow_empty_glob";
public static final String INCOMPATIBLE_DISALLOW_STRUCT_PROVIDER_SYNTAX =
"-incompatible_disallow_struct_provider_syntax";
"+incompatible_disallow_struct_provider_syntax";
public static final String INCOMPATIBLE_PACKAGE_GROUP_HAS_PUBLIC_SYNTAX =
FlagConstants.INCOMPATIBLE_PACKAGE_GROUP_HAS_PUBLIC_SYNTAX;
public static final String INCOMPATIBLE_FIX_PACKAGE_GROUP_REPOROOT_SYNTAX =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void testCatastrophicAnalysisErrorAspect_keepGoing_noCrashCatastrophicErr
"""
def _impl(target, ctx):
print("This aspect does nothing")
return struct()
return []
MyAspect = aspect(implementation = _impl)
""");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,9 +774,9 @@ private void writeAspectDefinition(String aspectPackage, String extraDep) throws
write(
aspectPackage + "/aspect.bzl",
"def _aspect_impl(target, ctx):",
" return struct()",
" return []",
"def _rule_impl(ctx):",
" return struct()",
" return []",
"MyAspect = aspect(",
" implementation=_aspect_impl,",
" attr_aspects=['deps'],",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void testAspectArtifactSharesPrefixWithTargetArtifact(
"""
def _aspect_impl(target, ctx):
if not getattr(ctx.rule.attr, "outs", None):
return struct(output_groups = {})
return [OutputGroupInfo()]
conflict_outputs = list()
for out in ctx.rule.attr.outs:
if out.name[1:] == ".bad":
Expand Down Expand Up @@ -271,7 +271,7 @@ public void testAspectArtifactPrefix(
"""
def _aspect_impl(target, ctx):
if not getattr(ctx.rule.attr, "outs", None):
return struct(output_groups = {})
return [OutputGroupInfo()]
conflict_outputs = list()
for out in ctx.rule.attr.outs:
if out.name[1:] == ".bad":
Expand Down Expand Up @@ -501,7 +501,7 @@ public void testConflictErrorAndUnfinishedAspectAnalysis_mergedAnalysisExecution
"""
def _aspect_impl(target, ctx):
if not getattr(ctx.rule.attr, "outs", None):
return struct(output_groups = {})
return [OutputGroupInfo()]
conflict_outputs = list()
for out in ctx.rule.attr.outs:
if out.name[1:] == ".bad":
Expand Down Expand Up @@ -713,7 +713,7 @@ public void testConflictAfterNullBuild(@TestParameter boolean keepGoing) throws
"""
def _aspect_impl(target, ctx):
if not getattr(ctx.rule.attr, "outs", None):
return struct(output_groups = {})
return [OutputGroupInfo()]
conflict_outputs = list()
for out in ctx.rule.attr.outs:
if out.name[1:] == ".bad":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ private void writeStarlarkDefinedRuleClassBzlFile() throws java.io.IOException {
"""
def custom_rule_impl(ctx):
ftb = depset(ctx.attr._secret_labels)
return struct(runfiles = ctx.runfiles(), files = ftb)
return DefaultInfo(runfiles = ctx.runfiles(), files = ftb)
def secret_labels_func(prefix, suffix):
return [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ public void getStrictLabelAspectKeys() throws Exception {
"test/aspect.bzl",
"""
def _aspect_impl(target, ctx):
return struct()
return []
def _rule_impl(ctx):
return struct()
return []
MyAspect = aspect(
implementation = _aspect_impl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ public void testCannotSpecifyRunfilesWithDataOrDefaultRunfiles_struct() throws E
"""
def custom_rule_impl(ctx):
rf = ctx.runfiles()
return struct(runfiles = rf, default_runfiles = rf)
return DefaultInfo(runfiles = rf, default_runfiles = rf)
custom_rule = rule(implementation = custom_rule_impl)
""");
Expand Down Expand Up @@ -3563,7 +3563,7 @@ public void testDisallowStructProviderSyntax() throws Exception {
"test/starlark/extension.bzl",
"""
def custom_rule_impl(ctx):
return struct()
return struct() # intentional
custom_rule = rule(implementation = custom_rule_impl)
""");
Expand Down Expand Up @@ -3597,7 +3597,7 @@ def _my_rule_impl(ctx):
print(ctx.attr.dep.my_info)
def _dep_rule_impl(ctx):
my_info = MyProvider(foo = 'bar')
return struct(my_info = my_info, providers = [my_info])
return [my_info]
my_rule = rule(
implementation = _my_rule_impl,
attrs = {
Expand Down Expand Up @@ -3639,7 +3639,7 @@ def _my_rule_impl(ctx):
print(ctx.attr.dep.actions)
def _dep_rule_impl(ctx):
my_info = MyProvider(foo = 'bar')
return struct(my_info = my_info, providers = [my_info])
return [my_info]
my_rule = rule(
implementation = _my_rule_impl,
attrs = {
Expand Down Expand Up @@ -3673,7 +3673,7 @@ def _my_rule_impl(ctx):
print(ctx.attr.dep.my_info)
def _dep_rule_impl(ctx):
my_info = MyProvider(foo = 'bar')
return struct(my_info = my_info, providers = [my_info])
return struct(my_info = my_info, providers = [my_info]) # intentional
my_rule = rule(
implementation = _my_rule_impl,
attrs = {
Expand Down Expand Up @@ -3733,7 +3733,7 @@ def _my_rule_impl(ctx):
ctx.actions.run_shell(outputs=[exe], command='touch exe')
runfile = ctx.actions.declare_file('rrr')
ctx.actions.run_shell(outputs=[runfile], command='touch rrr')
return struct(executable = exe, default_runfiles = ctx.runfiles(files = [runfile]))
return DefaultInfo(executable = exe, default_runfiles = ctx.runfiles(files = [runfile]))
my_rule = rule(implementation = _my_rule_impl, executable = True)
""");
scratch.file(
Expand Down Expand Up @@ -4404,7 +4404,7 @@ public void topLevelAspectOnTargetWithNonIdempotentRuleTransition() throws Excep
"""
def _impl(target, ctx):
print('This aspect does nothing')
return struct()
return []
MyAspect = aspect(implementation=_impl)
""");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3329,9 +3329,9 @@ def f(ctx):
"r/def.bzl",
"""
load(":create.bzl", "create")
Info = provider()
def f(ctx):
return struct(value = "NEW")
return Info(value = "NEW")
r = create(f)
""");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1906,7 +1906,7 @@ def symlink_impl(ctx):
'symlink_' + f.short_path: f
for f in ctx.files.symlink
}
return struct(
return DefaultInfo(
runfiles = ctx.runfiles(
symlinks=symlinks,
)
Expand Down Expand Up @@ -2007,7 +2007,7 @@ def root_symlink_impl(ctx):
'root_symlink_' + f.short_path: f
for f in ctx.files.root_symlink
}
return struct(
return DefaultInfo(
runfiles = ctx.runfiles(
root_symlinks=root_symlinks,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.StarlarkInfo;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
Expand Down Expand Up @@ -195,7 +196,7 @@ def _genfile_impl(ctx):
implementation = _genfile_impl,
outputs = {"my_output": "%{name}.txt"},
)
CheckInfo = provider()
def _check_impl(ctx):
source_file = ctx.attr.srcs[0].files.to_list()[0]
generated_file = ctx.attr.srcs[1].files.to_list()[0]
Expand All @@ -213,7 +214,7 @@ def _check_impl(ctx):
"source_root": source_file.root,
"generated_root": generated_file.root,
}
return struct(**prepare_params(objects))
return CheckInfo(**prepare_params(objects))
check = rule(
implementation = _check_impl,
Expand Down Expand Up @@ -329,13 +330,14 @@ public void testStringRepresentations_select() throws Exception {
public void testStringRepresentations_ruleContext() throws Exception {
generateFilesToTestStrings();
ConfiguredTarget target = getConfiguredTarget("//test/starlark:check");
StarlarkInfo checkInfo = getStarlarkProvider(target, "CheckInfo");

for (String suffix : SUFFIXES) {
assertThat(target.get("rule_ctx" + suffix))
assertThat(checkInfo.getValue("rule_ctx" + suffix))
.isEqualTo("<rule context for //test/starlark:check>");
assertThat(target.get("aspect_ctx" + suffix))
assertThat(checkInfo.getValue("aspect_ctx" + suffix))
.isEqualTo("<aspect context for //test/starlark:bar>");
assertThat(target.get("aspect_ctx.rule" + suffix))
assertThat(checkInfo.getValue("aspect_ctx.rule" + suffix))
.isEqualTo("<rule collection for //test/starlark:bar>");
}
}
Expand All @@ -344,11 +346,12 @@ public void testStringRepresentations_ruleContext() throws Exception {
public void testStringRepresentations_files() throws Exception {
generateFilesToTestStrings();
ConfiguredTarget target = getConfiguredTarget("//test/starlark:check");
StarlarkInfo checkInfo = getStarlarkProvider(target, "CheckInfo");

for (String suffix : SUFFIXES) {
assertThat(target.get("source_file" + suffix))
assertThat(checkInfo.getValue("source_file" + suffix))
.isEqualTo("<source file test/starlark/input.txt>");
assertThat(target.get("generated_file" + suffix))
assertThat(checkInfo.getValue("generated_file" + suffix))
.isEqualTo("<generated file test/starlark/output.txt>");
}
}
Expand All @@ -357,10 +360,11 @@ public void testStringRepresentations_files() throws Exception {
public void testStringRepresentations_root() throws Exception {
generateFilesToTestStrings();
ConfiguredTarget target = getConfiguredTarget("//test/starlark:check");
StarlarkInfo checkInfo = getStarlarkProvider(target, "CheckInfo");

for (String suffix : SUFFIXES) {
assertThat(target.get("source_root" + suffix)).isEqualTo("<source root>");
assertThat(target.get("generated_root" + suffix)).isEqualTo("<derived root>");
assertThat(checkInfo.getValue("source_root" + suffix)).isEqualTo("<source root>");
assertThat(checkInfo.getValue("generated_root" + suffix)).isEqualTo("<derived root>");
}
}

Expand Down Expand Up @@ -396,16 +400,17 @@ public void testStringRepresentations_attr() throws Exception {
public void testStringRepresentations_targets() throws Exception {
generateFilesToTestStrings();
ConfiguredTarget target = getConfiguredTarget("//test/starlark:check");
StarlarkInfo checkInfo = getStarlarkProvider(target, "CheckInfo");

for (String suffix : SUFFIXES) {
assertThat(target.get("target" + suffix)).isEqualTo("<target //test/starlark:foo>");
assertThat(target.get("input_target" + suffix))
assertThat(checkInfo.getValue("target" + suffix)).isEqualTo("<target //test/starlark:foo>");
assertThat(checkInfo.getValue("input_target" + suffix))
.isEqualTo("<input file target //test/starlark:input.txt>");
assertThat(target.get("output_target" + suffix))
assertThat(checkInfo.getValue("output_target" + suffix))
.isEqualTo("<output file target //test/starlark:output.txt>");
assertThat(target.get("alias_target" + suffix))
assertThat(checkInfo.getValue("alias_target" + suffix))
.isEqualTo("<alias target //test/starlark:foobar of //test/starlark:foo>");
assertThat(target.get("aspect_target" + suffix))
assertThat(checkInfo.getValue("aspect_target" + suffix))
.isEqualTo("<merged target //test/starlark:bar>");
}
}
Expand Down
32 changes: 16 additions & 16 deletions src/test/shell/bazel/bazel_java_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ def _impl(ctx):
strict_deps = "ERROR",
java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],
)
return struct(
files = depset([output_jar]),
providers = [compilation_provider]
)
return [
DefaultInfo(files = depset([output_jar])),
compilation_provider,
]
java_custom_library = rule(
implementation = _impl,
Expand Down Expand Up @@ -426,10 +426,10 @@ def _impl(ctx):
strict_deps = "ERROR",
java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],
)
return struct(
files = depset([output_jar]),
providers = [compilation_provider]
)
return [
DefaultInfo(files = depset([output_jar])),
compilation_provider
]
java_custom_library = rule(
implementation = _impl,
Expand Down Expand Up @@ -497,10 +497,10 @@ def _impl(ctx):
strict_deps = "ERROR",
java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],
)
return struct(
files = depset([output_jar]),
providers = [compilation_provider]
)
return [
DefaultInfo(files = depset([output_jar])),
compilation_provider,
]
java_custom_library = rule(
implementation = _impl,
Expand Down Expand Up @@ -1405,10 +1405,10 @@ def _impl(ctx):
print(final_provider.outputs.jars[0].class_jar)
print(final_provider.outputs.jars[1].class_jar)
return struct(
files = depset([compiled_jar, imported_jar]),
providers = [final_provider]
)
return [
DefaultInfo(files = depset([compiled_jar, imported_jar])),
final_provider,
]
java_custom_library = rule(
implementation = _impl,
Expand Down
4 changes: 2 additions & 2 deletions src/test/shell/bazel/external_starlark_load_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ EOF

cat > rule.bzl <<EOF
def test_aspect_impl(target, ctx):
return struct()
return []
test_aspect = aspect(
attrs = {
Expand All @@ -243,7 +243,7 @@ test_aspect = aspect(
)
def test_rule_impl(ctx):
return struct()
return []
test_rule = rule(
attrs = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ function test_downloads_minimal_bep_partially_failed_aspect() {
cat > semifailingaspect.bzl <<'EOF'
def _semifailing_aspect_impl(target, ctx):
if not ctx.rule.attr.outs:
return struct(output_groups = {})
return [OutputGroupInfo()]
bad_outputs = list()
good_outputs = list()
for out in ctx.rule.attr.outs:
Expand Down
Loading

0 comments on commit b9999f4

Please sign in to comment.