Skip to content

Commit

Permalink
Enable --incompatible_no_output_attr_default by default
Browse files Browse the repository at this point in the history 
    Fixes bazelbuild/bazel#7950

    RELNOTES[INC]: `--incompatible_no_output_attr_default` is enabled by default.

    PiperOrigin-RevId: 243362752
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent b690a93 commit 4ae717e
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 26 deletions.
17 changes: 0 additions & 17 deletions ....bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,22 +523,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
+ "See https://github.com/bazelbuild/bazel/issues/7670 for details.")
public boolean incompatibleDoNotSplitLinkingCmdline;

@Option(
name = "incompatible_objc_framework_cleanup",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If enabled, use the post-cleanup mode for prebuilt frameworks. The cleanup changes "
+ "the objc provider API pertaining to frameworks. This change is expected to be "
+ "transparent to most users unless they write their own Starlark rules to handle "
+ "frameworks. See https://github.com/bazelbuild/bazel/issues/7944 for details.")
public boolean incompatibleObjcFrameworkCleanup;

/** Constructs a {@link StarlarkSemantics} object corresponding to this set of option values. */
public StarlarkSemantics toSkylarkSemantics() {
return StarlarkSemantics.builder()
Expand Down Expand Up @@ -574,7 +558,6 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
.incompatibleObjcFrameworkCleanup(incompatibleObjcFrameworkCleanup)
.incompatibleRemapMainRepo(incompatibleRemapMainRepo)
.incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
.incompatibleStaticNameResolutionInBuildFiles(incompatibleStaticNameResolutionInBuildFiles)
Expand Down
6 changes: 0 additions & 6 deletions ...azelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public enum FlagIdentifier {
INCOMPATIBLE_NO_OUTPUT_ATTR_DEFAULT(StarlarkSemantics::incompatibleNoOutputAttrDefault),
INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP(StarlarkSemantics::incompatibleNoTargetOutputGroup),
INCOMPATIBLE_NO_ATTR_LICENSE(StarlarkSemantics::incompatibleNoAttrLicense),
INCOMPATIBLE_OBJC_FRAMEWORK_CLEANUP(StarlarkSemantics::incompatibleObjcFrameworkCleanup),
NONE(null);

// Using a Function here makes the enum definitions far cleaner, and, since this is
Expand Down Expand Up @@ -171,8 +170,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract boolean incompatibleNoTransitiveLoads();

public abstract boolean incompatibleObjcFrameworkCleanup();

public abstract boolean incompatibleRemapMainRepo();

public abstract boolean incompatibleRemoveNativeMavenJar();
Expand Down Expand Up @@ -229,7 +226,6 @@ public static Builder builderWithDefaults() {
.incompatibleNoSupportToolsInActionInputs(false)
.incompatibleNoTargetOutputGroup(false)
.incompatibleNoTransitiveLoads(true)
.incompatibleObjcFrameworkCleanup(false)
.incompatibleRemapMainRepo(false)
.incompatibleRemoveNativeMavenJar(false)
.incompatibleStaticNameResolutionInBuildFiles(false)
Expand Down Expand Up @@ -301,8 +297,6 @@ public abstract static class Builder {

public abstract Builder incompatibleNoTransitiveLoads(boolean value);

public abstract Builder incompatibleObjcFrameworkCleanup(boolean value);

public abstract Builder incompatibleRemapMainRepo(boolean value);

public abstract Builder incompatibleRemoveNativeMavenJar(boolean value);
Expand Down
108 changes: 105 additions & 3 deletions ....bazel/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,37 @@
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.OutputJar;
import com.google.devtools.build.lib.testutil.TestConstants;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

/** Tests JavaInfo API for Skylark. */
@RunWith(JUnit4.class)
@RunWith(Parameterized.class)
public class JavaInfoSkylarkApiTest extends BuildViewTestCase {

private static final String HOST_JAVA_RUNTIME_LABEL =
TestConstants.TOOLS_REPOSITORY + "//tools/jdk:current_host_java_runtime";

@Parameters(name = "Use legacy JavaInfo constructor: {0}")
public static Iterable<Object[]> legacyJavaInfoConstructor() {
return ImmutableList.of(new Object[] {false}, new Object[] {true});
}

private final boolean legacyJavaInfoConstructor;

public JavaInfoSkylarkApiTest(boolean legacyJavaInfoConstructor) {
this.legacyJavaInfoConstructor = legacyJavaInfoConstructor;
}

@Before
public void setIncompatibleFlag() throws Exception {
if (legacyJavaInfoConstructor) {
setSkylarkSemanticsOptions("--noincompatible_disallow_legacy_javainfo");
}
}

@Test
public void buildHelperCreateJavaInfoWithOutputJarOnly() throws Exception {
ruleBuilder().build();
Expand Down Expand Up @@ -610,6 +630,11 @@ public void buildHelperCreateJavaInfoPlugins() throws Exception {

@Test
public void buildHelperCreateJavaInfoWithOutputJarAndStampJar() throws Exception {
if (legacyJavaInfoConstructor) {
// Unsupported mode, don't test this
return;
}

ruleBuilder().withStampJar().build();

scratch.file(
Expand Down Expand Up @@ -668,6 +693,58 @@ public void buildHelperCreateJavaInfoWithJdeps_javaRuleOutputJarsProvider() thro
assertThat(ruleOutputs.getJdeps().prettyPrint()).isEqualTo("foo/my_jdeps.pb");
}

@Test
public void testMixMatchNewAndLegacyArgsIsError() throws Exception {
ImmutableList.Builder<String> lines = ImmutableList.builder();
lines.add(
"result = provider()",
"def _impl(ctx):",
" output_jar = ctx.actions.declare_file('output_jar')",
" source_jar = ctx.actions.declare_file('source_jar')",
" javaInfo = JavaInfo(",
" output_jar = output_jar, ",
" source_jar = source_jar,",
" source_jars = [source_jar],",
" )",
" return [result(property = javaInfo)]",
"my_rule = rule(",
" implementation = _impl,",
")");
scratch.file("foo/extension.bzl", lines.build().toArray(new String[] {}));
checkError(
"foo",
"my_skylark_rule",
"Cannot use deprecated arguments at the same time",
"load(':extension.bzl', 'my_rule')",
"my_rule(name = 'my_skylark_rule')");
}

@Test
public void testIncompatibleDisallowLegacyJavaInfo() throws Exception {
setSkylarkSemanticsOptions("--incompatible_disallow_legacy_javainfo");
ImmutableList.Builder<String> lines = ImmutableList.builder();
lines.add(
"result = provider()",
"def _impl(ctx):",
" output_jar = ctx.actions.declare_file('output_jar')",
" source_jar = ctx.actions.declare_file('source_jar')",
" javaInfo = JavaInfo(",
" output_jar = output_jar,",
" source_jars = [source_jar],", // No longer allowed
" )",
" return [result(property = javaInfo)]",
"my_rule = rule(",
" implementation = _impl,",
")");
scratch.file("foo/extension.bzl", lines.build().toArray(new String[] {}));
checkError(
"foo",
"my_skylark_rule",
"Cannot use deprecated argument when --incompatible_disallow_legacy_javainfo is set. ",
"load(':extension.bzl', 'my_rule')",
"my_rule(name = 'my_skylark_rule')");
}

private RuleBuilder ruleBuilder() {
return new RuleBuilder();
}
Expand Down Expand Up @@ -698,6 +775,31 @@ private RuleBuilder withSourceFiles() {
return this;
}

private String[] legacyJavaInfo() {
assertThat(stampJar).isFalse();
return new String[] {
" javaInfo = JavaInfo(",
" output_jar = ctx.outputs.output_jar, ",
useIJar ? " use_ijar = True," : " use_ijar = False,",
neverLink ? " neverlink = True," : "",
" source_jars = ctx.files.source_jars,",
" sources = ctx.files.sources,",
" deps = dp,",
" runtime_deps = dp_runtime,",
" exports = dp_exports,",
" jdeps = ctx.file.jdeps,",
useIJar || sourceFiles ? " actions = ctx.actions," : "",
useIJar || sourceFiles
? " java_toolchain = ctx.attr._toolchain[java_common.JavaToolchainInfo],"
: "",
sourceFiles
? " host_javabase = ctx.attr._host_javabase[java_common.JavaRuntimeInfo],"
: "",
" )",
" return [result(property = javaInfo)]"
};
}

private String[] newJavaInfo() {
assertThat(useIJar && stampJar).isFalse();
ImmutableList.Builder<String> lines = ImmutableList.builder();
Expand Down Expand Up @@ -764,7 +866,7 @@ private void build() throws Exception {
" dp = [dep[java_common.provider] for dep in ctx.attr.dep]",
" dp_runtime = [dep[java_common.provider] for dep in ctx.attr.dep_runtime]",
" dp_exports = [dep[java_common.provider] for dep in ctx.attr.dep_exports]");
lines.add(newJavaInfo());
lines.add(legacyJavaInfoConstructor ? legacyJavaInfo() : newJavaInfo());
lines.add(
"my_rule = rule(",
" implementation = _impl,",
Expand Down

0 comments on commit 4ae717e

Please sign in to comment.