Skip to content

Commit

Permalink
Move --incompatible_disable_third_party_license_checking to the grave…
Browse files Browse the repository at this point in the history
…yard.

This completes the disabling of the check that all BUILD files in //third_party contained a legacy licenses(["notice"]) style declaration.

PiperOrigin-RevId: 471393024
Change-Id: Ifb3fa80f3364fce1eac7cb14de05b5d21436bf84
  • Loading branch information
aiuto authored and copybara-github committed Sep 1, 2022
1 parent dcc54d5 commit 0aa750b
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import com.google.devtools.build.lib.graph.Node;
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.RuleTransitionData;
import com.google.devtools.build.lib.starlarkbuildapi.core.Bootstrap;
Expand Down Expand Up @@ -173,9 +172,6 @@ public static final class Builder implements RuleDefinitionEnvironment {
private ConstraintSemantics<RuleContext> constraintSemantics =
new RuleContextConstraintSemantics();

private ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy =
ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE;

// TODO(b/192694287): Remove once we migrate all tests from the allowlist
@Nullable private Label networkAllowlistForTests;

Expand Down Expand Up @@ -381,16 +377,6 @@ public Builder setConstraintSemantics(ConstraintSemantics<RuleContext> constrain
return this;
}

/**
* Sets the policy for checking if third_party rules declare <code>licenses()</code>. See {@link
* #thirdPartyLicenseExistencePolicy} for the default value.
*/
@CanIgnoreReturnValue
public Builder setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePolicy policy) {
this.thirdPartyLicenseExistencePolicy = policy;
return this;
}

/**
* Adds a transition factory that produces a trimming transition to be run over all targets
* after other transitions.
Expand Down Expand Up @@ -509,7 +495,6 @@ private void commitRuleDefinition(Class<? extends RuleDefinition> definitionClas
RuleClass.Builder builder =
new RuleClass.Builder(metadata.name(), metadata.type(), false, ancestorClasses);
builder.factory(factory);
builder.setThirdPartyLicenseExistencePolicy(thirdPartyLicenseExistencePolicy);
RuleClass ruleClass = instance.build(builder, this);
ruleMap.put(definitionClass, ruleClass);
ruleClassMap.put(ruleClass.getName(), ruleClass);
Expand Down Expand Up @@ -604,7 +589,6 @@ public ConfiguredRuleClassProvider build() {
ImmutableSet.copyOf(reservedActionMnemonics),
actionEnvironmentProvider,
constraintSemantics,
thirdPartyLicenseExistencePolicy,
networkAllowlistForTests);
}

Expand Down Expand Up @@ -697,8 +681,6 @@ public Builder setNetworkAllowlistForTests(Label allowlist) {

private final ConstraintSemantics<RuleContext> constraintSemantics;

private final ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy;

// TODO(b/192694287): Remove once we migrate all tests from the allowlist
@Nullable private final Label networkAllowlistForTests;

Expand Down Expand Up @@ -726,7 +708,6 @@ private ConfiguredRuleClassProvider(
ImmutableSet<String> reservedActionMnemonics,
Function<BuildOptions, ActionEnvironment> actionEnvironmentProvider,
ConstraintSemantics<RuleContext> constraintSemantics,
ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy,
@Nullable Label networkAllowlistForTests) {
this.preludeLabel = preludeLabel;
this.runfilesPrefix = runfilesPrefix;
Expand All @@ -753,7 +734,6 @@ private ConfiguredRuleClassProvider(
this.actionEnvironmentProvider = actionEnvironmentProvider;
this.configurationFragmentMap = createFragmentMap(fragmentRegistry.getAllFragments());
this.constraintSemantics = constraintSemantics;
this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy;
this.networkAllowlistForTests = networkAllowlistForTests;
}

Expand Down Expand Up @@ -934,11 +914,6 @@ public ConstraintSemantics<RuleContext> getConstraintSemantics() {
return constraintSemantics;
}

@Override
public ThirdPartyLicenseExistencePolicy getThirdPartyLicenseExistencePolicy() {
return thirdPartyLicenseExistencePolicy;
}

@Override
public Optional<Label> getNetworkAllowlistForTests() {
return Optional.ofNullable(networkAllowlistForTests);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
import com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.rules.android.AarImportBaseRule;
import com.google.devtools.build.lib.rules.android.AndroidApplicationResourceInfo;
import com.google.devtools.build.lib.rules.android.AndroidAssetsInfo;
Expand Down Expand Up @@ -277,7 +276,6 @@ public static void setup(ConfiguredRuleClassProvider.Builder builder) {
builder.setBuiltinsBzlZipResource(
ResourceFileLoader.resolveResource(BazelRuleClassProvider.class, "builtins_bzl.zip"));
builder.setBuiltinsBzlPackagePathInSource("src/main/starlark/builtins_bzl");
builder.setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePolicy.NEVER_CHECK);

for (RuleSet ruleSet : RULE_SETS) {
ruleSet.init(builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,15 @@ public static final class AllCommandGraveyardOptions extends OptionsBase {
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.EXECUTION},
help = "No-op")
public boolean useEventBasedBuildCompletionStatus;

// Moved here 2022/08/29
@Option(
name = "incompatible_disable_third_party_license_checking",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.NO_OP},
help = "No-op")
public boolean incompatibleDisableThirdPartyLicenseChecking;
}

@Override
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/com/google/devtools/build/lib/packages/Package.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.Package.Builder.DefaultPackageSettings;
import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
Expand Down Expand Up @@ -1031,9 +1030,6 @@ public boolean recordLoadedModules() {
private final List<TargetPattern> registeredExecutionPlatforms = new ArrayList<>();
private final List<TargetPattern> registeredToolchains = new ArrayList<>();

private ThirdPartyLicenseExistencePolicy thirdPartyLicenceExistencePolicy =
ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE;

/**
* True iff the "package" function has already been called in this package.
*/
Expand Down Expand Up @@ -1283,16 +1279,6 @@ public Builder setWorkspaceName(String workspaceName) {
return this;
}

@CanIgnoreReturnValue
Builder setThirdPartyLicenceExistencePolicy(ThirdPartyLicenseExistencePolicy policy) {
this.thirdPartyLicenceExistencePolicy = policy;
return this;
}

ThirdPartyLicenseExistencePolicy getThirdPartyLicenseExistencePolicy() {
return thirdPartyLicenceExistencePolicy;
}

/**
* Returns whether the "package" function has been called yet
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -649,9 +649,6 @@ private void executeBuildFileImpl(
StarlarkSemantics semantics,
Globber globber)
throws InterruptedException {
pkgBuilder.setThirdPartyLicenceExistencePolicy(
ruleClassProvider.getThirdPartyLicenseExistencePolicy());

// TODO(adonovan): opt: don't precompute this value, which is rarely needed
// and can be derived from Package.loads (if available) on demand.
pkgBuilder.setStarlarkFileDependencies(transitiveClosureOfLabels(loadedModules));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.RuleFactory.AttributeValues;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
Expand Down Expand Up @@ -771,33 +770,6 @@ public String toString() {

private boolean supportsConstraintChecking = true;

/**
* The policy on whether Bazel should enforce that third_party rules declare <code>licenses().
* </code>. This is only intended for the migration of <a
* href="https://github.com/bazelbuild/bazel/issues/7444">GitHub #7444</a>. Our final end state
* is to have no license-related logic whatsoever. But that's going to take some time.
*/
public enum ThirdPartyLicenseExistencePolicy {
/**
* Always do this check, overriding whatever {@link
* BuildLanguageOptions#incompatibleDisableThirdPartyLicenseChecking} says.
*/
ALWAYS_CHECK,

/**
* Never do this check, overriding whatever {@link
* BuildLanguageOptions#incompatibleDisableThirdPartyLicenseChecking} says.
*/
NEVER_CHECK,

/**
* Do whatever {@link BuildLanguageOptions#incompatibleDisableThirdPartyLicenseChecking} says.
*/
USER_CONTROLLABLE
}

private ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy;

private final Map<String, Attribute> attributes = new LinkedHashMap<>();
private final Set<ToolchainTypeRequirement> toolchainTypes = new HashSet<>();
private ToolchainResolutionMode useToolchainResolution = ToolchainResolutionMode.INHERIT;
Expand Down Expand Up @@ -960,7 +932,6 @@ public RuleClass build(String name, String key) {
ruleDefinitionEnvironmentDigest,
configurationFragmentPolicy.build(),
supportsConstraintChecking,
thirdPartyLicenseExistencePolicy,
toolchainTypes,
useToolchainResolution,
executionPlatformConstraints,
Expand Down Expand Up @@ -1207,12 +1178,6 @@ public Builder factory(ConfiguredTargetFactory<?, ?, ?> factory) {
return this;
}

@CanIgnoreReturnValue
public Builder setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePolicy policy) {
this.thirdPartyLicenseExistencePolicy = policy;
return this;
}

@CanIgnoreReturnValue
public Builder setValidityPredicate(PredicateWithMessage<Rule> predicate) {
this.validityPredicate = predicate;
Expand Down Expand Up @@ -1759,8 +1724,6 @@ public Attribute.Builder<?> copy(String name) {
*/
private final boolean supportsConstraintChecking;

private final ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy;

private final ImmutableSet<ToolchainTypeRequirement> toolchainTypes;
private final ToolchainResolutionMode useToolchainResolution;
private final ImmutableSet<Label> executionPlatformConstraints;
Expand Down Expand Up @@ -1816,7 +1779,6 @@ public Attribute.Builder<?> copy(String name) {
@Nullable byte[] ruleDefinitionEnvironmentDigest,
ConfigurationFragmentPolicy configurationFragmentPolicy,
boolean supportsConstraintChecking,
ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy,
Set<ToolchainTypeRequirement> toolchainTypes,
ToolchainResolutionMode useToolchainResolution,
Set<Label> executionPlatformConstraints,
Expand Down Expand Up @@ -1856,7 +1818,6 @@ public Attribute.Builder<?> copy(String name) {
this.ignoreLicenses = ignoreLicenses;
this.configurationFragmentPolicy = configurationFragmentPolicy;
this.supportsConstraintChecking = supportsConstraintChecking;
this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy;
this.toolchainTypes = ImmutableSet.copyOf(toolchainTypes);
this.useToolchainResolution = useToolchainResolution;
this.executionPlatformConstraints = ImmutableSet.copyOf(executionPlatformConstraints);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.vfs.Root;
import java.util.Map;

Expand Down Expand Up @@ -109,7 +108,4 @@ public interface RuleClassProvider extends RuleDefinitionEnvironment {
* class.
*/
ImmutableMap<String, Class<?>> getConfigurationFragmentMap();

/** Returns the policy on checking that third-party rules have licenses. */
ThirdPartyLicenseExistencePolicy getThirdPartyLicenseExistencePolicy();
}
Original file line number Diff line number Diff line change
Expand Up @@ -397,23 +397,6 @@ public final class BuildLanguageOptions extends OptionsBase {
+ "https://github.com/bazelbuild/bazel/issues/9014 for details.")
public boolean incompatibleDisableTargetProviderFields;

// For Bazel, this flag is a no-op. Bazel doesn't support built-in third party license checking
// (see https://github.com/bazelbuild/bazel/issues/7444).
//
// For Blaze in Google, this flag is still needed to deprecate the logic that's already been
// removed from Bazel. That logic was introduced before Bazel existed, so Google's dependency on
// it is deeper. But we don't want that to add unnecessary baggage to Bazel or slow down Bazel's
// development. So this flag lets Blaze migrate on a slower timeline without blocking Bazel. This
// means you as a Bazel user are getting better code than Google has! (for a while, at least)
@Option(
name = "incompatible_disable_third_party_license_checking",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help = "If true, disables all license checking logic")
public boolean incompatibleDisableThirdPartyLicenseChecking;

@Option(
name = "incompatible_disallow_empty_glob",
defaultValue = "false",
Expand Down Expand Up @@ -669,9 +652,6 @@ public StarlarkSemantics toStarlarkSemantics() {
.setBool(
INCOMPATIBLE_DISABLE_TARGET_PROVIDER_FIELDS,
incompatibleDisableTargetProviderFields)
.setBool(
INCOMPATIBLE_DISABLE_THIRD_PARTY_LICENSE_CHECKING,
incompatibleDisableThirdPartyLicenseChecking)
.setBool(
INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS, incompatibleAlwaysCheckDepsetElements)
.setBool(INCOMPATIBLE_DISALLOW_EMPTY_GLOB, incompatibleDisallowEmptyGlob)
Expand Down Expand Up @@ -757,8 +737,6 @@ public StarlarkSemantics toStarlarkSemantics() {
"+incompatible_depset_for_libraries_to_link_getter";
public static final String INCOMPATIBLE_DISABLE_TARGET_PROVIDER_FIELDS =
"-incompatible_disable_target_provider_fields";
public static final String INCOMPATIBLE_DISABLE_THIRD_PARTY_LICENSE_CHECKING =
"+incompatible_disable_third_party_license_checking";
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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate.CannotPrecomputeDefaultsException;
import com.google.devtools.build.lib.packages.Attribute.ValidityPredicate;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory;
import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
Expand Down Expand Up @@ -1071,7 +1070,6 @@ private static RuleClass newRuleClass(
.requiresConfigurationFragments(allowedConfigurationFragments)
.build(),
supportsConstraintChecking,
ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE,
/*toolchainTypes=*/ ImmutableSet.of(),
/*useToolchainResolution=*/ ToolchainResolutionMode.ENABLED,
/* executionPlatformConstraints= */ ImmutableSet.of(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep
"--incompatible_always_check_depset_elements=" + rand.nextBoolean(),
"--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
"--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
"--incompatible_disable_third_party_license_checking=" + rand.nextBoolean(),
"--incompatible_disallow_empty_glob=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean(),
Expand Down Expand Up @@ -186,9 +185,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
rand.nextBoolean())
.setBool(
BuildLanguageOptions.INCOMPATIBLE_DISABLE_TARGET_PROVIDER_FIELDS, rand.nextBoolean())
.setBool(
BuildLanguageOptions.INCOMPATIBLE_DISABLE_THIRD_PARTY_LICENSE_CHECKING,
rand.nextBoolean())
.setBool(BuildLanguageOptions.INCOMPATIBLE_DISALLOW_EMPTY_GLOB, rand.nextBoolean())
.setBool(
BuildLanguageOptions.INCOMPATIBLE_DISALLOW_STRUCT_PROVIDER_SYNTAX, rand.nextBoolean())
Expand Down

0 comments on commit 0aa750b

Please sign in to comment.