Skip to content

Commit

Permalink
Delete package level default_copts.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 494234378
Change-Id: I4ff8b5d52e4dc2afaf6b5bf971a3e6bc61378344
  • Loading branch information
buildbreaker2021 authored and copybara-github committed Dec 9, 2022
1 parent 75e0f7d commit 0cc796a
Show file tree
Hide file tree
Showing 17 changed files with 6 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,6 @@ public String getPackageDefaultDeprecation() {
return ruleAttributes.getPackageDefaultDeprecation();
}

@Override
public ImmutableList<String> getPackageDefaultCopts() {
return ruleAttributes.getPackageDefaultCopts();
}

@Override
public boolean has(String attrName) {
if (ruleAttributes.has(attrName)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,6 @@ public String getPackageDefaultDeprecation() {
return rule.getPackage().getDefaultDeprecation();
}

@Override
public ImmutableList<String> getPackageDefaultCopts() {
return rule.getPackage().getDefaultCopts();
}

@Override
public final void visitAllLabels(BiConsumer<Attribute, Label> consumer) {
visitLabels(DependencyFilter.ALL_DEPS, consumer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.packages;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
Expand Down Expand Up @@ -148,5 +147,4 @@ default <T> T getOrDefault(String attributeName, Type<T> type, T defaultValue) {

String getPackageDefaultDeprecation();

ImmutableList<String> getPackageDefaultCopts();
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package com.google.devtools.build.lib.packages;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
Expand Down Expand Up @@ -113,11 +112,6 @@ public String getPackageDefaultDeprecation() {
return delegate.getPackageDefaultDeprecation();
}

@Override
public ImmutableList<String> getPackageDefaultCopts() {
return delegate.getPackageDefaultCopts();
}

@Override
public boolean has(String attrName) {
return delegate.has(attrName);
Expand Down
24 changes: 0 additions & 24 deletions src/main/java/com/google/devtools/build/lib/packages/Package.java
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,6 @@ public enum ConfigSettingVisibilityPolicy {
*/
private String defaultHdrsCheck;

/** Default copts for cc_* rules. The rules' individual copts will append to this value. */
private ImmutableList<String> defaultCopts;

/**
* The InputFile target corresponding to this package's BUILD file.
*/
Expand Down Expand Up @@ -469,11 +466,6 @@ private void finishInit(Builder builder) {
this.defaultVisibility = builder.defaultVisibility;
this.defaultVisibilitySet = builder.defaultVisibilitySet;
this.configSettingVisibilityPolicy = builder.configSettingVisibilityPolicy;
if (builder.defaultCopts == null) {
this.defaultCopts = ImmutableList.of();
} else {
this.defaultCopts = ImmutableList.copyOf(builder.defaultCopts);
}
this.buildFile = builder.buildFile;
this.containsErrors = builder.containsErrors;
this.failureDetail = builder.getFailureDetail();
Expand Down Expand Up @@ -779,14 +771,6 @@ public String getDefaultHdrsCheck() {
return defaultHdrsCheck != null ? defaultHdrsCheck : "strict";
}

/**
* Returns the default copts value, to which rules should append their
* specific copts.
*/
public ImmutableList<String> getDefaultCopts() {
return defaultCopts;
}

/**
* Returns whether the default header checking mode has been set or it is the
* default value.
Expand Down Expand Up @@ -1004,7 +988,6 @@ public boolean recordLoadedModules() {
private RuleVisibility defaultVisibility = ConstantRuleVisibility.PRIVATE;
private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy;
private boolean defaultVisibilitySet;
private List<String> defaultCopts = null;
private final List<String> features = new ArrayList<>();
private final List<Event> events = Lists.newArrayList();
private final List<Postable> posts = Lists.newArrayList();
Expand Down Expand Up @@ -1330,13 +1313,6 @@ public Builder setDefaultHdrsCheck(String hdrsCheck) {
return this;
}

/** Sets the default value of copts. Rule-level copts will append to this. */
@CanIgnoreReturnValue
public Builder setDefaultCopts(List<String> defaultCopts) {
this.defaultCopts = defaultCopts;
return this;
}

@CanIgnoreReturnValue
public Builder addFeatures(Iterable<String> features) {
Iterables.addAll(this.features, features);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,18 +262,7 @@ public ImmutableList<String> getCopts() {
Preconditions.checkNotNull(getNoCoptsPattern(ruleContext))));
}

return ImmutableList.<String>builder()
.addAll(CppHelper.getPackageCopts(ruleContext))
.addAll(CppHelper.getAttributeCopts(ruleContext))
.build();
}

// TODO(gnish): Delete this method once package default copts are gone.
// All of the package default copts will be included in rule attribute
// after the migration.
@StarlarkMethod(name = "unexpanded_package_copts", structField = true, documented = false)
public Sequence<String> getUnexpandedPackageCoptsForStarlark() {
return StarlarkList.immutableCopyOf(ruleContext.getRule().getPackage().getDefaultCopts());
return ImmutableList.copyOf(CppHelper.getAttributeCopts(ruleContext));
}

private boolean hasAttribute(String name, Type<?> type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,24 +129,6 @@ public Linkstamp createLinkstamp(
}
}

static class DefaultCoptsBuiltinComputedDefault extends ComputedDefault
implements NativeComputedDefaultApi {
@Override
public Object getDefault(AttributeMap rule) {
return rule.getPackageDefaultCopts();
}

@Override
public boolean resolvableWithRawAttributes() {
return true;
}
}

@StarlarkMethod(name = "default_copts_computed_default", documented = false)
public ComputedDefault getDefaultCoptsComputedDefault() {
return new DefaultCoptsBuiltinComputedDefault();
}

static class DefaultHdrsCheckBuiltinComputedDefault extends ComputedDefault
implements NativeComputedDefaultApi {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,6 @@ public static ImmutableList<String> getAttributeCopts(RuleContext ruleContext) {
return ImmutableList.copyOf(expandMakeVariables(ruleContext, attr, unexpanded));
}

// Called from CcCommon.
static ImmutableList<String> getPackageCopts(RuleContext ruleContext) {
List<String> unexpanded = ruleContext.getRule().getPackage().getDefaultCopts();
return ImmutableList.copyOf(expandMakeVariables(ruleContext, "copts", unexpanded));
}

/** Tokenizes and expands make variables. */
public static List<String> expandLinkopts(
RuleContext ruleContext, String attrName, Iterable<String> values) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ def cc_binary_impl(ctx, additional_linkopts):
actions = ctx.actions,
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
user_compile_flags = cc_helper.get_copts(ctx, common, feature_configuration, additional_make_variable_substitutions),
user_compile_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions),
defines = common.defines,
local_defines = common.local_defines + cc_helper.get_local_defines_for_runfiles_lookup(ctx),
loose_includes = common.loose_include_dirs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ cc_binary_attrs_with_aspects = {
"_stl": semantics.get_stl(),
"_cc_toolchain": attr.label(default = "@" + semantics.get_repo() + "//tools/cpp:current_cc_toolchain"),
"_cc_toolchain_type": attr.label(default = "@" + semantics.get_repo() + "//tools/cpp:toolchain_type"),
"_default_copts": attr.string_list(default = cc_internal.default_copts_computed_default()),
"_def_parser": semantics.get_def_parser(),
}

Expand Down
8 changes: 2 additions & 6 deletions src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -895,17 +895,13 @@ def _expand_make_variables_for_copts(ctx, tokenization, unexpanded_tokens, addit
tokens.append(_expand(ctx, token, additional_make_variable_substitutions))
return tokens

def _get_copts(ctx, common, feature_configuration, additional_make_variable_substitutions):
def _get_copts(ctx, feature_configuration, additional_make_variable_substitutions):
if not hasattr(ctx.attr, "copts"):
fail("could not find rule attribute named: 'copts'")
package_copts = []
if common:
package_copts = common.unexpanded_package_copts
attribute_copts = ctx.attr.copts
tokenization = not (cc_common.is_enabled(feature_configuration = feature_configuration, feature_name = "no_copts_tokenization") or "no_copts_tokenization" in ctx.features)
expanded_package_copts = _expand_make_variables_for_copts(ctx, tokenization, package_copts, additional_make_variable_substitutions)
expanded_attribute_copts = _expand_make_variables_for_copts(ctx, tokenization, attribute_copts, additional_make_variable_substitutions)
return expanded_package_copts + expanded_attribute_copts
return expanded_attribute_copts

def _get_expanded_env(ctx, additional_make_variable_substitutions):
if not hasattr(ctx.attr, "env"):
Expand Down
3 changes: 1 addition & 2 deletions src/main/starlark/builtins_bzl/common/cc/cc_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def _cc_library_impl(ctx):
name = ctx.label.name,
cc_toolchain = cc_toolchain,
feature_configuration = feature_configuration,
user_compile_flags = cc_helper.get_copts(ctx, common, feature_configuration, additional_make_variable_substitutions),
user_compile_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions),
defines = common.defines,
local_defines = common.local_defines + cc_helper.get_local_defines_for_runfiles_lookup(ctx),
loose_includes = common.loose_include_dirs,
Expand Down Expand Up @@ -584,7 +584,6 @@ attrs = {
"includes": attr.string_list(),
"defines": attr.string_list(),
"copts": attr.string_list(),
"_default_copts": attr.string_list(default = cc_internal.default_copts_computed_default()),
"hdrs_check": attr.string(default = cc_internal.default_hdrs_check_computed_default()),
"local_defines": attr.string_list(),
"deps": attr.label_list(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
import static com.google.devtools.build.lib.packages.Type.STRING;
import static com.google.devtools.build.lib.packages.Type.STRING_LIST;

import com.google.auto.value.AutoValue;
import com.google.common.base.Function;
Expand Down Expand Up @@ -48,7 +47,6 @@
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
import com.google.devtools.build.lib.packages.Attribute.LabelListLateBoundDefault;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
Expand Down Expand Up @@ -443,17 +441,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
public static final ComputedAttributeAspect COMPUTED_ATTRIBUTE_ASPECT =
new ComputedAttributeAspect();
private static final AspectDefinition COMPUTED_ATTRIBUTE_ASPECT_DEFINITION =
new AspectDefinition.Builder(COMPUTED_ATTRIBUTE_ASPECT)
.add(
attr("$default_copts", STRING_LIST)
.value(
new ComputedDefault() {
@Override
public Object getDefault(AttributeMap rule) {
return rule.getPackageDefaultCopts();
}
}))
.build();
new AspectDefinition.Builder(COMPUTED_ATTRIBUTE_ASPECT).build();

/** An aspect that defines its own computed default attribute. */
public static class ComputedAttributeAspect extends BaseAspect {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ public void testRegressionBug1686119() {
// Fileset doesn't exist in Bazel.
}

@Override
@Test
public void testDefaultCopts() {
// There's no default_copts attribute in Bazel.
}

@Override
@Test
public void testHdrsCheck() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1472,12 +1472,6 @@ public void testHdrsCheck() throws Exception {
assertThat(eval("attr('hdrs_check', 'loose', //x:all)")).isEqualTo(eval("//x:b"));
}

@Test
public void testDefaultCopts() throws Exception {
writeFile("x/BUILD", "package(default_copts=['-a'])", "cc_library(name='a')");
assertThat(eval("attr('$default_copts', '\\[-a\\]', //x:all)")).isEqualTo(eval("//x:a"));
}

@Test
public void testTestSuiteWithFile() throws Exception {
// Note that test_suite does not restrict the set of targets that can appear here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,9 +608,6 @@ public void testLabelsOperator() {}
@Test
public void testEqualityOfOrderedThreadSafeImmutableSet() {}

@Override
public void testDefaultCopts() {}

@Override
public void testHdrsCheck() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
Expand Down Expand Up @@ -137,11 +136,6 @@ public String getPackageDefaultDeprecation() {
return "???";
}

@Override
public ImmutableList<String> getPackageDefaultCopts() {
return ImmutableList.of();
}

public static FakeAttributeMapper empty() {
return builder().build();
}
Expand Down

0 comments on commit 0cc796a

Please sign in to comment.