Skip to content

Commit abe6667

Browse files
oquenchilcopybara-github
oquenchil
authored and
copybara-github
committed
Revert interface_deps back to implementation_deps
We decided to rename implementation_deps to interface_deps since this would be in line with the behavior of the exports attribute in java_library and proto_library. In turn, the attribute deps would behave like implementation_deps. This is an incompatible change and it didn’t consider the roll-out of interface_deps between different repositories where it would be a breaking change to use this behavior if an external repository dependency hasn’t been migrated. This was first reported in #14950. I thought I could fix this with some WORKSPACE magic where the incompatible change would be applied on a per repository basis but this isn’t feasible. The aesthetic/consistency benefit of using interface_deps rather than implementation_deps does not seem worth it. RELNOTES:Revert interface_deps back to implementation_deps after problem reported in. Use `buildozer 'rename deps implementation_deps' //...:%cc_library; buildozer 'rename interface_deps deps' //...:%cc_library` PiperOrigin-RevId: 464512974 Change-Id: Ib7c9854175835694a8bea523c347b1a1605ee777
1 parent a25ba11 commit abe6667

File tree

14 files changed

+93
-115
lines changed

14 files changed

+93
-115
lines changed

‎src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcLibraryRule.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,17 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
5050
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
5151
.add(attr("alwayslink", BOOLEAN))
5252
.override(attr("linkstatic", BOOLEAN).value(false))
53-
/*<!-- #BLAZE_RULE(cc_library).ATTRIBUTE(interface_deps) -->
54-
When <code>--experimental_cc_interface_deps</code> is True, the targets listed in deps will
55-
behave as implementation deps. Unlike with regular deps, the headers and include paths of
56-
implementation deps (and all their transitive deps) are only used for compilation of this
57-
library, and not libraries that depend on it. Libraries depended on as implementation deps
58-
are still linked in binary targets that depend on this library. The dependencies listed in
59-
interface_deps will continue having the same behavior as the old deps where the headers and
60-
include paths are propagated downstream.
53+
/*<!-- #BLAZE_RULE(cc_library).ATTRIBUTE(implementation_deps) -->
54+
The list of other libraries that the library target depends on. Unlike with
55+
<code>deps</code>, the headers and include paths of these libraries (and all their
56+
transitive deps) are only used for compilation of this library, and not libraries that
57+
depend on it. Libraries specified with <code>implementation_deps</code> are still linked in
58+
binary targets that depend on this library.
59+
<p>For now usage is limited to cc_libraries and guarded by the flag
60+
<code>--experimental_cc_implementation_deps</code>.</p>
6161
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
6262
.add(
63-
attr("interface_deps", LABEL_LIST)
63+
attr("implementation_deps", LABEL_LIST)
6464
.allowedFileTypes(FileTypeSet.NO_FILE)
6565
.mandatoryProviders(CcInfo.PROVIDER.id()))
6666
.advertiseStarlarkProvider(CcInfo.PROVIDER.id())

‎src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,13 @@ public boolean createEmptyArchive() {
171171
}
172172

173173
@Override
174-
public boolean shouldUseInterfaceDepsBehavior(RuleContext ruleContext) {
175-
boolean experimentalCcInterfaceDeps =
176-
ruleContext.getFragment(CppConfiguration.class).experimentalCcInterfaceDeps();
177-
if (!experimentalCcInterfaceDeps
178-
&& ruleContext.attributes().isAttributeValueExplicitlySpecified("interface_deps")) {
179-
ruleContext.attributeError("interface_deps", "requires --experimental_cc_interface_deps");
174+
public void checkCanUseImplementationDeps(RuleContext ruleContext) {
175+
boolean experimentalCcImplementationDeps =
176+
ruleContext.getFragment(CppConfiguration.class).experimentalCcImplementationDeps();
177+
if (!experimentalCcImplementationDeps
178+
&& ruleContext.attributes().isAttributeValueExplicitlySpecified("implementation_deps")) {
179+
ruleContext.attributeError(
180+
"implementation_deps", "requires --experimental_cc_implementation_deps");
180181
}
181-
return experimentalCcInterfaceDeps;
182182
}
183183
}

‎src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public static void init(
123123
return;
124124
}
125125

126-
boolean shouldUseInterfaceDepsBehavior = semantics.shouldUseInterfaceDepsBehavior(ruleContext);
126+
semantics.checkCanUseImplementationDeps(ruleContext);
127127

128128
final CcCommon common = new CcCommon(ruleContext);
129129
common.reportInvalidOptions(ruleContext);
@@ -158,18 +158,12 @@ public static void init(
158158
ImmutableList.Builder<CcCompilationContext> interfaceDeps = ImmutableList.builder();
159159
ImmutableList.Builder<CcCompilationContext> implementationDeps = ImmutableList.builder();
160160

161-
if (shouldUseInterfaceDepsBehavior) {
162-
interfaceDeps.addAll(
163-
CppHelper.getCompilationContextsFromDeps(
164-
ImmutableList.copyOf(ruleContext.getPrerequisites("interface_deps"))));
165-
implementationDeps.addAll(
166-
CppHelper.getCompilationContextsFromDeps(
167-
ImmutableList.copyOf(ruleContext.getPrerequisites("deps"))));
168-
} else {
169-
interfaceDeps.addAll(
170-
CppHelper.getCompilationContextsFromDeps(
171-
ImmutableList.copyOf(ruleContext.getPrerequisites("deps"))));
172-
}
161+
interfaceDeps.addAll(
162+
CppHelper.getCompilationContextsFromDeps(
163+
ImmutableList.copyOf(ruleContext.getPrerequisites("deps"))));
164+
implementationDeps.addAll(
165+
CppHelper.getCompilationContextsFromDeps(
166+
ImmutableList.copyOf(ruleContext.getPrerequisites("implementation_deps"))));
173167
interfaceDeps.add(CcCompilationHelper.getStlCcCompilationContext(ruleContext));
174168

175169
CcCompilationHelper compilationHelper =
@@ -212,7 +206,10 @@ public static void init(
212206
.fromCommon(ruleContext, common)
213207
.addCcLinkingContexts(
214208
CppHelper.getLinkingContextsFromDeps(
215-
ImmutableList.copyOf(ruleContext.getPrerequisites("interface_deps"))))
209+
ImmutableList.copyOf(ruleContext.getPrerequisites("deps"))))
210+
.addCcLinkingContexts(
211+
CppHelper.getLinkingContextsFromDeps(
212+
ImmutableList.copyOf(ruleContext.getPrerequisites("implementation_deps"))))
216213
.setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
217214
.setTestOrTestOnlyTarget(ruleContext.isTestOnlyTarget())
218215
.addLinkopts(common.getLinkopts())

‎src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -883,17 +883,17 @@ public boolean objcEnableBinaryStripping() {
883883
}
884884

885885
@StarlarkMethod(
886-
name = "experimental_cc_interface_deps",
886+
name = "experimental_cc_implementation_deps",
887887
documented = false,
888888
useStarlarkThread = true)
889-
public boolean experimentalCcInterfaceDepsForStarlark(StarlarkThread thread)
889+
public boolean experimentalCcImplementationDepsForStarlark(StarlarkThread thread)
890890
throws EvalException {
891891
CcModule.checkPrivateStarlarkificationAllowlist(thread);
892-
return experimentalCcInterfaceDeps();
892+
return experimentalCcImplementationDeps();
893893
}
894894

895-
public boolean experimentalCcInterfaceDeps() {
896-
return cppOptions.experimentalCcInterfaceDeps;
895+
public boolean experimentalCcImplementationDeps() {
896+
return cppOptions.experimentalCcImplementationDeps;
897897
}
898898

899899
public boolean getExperimentalCppCompileResourcesEstimation() {

‎src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,15 +1072,15 @@ public Label getPropellerOptimizeLabel() {
10721072
public boolean objcGenerateDotdFiles;
10731073

10741074
@Option(
1075-
name = "experimental_cc_interface_deps",
1075+
name = "experimental_cc_implementation_deps",
10761076
defaultValue = "false",
10771077
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
10781078
effectTags = {
10791079
OptionEffectTag.LOADING_AND_ANALYSIS,
10801080
},
10811081
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
1082-
help = "If enabled, cc_library targets can use attribute `interface_deps`.")
1083-
public boolean experimentalCcInterfaceDeps;
1082+
help = "If enabled, cc_library targets can use attribute `implementation_deps`.")
1083+
public boolean experimentalCcImplementationDeps;
10841084

10851085
@Option(
10861086
name = "experimental_link_static_libraries_once",
@@ -1200,7 +1200,7 @@ public FragmentOptions getHost() {
12001200
host.experimentalLinkStaticLibrariesOnce = experimentalLinkStaticLibrariesOnce;
12011201
host.experimentalEnableTargetExportCheck = experimentalEnableTargetExportCheck;
12021202
host.experimentalCcSharedLibraryDebug = experimentalCcSharedLibraryDebug;
1203-
host.experimentalCcInterfaceDeps = experimentalCcInterfaceDeps;
1203+
host.experimentalCcImplementationDeps = experimentalCcImplementationDeps;
12041204

12051205
host.coptList = coptListBuilder.addAll(hostCoptList).build();
12061206
host.cxxoptList = cxxoptListBuilder.addAll(hostCxxoptList).build();

‎src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public static ToolchainTypeRequirement ccToolchainTypeRequirement(RuleDefinition
110110
FileTypeSet.of(
111111
CPP_SOURCE, C_SOURCE, CPP_HEADER, ASSEMBLER_WITH_C_PREPROCESSOR, ASSEMBLER))
112112
.withSourceAttributes("srcs", "hdrs")
113-
.withDependencyAttributes("interface_deps", "deps", "data");
113+
.withDependencyAttributes("implementation_deps", "deps", "data");
114114

115115
/** Implicit outputs for cc_binary rules. */
116116
public static final SafeImplicitOutputsFunction CC_BINARY_STRIPPED =
@@ -487,9 +487,6 @@ public static ToolchainTypeRequirement ccToolchainTypeRequirement(RuleDefinition
487487
*/
488488
public static final String LEGACY_IS_CC_TEST_FEATURE_NAME = "legacy_is_cc_test";
489489

490-
/** Tag used to opt in into interface_deps behavior. */
491-
public static final String INTERFACE_DEPS_TAG = "__INTERFACE_DEPS__";
492-
493490
/** Ancestor for all rules that do include scanning. */
494491
public static final class CcIncludeScanningRule implements RuleDefinition {
495492
private final boolean addGrepIncludes;

‎src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,5 @@ void validateLayeringCheckFeatures(
7979

8080
boolean createEmptyArchive();
8181

82-
boolean shouldUseInterfaceDepsBehavior(RuleContext ruleContext);
82+
void checkCanUseImplementationDeps(RuleContext ruleContext);
8383
}

‎src/main/java/com/google/devtools/build/lib/rules/cpp/ObjcCppSemantics.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,5 @@ public boolean createEmptyArchive() {
134134
}
135135

136136
@Override
137-
public boolean shouldUseInterfaceDepsBehavior(RuleContext ruleContext) {
138-
return false;
139-
}
137+
public void checkCanUseImplementationDeps(RuleContext ruleContext) {}
140138
}

‎src/main/starlark/builtins_bzl/common/cc/cc_library.bzl‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,9 @@ def _cc_library_impl(ctx):
4141
semantics.validate_attributes(ctx = ctx)
4242
_check_no_repeated_srcs(ctx)
4343

44-
interface_deps = None
45-
implementation_deps = []
46-
should_use_interface_deps_behavior = semantics.should_use_interface_deps_behavior(ctx)
47-
if should_use_interface_deps_behavior:
48-
interface_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.interface_deps)
49-
implementation_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.deps)
50-
else:
51-
interface_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.deps)
44+
semantics.check_can_use_implementation_deps(ctx)
45+
interface_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.deps)
46+
implementation_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.implementation_deps)
5247

5348
if not _is_stl(ctx.attr.tags) and ctx.attr._stl != None:
5449
interface_deps.append(ctx.attr._stl[CcInfo].compilation_context)
@@ -113,7 +108,7 @@ def _cc_library_impl(ctx):
113108
is_google = True
114109

115110
linking_contexts = cc_helper.get_linking_contexts_from_deps(ctx.attr.deps)
116-
linking_contexts.extend(cc_helper.get_linking_contexts_from_deps(ctx.attr.interface_deps))
111+
linking_contexts.extend(cc_helper.get_linking_contexts_from_deps(ctx.attr.implementation_deps))
117112
if ctx.file.linkstamp != None:
118113
linkstamps = []
119114
linkstamps.append(cc_internal.create_linkstamp(
@@ -575,7 +570,7 @@ attrs = {
575570
),
576571
"alwayslink": attr.bool(default = False),
577572
"linkstatic": attr.bool(default = False),
578-
"interface_deps": attr.label_list(providers = [CcInfo], allow_files = False),
573+
"implementation_deps": attr.label_list(providers = [CcInfo], allow_files = False),
579574
"hdrs": attr.label_list(
580575
allow_files = True,
581576
flags = ["ORDER_INDEPENDENT", "DIRECT_COMPILE_TIME_INPUT"],
@@ -619,7 +614,7 @@ attrs = {
619614
}
620615
attrs.update(semantics.get_distribs_attr())
621616
attrs.update(semantics.get_loose_mode_in_hdrs_check_allowed_attr())
622-
attrs.update(semantics.get_interface_deps_allowed_attr())
617+
attrs.update(semantics.get_implementation_deps_allowed_attr())
623618

624619
cc_library = rule(
625620
implementation = _cc_library_impl,

‎src/main/starlark/builtins_bzl/common/cc/semantics.bzl‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,13 @@ def _get_coverage_env(ctx):
122122
def _should_use_legacy_cc_test(_):
123123
return True
124124

125-
def _get_interface_deps_allowed_attr():
125+
def _get_implementation_deps_allowed_attr():
126126
return {}
127127

128-
def _should_use_interface_deps_behavior(ctx):
129-
experimental_cc_interface_deps = ctx.fragments.cpp.experimental_cc_interface_deps()
130-
if (not experimental_cc_interface_deps and
131-
len(ctx.attr.interface_deps) > 0):
132-
fail("requires --experimental_cc_interface_deps", attr = "interface_deps")
133-
134-
return experimental_cc_interface_deps
128+
def _check_can_use_implementation_deps(ctx):
129+
experimental_cc_implementation_deps = ctx.fragments.cpp.experimental_cc_implementation_deps()
130+
if (not experimental_cc_implementation_deps and ctx.attr.implementation_deps):
131+
fail("requires --experimental_cc_implementation_deps", attr = "implementation_deps")
135132

136133
def _check_experimental_cc_shared_library(ctx):
137134
if not cc_common.check_experimental_cc_shared_library():
@@ -175,8 +172,8 @@ semantics = struct(
175172
get_stl = _get_stl,
176173
should_create_empty_archive = _should_create_empty_archive,
177174
get_grep_includes = _get_grep_includes,
178-
get_interface_deps_allowed_attr = _get_interface_deps_allowed_attr,
179-
should_use_interface_deps_behavior = _should_use_interface_deps_behavior,
175+
get_implementation_deps_allowed_attr = _get_implementation_deps_allowed_attr,
176+
check_can_use_implementation_deps = _check_can_use_implementation_deps,
180177
check_experimental_cc_shared_library = _check_experimental_cc_shared_library,
181178
get_linkstatic_default = _get_linkstatic_default,
182179
get_test_malloc_attr = _get_test_malloc_attr,

0 commit comments

Comments
 (0)