Skip to content

Commit

Permalink
Don't crash when Starlark rules declare attributes named "licenses".
Browse files Browse the repository at this point in the history
The specific use case that triggers this is writing a rule with
a non-LICENSE-typed attribute named "licenses", then creating
a BUILD instance that doesn't explicitly set its value. See the test
for an example.

Fixes #7194.  Also fixes #6420 (comment),
which blocks the --incompatible_no_attr_license migration.

RELNOTES: Starlark rules can safely declare attributes named "licenses"
PiperOrigin-RevId: 235919628
  • Loading branch information
gregestren authored and copybara-github committed Feb 27, 2019
1 parent 1a90fe6 commit 82281ad
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -2230,7 +2230,9 @@ private static void checkForValidSizeAndTimeoutValues(Rule rule, EventHandler ev
*/
private static Object getAttributeNoncomputedDefaultValue(Attribute attr,
Package.Builder pkgBuilder) {
if (attr.getName().equals("licenses")) {
// Starlark rules may define their own "licenses" attributes with different types -
// we shouldn't trigger the special "licenses" on those cases.
if (attr.getName().equals("licenses") && attr.getType() == BuildType.LICENSE) {
return pkgBuilder.getDefaultLicense();
}
if (attr.getName().equals("distribs")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
import com.google.devtools.build.lib.packages.License;
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.License.LicenseParsingException;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.util.MockCcSupport;
import com.google.devtools.build.lib.syntax.Type;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.HashMap;
Expand Down Expand Up @@ -683,6 +686,28 @@ public void testTargetLicenseEquality() throws Exception {
.testEquals();
}

/** Regression fix for https://github.com/bazelbuild/bazel/issues/7194. */
@Test
public void testStarlarkLicensesAttributeCanUseUseCustomDefault() throws Exception {
scratch.file(
"foo/rules.bzl",
"def _myrule_impl(ctx):",
" return []",
"",
"myrule = rule(",
" implementation = _myrule_impl,",
" attrs = {",
" 'licenses': attr.string(default = 'custom_licenses_default'),",
" }",
")");

scratch.file("foo/BUILD", "load('//foo:rules.bzl', 'myrule')", "myrule(name = 'hi')");

assertThat(RawAttributeMapper.of((Rule) getTarget("//foo:hi")).get("licenses", Type.STRING))
.isEqualTo("custom_licenses_default");
assertNoEvents();
}

/**
* Gets the licenses of all targets that are in the transitive closure of a
* target. The result includes this target itself.
Expand Down

0 comments on commit 82281ad

Please sign in to comment.