Skip to content

Commit 0b6dbc0

Browse files
kcsiegalfacebook-github-bot
authored andcommitted
Disable indexing with build flags for targets without swift
Summary: Restricts indexing via build flags to targets containing Swift source code for `buck project` fbshipit-source-id: c7c707f9b1150137f21009aad90959546a296933
1 parent 548ec37 commit 0b6dbc0

4 files changed

Lines changed: 53 additions & 32 deletions

File tree

src/com/facebook/buck/features/apple/projectV2/FlagParser.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class FlagParser {
100100
private final AppleDependenciesCache dependenciesCache;
101101
private final SourcePathResolverAdapter defaultPathResolver;
102102
private final HeaderSearchPaths headerSearchPaths;
103-
private final boolean indexViaBuildFlags;
103+
private final boolean indexViaCompileArgs;
104104
private final ImmutableMap<Flavor, CxxBuckConfig> platformCxxBuckConfigs;
105105

106106
FlagParser(
@@ -130,9 +130,7 @@ class FlagParser {
130130
this.dependenciesCache = dependenciesCache;
131131
this.defaultPathResolver = defaultPathResolver;
132132
this.headerSearchPaths = headerSearchPaths;
133-
this.indexViaBuildFlags =
134-
appleConfig.getProjectGeneratorIndexViaBuildFlags()
135-
|| appleConfig.getProjectGeneratorIndexViaCompileArgs();
133+
this.indexViaCompileArgs = appleConfig.getProjectGeneratorIndexViaCompileArgs();
136134
this.platformCxxBuckConfigs = cxxBuckConfig.getFlavoredConfigs();
137135
}
138136

@@ -182,7 +180,9 @@ void parseFlags(
182180
.orElse(ImmutableList.of()));
183181

184182
ImmutableList.Builder<String> cFlagsBuilder = ImmutableList.builder();
185-
if (indexViaBuildFlags) {
183+
if (com.facebook.buck.features.apple.projectV2.Utils.getShouldIndexViaBuildFlagsForTargetNode(
184+
targetNode, appleConfig)
185+
|| indexViaCompileArgs) {
186186
// We use argfiles to prevent increasing the already large xcconfig files in size
187187
try {
188188
cFlagsBuilder.add(

src/com/facebook/buck/features/apple/projectV2/HeaderSearchPaths.java

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,11 @@ class HeaderSearchPaths {
116116
private final ProjectSourcePathResolver projectSourcePathResolver;
117117
private final PathRelativizer pathRelativizer;
118118
private final SwiftAttributeParser swiftAttributeParser;
119-
private final ImmutableSet<String> swiftLabels;
120-
private final boolean indexViaBuildFlags;
121119
private final ProjectFilesystem projectFilesystem;
122120
private final BuildContext buildContext;
123121
private final UnresolvedCxxPlatform unresolvedCxxPlatform;
124122
private final boolean indexViaCompileArgs;
123+
private final AppleConfig appleConfig;
125124

126125
private static final LoadingCache<ActionGraphBuilder, Map<UnflavoredBuildTarget, Set<BuildRule>>>
127126
mappingCache =
@@ -160,10 +159,9 @@ class HeaderSearchPaths {
160159
this.projectSourcePathResolver = projectSourcePathResolver;
161160
this.pathRelativizer = pathRelativizer;
162161
this.swiftAttributeParser = swiftAttributeParser;
163-
this.swiftLabels = ImmutableSet.copyOf(appleConfig.getProjectGeneratorSwiftLabels());
164-
this.indexViaBuildFlags = appleConfig.getProjectGeneratorIndexViaBuildFlags();
165162
this.indexViaCompileArgs = appleConfig.getProjectGeneratorIndexViaCompileArgs();
166163
this.projectFilesystem = projectCell.getFilesystem();
164+
this.appleConfig = appleConfig;
167165
}
168166

169167
/** Derives header search path attributes for the {@code targetNode}. */
@@ -207,7 +205,7 @@ HeaderSearchPathAttributes getHeaderSearchPathAttributes(
207205
ImmutableSet<Path> swiftIncludePaths = collectRecursiveSwiftIncludePaths(targetNode);
208206
builder.setSwiftIncludePaths(swiftIncludePaths);
209207

210-
if (indexViaBuildFlags) {
208+
if (Utils.getShouldIndexViaBuildFlagsForTargetNode(targetNode, appleConfig)) {
211209
getIncludeFlags(targetNode, builder);
212210
}
213211
}
@@ -522,7 +520,7 @@ private void addPrivateHeaderIncludes(
522520
requiredBuildTargets.add(buildTarget);
523521

524522
// If the module is mixed then we need to import the VFS overlay and the -Swift.h header hmap
525-
if (targetNodeContainsSwift(targetNode)
523+
if (Utils.targetNodeContainsSwift(targetNode, appleConfig)
526524
&& !targetNode.getConstructorArg().getExportedHeaders().isEmpty()) {
527525
BuildTarget privateSwiftHeaderMapTarget =
528526
targetNode
@@ -584,7 +582,7 @@ private void addPublicHeaderIncludes(
584582
includeFlags.add("-I" + targetFilesystem.resolve(includePath));
585583
requiredBuildTargets.add(buildTarget);
586584

587-
if (targetNodeContainsSwift(targetNode)) {
585+
if (Utils.targetNodeContainsSwift(targetNode, appleConfig)) {
588586
// Swift libraries also need to include their dependent libraries swiftmodule files
589587
BuildTarget swiftCompileTarget =
590588
targetNode
@@ -872,7 +870,7 @@ private ImmutableSortedMap<Path, SourcePath> getPrivateCxxHeaders(
872870
SourceSortedSet.ofNamedSources(ImmutableSortedMap.of())));
873871
}
874872

875-
if (targetNodeContainsSwift(targetNode)) {
873+
if (Utils.targetNodeContainsSwift(targetNode, appleConfig)) {
876874
// Mixed libraries need to see their -Swift.h header. This is exposed through the module,
877875
// but is also expected to be available in unprefixed form internally.
878876
String moduleName =
@@ -1118,7 +1116,7 @@ private ImmutableSet<Path> collectRecursiveHeaderSearchPaths(
11181116
// but to do it right, it's likely that we'll need to add that. When that happens, we can also
11191117
// update this "if" statement. If, long-term, we move towards using modules for all Objective-C
11201118
// code, the merged header map will no longer work at all.
1121-
if (targetNodeContainsSwift(targetNode)) {
1119+
if (Utils.targetNodeContainsSwift(targetNode, appleConfig)) {
11221120
Flavor defaultPlatformFlavor =
11231121
targetNode.getConstructorArg().getDefaultPlatform().orElse(cxxPlatform.getFlavor());
11241122

@@ -1180,7 +1178,7 @@ private ImmutableSet<Path> collectRecursiveSwiftIncludePaths(
11801178
// we do need to add the paths to .swiftmodule files for dependencies.
11811179
if (nativeNode != targetNode
11821180
&& headerVisibility.equals(HeaderVisibility.PUBLIC)
1183-
&& targetNodeContainsSwift(nativeNode)) {
1181+
&& Utils.targetNodeContainsSwift(nativeNode, appleConfig)) {
11841182
BuildTarget flavoredSwiftCompileTarget =
11851183
NodeHelper.getSwiftModuleTarget(nativeNode, defaultPlatformFlavor);
11861184

@@ -1193,19 +1191,6 @@ && targetNodeContainsSwift(nativeNode)) {
11931191
return builder.build();
11941192
}
11951193

1196-
private boolean targetNodeContainsSwift(
1197-
TargetNode<? extends CxxLibraryDescription.CommonArg> targetNode) {
1198-
// Codegen libraries will have empty srcs so we cannot only rely on target inputs.
1199-
// We first check for any of the labels specified in apple.project_generator_swift_labels
1200-
// to know if this target is specified as Swift in the build rules.
1201-
for (String label : targetNode.getConstructorArg().getLabels()) {
1202-
if (swiftLabels.contains(label)) {
1203-
return true;
1204-
}
1205-
}
1206-
return AppleDescriptions.targetNodeContainsSwiftSourceCode(targetNode);
1207-
}
1208-
12091194
/** Adds the set of headers defined by headerVisibility to the merged header maps. */
12101195
private void addToMergedHeaderMap(
12111196
TargetNode<? extends CxxLibraryDescription.CommonArg> targetNode,

src/com/facebook/buck/features/apple/projectV2/Utils.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package com.facebook.buck.features.apple.projectV2;
1818

19+
import com.facebook.buck.apple.AppleConfig;
20+
import com.facebook.buck.apple.AppleDescriptions;
1921
import com.facebook.buck.core.exceptions.HumanReadableException;
2022
import com.facebook.buck.core.filesystems.AbsPath;
2123
import com.facebook.buck.core.model.BuildTarget;
@@ -109,4 +111,38 @@ public static void addRequiredBuildTargetFromSourcePath(
109111
requiredBuildTargetsBuilder.add(buildTarget);
110112
}
111113
}
114+
115+
/**
116+
* Checks whether the given target node contains Swift source code, including in generated files.
117+
*
118+
* @param targetNode The target node to check for Swift code.
119+
* @param appleConfig The Apple config for the project.
120+
*/
121+
public static boolean targetNodeContainsSwift(
122+
TargetNode<? extends CxxLibraryDescription.CommonArg> targetNode, AppleConfig appleConfig) {
123+
// Codegen libraries will have empty srcs so we cannot only rely on target inputs.
124+
// We first check for any of the labels specified in apple.project_generator_swift_labels
125+
// to know if this target is specified as Swift in the build rules.
126+
ImmutableSet<String> swiftLabels =
127+
ImmutableSet.copyOf(appleConfig.getProjectGeneratorSwiftLabels());
128+
for (String label : targetNode.getConstructorArg().getLabels()) {
129+
if (swiftLabels.contains(label)) {
130+
return true;
131+
}
132+
}
133+
return AppleDescriptions.targetNodeContainsSwiftSourceCode(targetNode);
134+
}
135+
136+
/**
137+
* Determines whether indexing via build flags should be enabled for the given target node.
138+
*
139+
* @param targetNode The target node for the indexing via build flags check.
140+
* @param appleConfig The Apple config for the project.
141+
*/
142+
public static boolean getShouldIndexViaBuildFlagsForTargetNode(
143+
TargetNode<? extends CxxLibraryDescription.CommonArg> targetNode, AppleConfig appleConfig) {
144+
// Only enable indexing with build flags if the target contains Swift
145+
return appleConfig.getProjectGeneratorIndexViaBuildFlags()
146+
&& Utils.targetNodeContainsSwift(targetNode, appleConfig);
147+
}
112148
}

src/com/facebook/buck/features/apple/projectV2/XcodeNativeTargetGenerator.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,7 +1488,7 @@ && shouldEmbedSwiftRuntimeInBundleTarget(bundle)
14881488
// Don't use Xcode's PCH generation when indexing with build flags. We will already have the
14891489
// correct include in the indexing flags.
14901490
if (prefixHeaderOptional.isPresent()
1491-
&& !appleConfig.getProjectGeneratorIndexViaBuildFlags()
1491+
&& !Utils.getShouldIndexViaBuildFlagsForTargetNode(targetNode, appleConfig)
14921492
&& !appleConfig.getProjectGeneratorIndexViaCompileArgs()) {
14931493
RelPath prefixHeaderRelative = resolveSourcePath(prefixHeaderOptional.get());
14941494
Path prefixHeaderPath =
@@ -1554,7 +1554,7 @@ && shouldEmbedSwiftRuntimeInBundleTarget(bundle)
15541554
getFrameworkAndLibrarySearchPathConfigs(
15551555
targetNode, xcodeNativeTargetAttributesBuilder, includeFrameworks));
15561556

1557-
if (!appleConfig.getProjectGeneratorIndexViaBuildFlags()
1557+
if (!Utils.getShouldIndexViaBuildFlagsForTargetNode(targetNode, appleConfig)
15581558
&& !appleConfig.getProjectGeneratorIndexViaCompileArgs()) {
15591559
appendConfigsBuilder.put(
15601560
"HEADER_SEARCH_PATHS",
@@ -1859,7 +1859,7 @@ private ImmutableSet<TargetNode<?>> recursivelyGetBuildDependencies(TargetNode<?
18591859
}
18601860

18611861
private void addDefaultSettingsAndExtraSettingsForNode(
1862-
TargetNode<? extends ConstructorArg> targetNode,
1862+
TargetNode<? extends CommonArg> targetNode,
18631863
Builder<String, String> defaultSettingsBuilder,
18641864
Builder<String, String> extraSettingsBuilder,
18651865
Builder<String, String> appendConfigsBuilder,
@@ -1893,7 +1893,7 @@ private void addDefaultSettingsAndExtraSettingsForNode(
18931893
defaultSettingsBuilder.put("CONFIGURATION_BUILD_DIR", "$BUILT_PRODUCTS_DIR");
18941894
defaultSettingsBuilder.put("EXECUTABLE_PREFIX", "lib");
18951895

1896-
if (!appleConfig.getProjectGeneratorIndexViaBuildFlags()
1896+
if (!Utils.getShouldIndexViaBuildFlagsForTargetNode(targetNode, appleConfig)
18971897
&& !appleConfig.getProjectGeneratorIndexViaCompileArgs()) {
18981898
appendConfigsBuilder.put(
18991899
"HEADER_SEARCH_PATHS",

0 commit comments

Comments
 (0)