Add --incompatible_disable_third_party_license_checking. This flag makes all license-related BUILD syntax no-ops. After this flag is permanently turned on in Bazel, we can start stripping out the syntax. This is unfortunately complex because it has to coherently interplay with the related flag --check_third_party_targets_have_licenses. See #7444 and #7553. PiperOrigin-RevId: 235779781
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index f24fb4d..56ce67b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -49,6 +49,7 @@ import com.google.devtools.build.lib.packages.OutputFile; import com.google.devtools.build.lib.packages.Rule; 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.RuleErrorConsumer; import com.google.devtools.build.lib.packages.RuleTransitionFactory; @@ -261,6 +262,9 @@ (BuildOptions options) -> ActionEnvironment.EMPTY; private ConstraintSemantics constraintSemantics = new ConstraintSemantics(); + private ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy = + ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE; + public Builder addWorkspaceFilePrefix(String contents) { defaultWorkspaceFilePrefix.append(contents); return this; @@ -385,6 +389,15 @@ } /** + * Sets the policy for checking if third_party rules declare <code>licenses()</code>. See {@link + * #thirdPartyLicenseExistencePolicy} for the default value. + */ + 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. * @@ -486,6 +499,7 @@ 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); @@ -520,7 +534,8 @@ skylarkBootstraps.build(), ImmutableSet.copyOf(reservedActionMnemonics), actionEnvironmentProvider, - constraintSemantics); + constraintSemantics, + thirdPartyLicenseExistencePolicy); } @Override @@ -609,6 +624,8 @@ private final ConstraintSemantics constraintSemantics; + private final ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy; + private ConfiguredRuleClassProvider( Label preludeLabel, String runfilesPrefix, @@ -629,7 +646,8 @@ ImmutableList<Bootstrap> skylarkBootstraps, ImmutableSet<String> reservedActionMnemonics, BuildConfiguration.ActionEnvironmentProvider actionEnvironmentProvider, - ConstraintSemantics constraintSemantics) { + ConstraintSemantics constraintSemantics, + ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy) { this.preludeLabel = preludeLabel; this.runfilesPrefix = runfilesPrefix; this.toolsRepository = toolsRepository; @@ -650,6 +668,7 @@ this.actionEnvironmentProvider = actionEnvironmentProvider; this.configurationFragmentMap = createFragmentMap(configurationFragmentFactories); this.constraintSemantics = constraintSemantics; + this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy; } public PrerequisiteValidator getPrerequisiteValidator() { @@ -841,6 +860,11 @@ return constraintSemantics; } + @Override + public ThirdPartyLicenseExistencePolicy getThirdPartyLicenseExistencePolicy() { + return thirdPartyLicenseExistencePolicy; + } + /** Returns all skylark objects in global scope for this RuleClassProvider. */ public Map<String, Object> getTransitiveGlobalBindings() { return globals.getTransitiveBindings();
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/LicenseCheckingModule.java b/src/main/java/com/google/devtools/build/lib/bazel/LicenseCheckingModule.java index 869b9a0..6cb5025 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/LicenseCheckingModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/LicenseCheckingModule.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.profiler.ProfilePhase; @@ -56,7 +57,11 @@ */ public class LicenseCheckingModule extends BlazeModule { - private boolean shouldCheckLicenses(BuildOptions buildOptions) { + protected boolean shouldCheckLicenses(BuildRequest request, BuildOptions buildOptions) { + if (request.getOptions(StarlarkSemanticsOptions.class) + .incompatibleDisableThirdPartyLicenseChecking) { + return false; + } return buildOptions.get(BuildConfiguration.Options.class).checkLicenses; } @@ -71,7 +76,7 @@ // We check licenses if the first target configuration has license checking enabled. Right // now, it is not possible to have multiple target configurations with different settings // for this flag, which allows us to take this short cut. - if (shouldCheckLicenses(buildOptions)) { + if (shouldCheckLicenses(request, buildOptions)) { Profiler.instance().markPhase(ProfilePhase.LICENSE); try (SilentCloseable c = Profiler.instance().profile("validateLicensingForTargets")) { validateLicensingForTargets(env, configuredTargets, request.getKeepGoing());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index 223b932..dea4426 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -187,6 +187,9 @@ public static ConfiguredRuleClassProvider create() { ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); builder.setToolsRepository(TOOLS_REPOSITORY); + // TODO(gregce): uncomment the below line in the same change that retires + // --incompatible_disable_third_party_license_checking. See the flag's comments for details. + // builder.setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePolicy.NEVER_CHECK); setup(builder); return builder.build(); }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index a6f55ec..290883b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.License.DistributionType; +import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy; import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; import com.google.devtools.build.lib.skyframe.serialization.SerializationContext; @@ -837,6 +838,9 @@ private final List<String> registeredExecutionPlatforms = new ArrayList<>(); private final List<String> registeredToolchains = new ArrayList<>(); + private ThirdPartyLicenseExistencePolicy thirdPartyLicenceExistencePolicy = + ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE; + /** * True iff the "package" function has already been called in this package. */ @@ -1016,6 +1020,15 @@ return this; } + public Builder setThirdPartyLicenceExistencePolicy(ThirdPartyLicenseExistencePolicy policy) { + this.thirdPartyLicenceExistencePolicy = policy; + return this; + } + + public ThirdPartyLicenseExistencePolicy getThirdPartyLicenseExistencePolicy() { + return thirdPartyLicenceExistencePolicy; + } + /** * Returns whether the "package" function has been called yet */
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 5887300..a7d83c6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Globber.BadGlobException; import com.google.devtools.build.lib.packages.License.DistributionType; +import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy; import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap; import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; @@ -735,7 +736,23 @@ String.format("licenses for exported file '%s' declared twice", inputFile.getName())); } - if (env.getSemantics().checkThirdPartyTargetsHaveLicenses() + + // See if we should check third-party licenses: first checking for any hard-coded policy, + // then falling back to user-settable flags. + boolean checkLicenses; + if (pkgBuilder.getThirdPartyLicenseExistencePolicy() + == ThirdPartyLicenseExistencePolicy.ALWAYS_CHECK) { + checkLicenses = true; + } else if (pkgBuilder.getThirdPartyLicenseExistencePolicy() + == ThirdPartyLicenseExistencePolicy.NEVER_CHECK) { + checkLicenses = false; + } else { + checkLicenses = + env.getSemantics().checkThirdPartyTargetsHaveLicenses() + && !env.getSemantics().incompatibleDisableThirdPartyLicenseChecking(); + } + + if (checkLicenses && license == null && !pkgBuilder.getDefaultLicense().isSpecified() && RuleClass.isThirdPartyPackage(pkgBuilder.getPackageIdentifier())) { @@ -1666,6 +1683,9 @@ pkgBuilder.setContainsErrors(); } + pkgBuilder.setThirdPartyLicenceExistencePolicy( + ruleClassProvider.getThirdPartyLicenseExistencePolicy()); + if (maxDirectoriesToEagerlyVisitInGlobbing == -2) { GlobPatternExtractor extractor = new GlobPatternExtractor(); extractor.visit(buildFileAST);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index 7f96fc4..a71cdaa 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -48,6 +48,7 @@ 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.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; @@ -671,6 +672,31 @@ 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 + * StarlarkSemanticsOptions#checkThirdPartyTargetsHaveLicenses} says. + */ + ALWAYS_CHECK, + + /** + * Never do this check, overriding whatever {@link + * StarlarkSemanticsOptions#checkThirdPartyTargetsHaveLicenses} says. + */ + NEVER_CHECK, + + /** Do whatever {@link StarlarkSemanticsOptions#checkThirdPartyTargetsHaveLicenses} says. */ + USER_CONTROLLABLE + } + + private ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy; + private final Map<String, Attribute> attributes = new LinkedHashMap<>(); private final Set<Label> requiredToolchains = new HashSet<>(); private boolean supportsPlatforms = true; @@ -826,6 +852,7 @@ ruleDefinitionEnvironmentHashCode, configurationFragmentPolicy.build(), supportsConstraintChecking, + thirdPartyLicenseExistencePolicy, requiredToolchains, supportsPlatforms, executionPlatformConstraintsAllowed, @@ -1026,6 +1053,11 @@ return this; } + public Builder setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePolicy policy) { + this.thirdPartyLicenseExistencePolicy = policy; + return this; + } + public Builder setValidityPredicate(PredicateWithMessage<Rule> predicate) { this.validityPredicate = predicate; return this; @@ -1497,6 +1529,8 @@ */ private final boolean supportsConstraintChecking; + private final ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy; + private final ImmutableSet<Label> requiredToolchains; private final boolean supportsPlatforms; private final ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed; @@ -1552,6 +1586,7 @@ String ruleDefinitionEnvironmentHashCode, ConfigurationFragmentPolicy configurationFragmentPolicy, boolean supportsConstraintChecking, + ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy, Set<Label> requiredToolchains, boolean supportsPlatforms, ExecutionPlatformConstraintsAllowed executionPlatformConstraintsAllowed, @@ -1591,6 +1626,7 @@ this.ignorePackageLicenses = ignorePackageLicenses; this.configurationFragmentPolicy = configurationFragmentPolicy; this.supportsConstraintChecking = supportsConstraintChecking; + this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy; this.requiredToolchains = ImmutableSet.copyOf(requiredToolchains); this.supportsPlatforms = supportsPlatforms; this.executionPlatformConstraintsAllowed = executionPlatformConstraintsAllowed; @@ -1835,7 +1871,17 @@ populateAttributeLocations(rule, ast); } checkForDuplicateLabels(rule, eventHandler); - if (checkThirdPartyRulesHaveLicenses) { + + boolean actuallyCheckLicense; + if (thirdPartyLicenseExistencePolicy == ThirdPartyLicenseExistencePolicy.ALWAYS_CHECK) { + actuallyCheckLicense = true; + } else if (thirdPartyLicenseExistencePolicy == ThirdPartyLicenseExistencePolicy.NEVER_CHECK) { + actuallyCheckLicense = false; + } else { + actuallyCheckLicense = checkThirdPartyRulesHaveLicenses; + } + + if (actuallyCheckLicense) { checkThirdPartyRuleHasLicense(rule, pkgBuilder, eventHandler); } checkForValidSizeAndTimeoutValues(rule, eventHandler);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java index e4cedd4..49b830e 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -18,6 +18,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.Mutability; @@ -103,4 +104,7 @@ * class. */ Map<String, Class<?>> getConfigurationFragmentMap(); + + /** Returns the policy on checking that third-party rules have licenses. */ + ThirdPartyLicenseExistencePolicy getThirdPartyLicenseExistencePolicy(); }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java index 1bbb1de..efea73a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
@@ -128,6 +128,20 @@ AttributesAndLocation generator = generatorAttributesForMacros(attributeValues, env, location, label); try { + // Examines --check_third_party_targets_have_licenses and + // --incompatible_disable_third_party_license_checking to see if we should check third_party + // targets for license existence. The latter flag overrides the former. + // + // Note that *both* flags are overridable by RuleClass.ThirdPartyLicenseEnforcementPolicy + // (which is checked in RuleClass). This lets Bazel and Blaze migrate away from license logic + // on independent timelines. See --incompatible_disable_third_party_license_checking comments + // for details. + boolean checkThirdPartyLicenses; + if (env == null || env.getSemantics().incompatibleDisableThirdPartyLicenseChecking()) { + checkThirdPartyLicenses = false; + } else { + checkThirdPartyLicenses = env.getSemantics().checkThirdPartyTargetsHaveLicenses(); + } return ruleClass.createRule( pkgBuilder, label, @@ -136,7 +150,7 @@ ast, generator.location, attributeContainer, - env == null ? false : env.getSemantics().checkThirdPartyTargetsHaveLicenses()); + checkThirdPartyLicenses); } catch (LabelSyntaxException | CannotPrecomputeDefaultsException e) { throw new RuleFactory.InvalidRuleException(ruleClass + " " + e.getMessage()); }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index 18883dc..3d639c0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -60,8 +60,10 @@ // <== Add new options here in alphabetic order ==> - // TODO(gregce): remove license checking completely from Bazel. aiuto@ is working on replacing - // this with a new and more useful model. + /** + * This can be overridden by {@link RuleClass.Builder.ThirdPartyLicenseExistencePolicy} and {@link + * #incompatibleDisableThirdPartyLicenseChecking}. + */ @Option( name = "check_third_party_targets_have_licenses", defaultValue = "true", @@ -244,6 +246,36 @@ help = "If set to true, disallow use of deprecated resource fields on the Objc provider.") public boolean incompatibleDisableObjcProviderResources; + // Once this migration is complete, instead of removing this flag we need to make it a no-op. + // This is because we'll need to keep it around for a while so Google's migration can complete + // after Bazel's. This is an example of Bazel's Google roots being methodically torn out: + // because this functionality was introduced for Google before Bazel existed, Google's + // dependency on it is deeper. We don't want this to add unnecessary baggage to Bazel or slow + // down Bazel's development. So this approach, while slightly awkward, relieves Bazel of + // Google's technical debt (which shouldn't be Bazel's problem). This means you as a Bazel + // user are getting better code than Google has! (for a while, at least) + // + // Track migration at https://github.com/bazelbuild/bazel/issues/7444. When we're ready to + // remove Bazel support, instead of removing the flag we should do these things: + // + // 1) BazelRuleClassProvider: set the third party license existence policy to NEVER_CHECK (see + // the related TODO(gregce) comment in that file). + // 2) Remove LicenseCheckingModule. + // 3) Remove --check_third_party_targets_have_licenses. + @Option( + name = "incompatible_disable_third_party_license_checking", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If true, disables all license checking logic. This overrides " + + "--check_third_party_targets_have_licenses") + public boolean incompatibleDisableThirdPartyLicenseChecking; + @Option( name = "incompatible_disallow_data_transition", defaultValue = "true", @@ -537,6 +569,7 @@ .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement) .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable) .incompatibleDepsetUnion(incompatibleDepsetUnion) + .incompatibleDisableThirdPartyLicenseChecking(incompatibleDisableThirdPartyLicenseChecking) .incompatibleDisableDeprecatedAttrParams(incompatibleDisableDeprecatedAttrParams) .incompatibleDisableObjcProviderResources(incompatibleDisableObjcProviderResources) .incompatibleDisallowDataTransition(incompatibleDisallowDataTransition)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index 775289e..b66b6ac 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -139,6 +139,8 @@ public abstract boolean incompatibleDepsetUnion(); + public abstract boolean incompatibleDisableThirdPartyLicenseChecking(); + public abstract boolean incompatibleDisableDeprecatedAttrParams(); public abstract boolean incompatibleDisableObjcProviderResources(); @@ -215,6 +217,7 @@ .incompatibleBzlDisallowLoadAfterStatement(false) .incompatibleDepsetIsNotIterable(false) .incompatibleDepsetUnion(false) + .incompatibleDisableThirdPartyLicenseChecking(false) .incompatibleDisableDeprecatedAttrParams(false) .incompatibleDisableObjcProviderResources(false) .incompatibleDisallowDataTransition(true) @@ -269,6 +272,8 @@ public abstract Builder incompatibleDepsetUnion(boolean value); + public abstract Builder incompatibleDisableThirdPartyLicenseChecking(boolean value); + public abstract Builder incompatibleDisableDeprecatedAttrParams(boolean value); public abstract Builder incompatibleRequireFeatureConfigurationForPic(boolean value);