Skip to content

Commit

Permalink
Stop generating swift module maps in objc_library and objc_import
Browse files Browse the repository at this point in the history
swift_library will now generate those.  Unfortunately, we still need
to generate swift module maps for j2objc -- we really only need the
umbrella header, but the two are intertwined.

PiperOrigin-RevId: 395752339
  • Loading branch information
googlewalt authored and copybara-github committed Sep 9, 2021
1 parent 48dd159 commit 31bec27
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ private FeatureConfiguration getFeatureConfiguration(
RuleContext ruleContext,
CcToolchainProvider ccToolchain,
BuildConfiguration configuration,
CppSemantics cppSemantics,
boolean forSwiftModuleMap) {
CppSemantics cppSemantics) {
ImmutableSet.Builder<String> activatedCrosstoolSelectables =
ImmutableSet.<String>builder()
.addAll(ruleContext.getFeatures())
Expand All @@ -203,16 +202,6 @@ private FeatureConfiguration getFeatureConfiguration(
if (disableLayeringCheck) {
disabledFeatures.add(CppRuleClasses.LAYERING_CHECK);
}
if (forSwiftModuleMap) {
activatedCrosstoolSelectables
.add(CppRuleClasses.MODULE_MAPS)
.add(CppRuleClasses.COMPILE_ALL_MODULES)
.add(CppRuleClasses.ONLY_DOTH_HEADERS_IN_MODULE_MAPS)
.add(CppRuleClasses.EXCLUDE_PRIVATE_HEADERS_IN_MODULE_MAPS)
.add(CppRuleClasses.MODULE_MAP_WITHOUT_EXTERN_MODULE)
.add(CppRuleClasses.ONLY_DOTH_HEADERS_IN_MODULE_MAPS);
disabledFeatures.add(CppRuleClasses.GENERATE_SUBMODULES);
}

return CcCommon.configureFeaturesOrReportRuleError(
ruleContext,
Expand All @@ -223,24 +212,6 @@ private FeatureConfiguration getFeatureConfiguration(
cppSemantics);
}

private FeatureConfiguration getFeatureConfiguration(
RuleContext ruleContext,
CcToolchainProvider ccToolchain,
BuildConfiguration configuration,
CppSemantics cppSemantics) {
return getFeatureConfiguration(
ruleContext, ccToolchain, configuration, cppSemantics, /* forSwiftModuleMap= */ false);
}

private FeatureConfiguration getFeatureConfigurationForSwiftModuleMap(
RuleContext ruleContext,
CcToolchainProvider ccToolchain,
BuildConfiguration configuration,
CppSemantics cppSemantics) {
return getFeatureConfiguration(
ruleContext, ccToolchain, configuration, cppSemantics, /* forSwiftModuleMap= */ true);
}

/** Iterable wrapper providing strong type safety for arguments to binary linking. */
static final class ExtraLinkArgs extends IterableWrapper<String> {
ExtraLinkArgs(String... args) {
Expand Down
91 changes: 40 additions & 51 deletions src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ def _compile(
pch_hdr,
module_map,
purpose,
generate_module_map,
should_process_headers):
generate_module_map):
objc_compilation_context = common_variables.objc_compilation_context
includes = []
includes.extend(priority_headers)
Expand Down Expand Up @@ -210,7 +209,7 @@ def _compile(
language = "objc",
code_coverage_enabled = cc_helper.is_code_coverage_enabled(ctx = common_variables.ctx),
hdrs_checking_mode = "strict",
do_not_generate_module_map = module_map.file().is_source or not generate_module_map,
do_not_generate_module_map = not generate_module_map or module_map.file().is_source,
purpose = purpose,
)

Expand Down Expand Up @@ -289,12 +288,17 @@ def _register_compile_and_archive_actions_for_j2objc(
apple_config = ctx.fragments.apple,
objc_provider = None,
)
return _register_compile_and_archive_actions(common_variables, extra_compile_args)
return _register_compile_and_archive_actions(
common_variables,
extra_compile_args,
generate_module_map_for_swift = True,
)

def _register_compile_and_archive_actions(
common_variables,
extra_compile_args = [],
priority_headers = []):
priority_headers = [],
generate_module_map_for_swift = False):
compilation_result = None

if common_variables.compilation_artifacts.archive != None:
Expand All @@ -307,6 +311,7 @@ def _register_compile_and_archive_actions(
"OBJC_ARCHIVE",
obj_list,
["ARCHIVE_VARIABLE"],
generate_module_map_for_swift,
)

_register_obj_file_list_action(
Expand All @@ -322,6 +327,7 @@ def _register_compile_and_archive_actions(
link_type = None,
link_action_input = None,
variable_categories = [],
generate_module_map_for_swift = generate_module_map_for_swift,
)

return compilation_result
Expand All @@ -342,7 +348,8 @@ def _cc_compile_and_link(
priority_headers,
link_type,
link_action_input,
variable_categories):
variable_categories,
generate_module_map_for_swift):
compilation_artifacts = common_variables.compilation_artifacts
intermediate_artifacts = common_variables.intermediate_artifacts
compilation_attributes = common_variables.compilation_attributes
Expand All @@ -352,55 +359,29 @@ def _cc_compile_and_link(
public_hdrs.extend(compilation_attributes.hdrs.to_list())
public_hdrs.extend(compilation_artifacts.additional_hdrs.to_list())
pch_header = _get_pch_file(common_variables)
feature_configuration = _build_feature_configuration(common_variables, False, True)

# Generate up to two module maps, while minimizing the number of actions created. If
# module_map feature is off, generate a swift module map. If module_map feature is on,
# generate a layering check and a swift module map. In the latter case, the layering check
# module map must be the primary one.
#
# TODO(waltl): Delete this logic when swift module map is migrated to swift_library.

primary_module_map = None
arc_primary_module_map_fc = None
non_arc_primary_module_map_fc = None
extra_module_map = None
extra_module_map_fc = None
fc_for_swift_module_map = _build_feature_configuration(
feature_configuration = _build_feature_configuration(
common_variables,
for_swift_module_map = True,
for_swift_module_map = False,
support_parse_headers = True,
)
if cc_common.is_enabled(feature_configuration = feature_configuration, feature_name = "module_maps"):
primary_module_map = intermediate_artifacts.internal_module_map
arc_primary_module_map_fc = feature_configuration
non_arc_primary_module_map_fc = _build_feature_configuration(
common_variables,
for_swift_module_map = True,
support_parse_headers = False,
)
extra_module_map = intermediate_artifacts.swift_module_map
extra_module_map_fc = fc_for_swift_module_map
else:
primary_module_map = intermediate_artifacts.swift_module_map
arc_primary_module_map_fc = fc_for_swift_module_map
non_arc_primary_module_map_fc = _build_feature_configuration(
common_variables,
for_swift_module_map = True,
support_parse_headers = False,
)
extra_module_map = None
extra_module_map_fc = None

generate_module_map = cc_common.is_enabled(
feature_configuration = feature_configuration,
feature_name = "module_maps",
)
module_map = None
if generate_module_map:
module_map = intermediate_artifacts.internal_module_map

purpose = "{}_objc_arc".format(_get_purpose(common_variables))
arc_primary_module_map_fc = feature_configuration
arc_extensions = _build_variable_extensions(
common_variables,
ctx,
intermediate_artifacts,
variable_categories,
arc_enabled = True,
)

(arc_compilation_context, arc_compilation_outputs) = _compile(
common_variables,
arc_primary_module_map_fc,
Expand All @@ -411,12 +392,17 @@ def _cc_compile_and_link(
compilation_artifacts.private_hdrs,
public_hdrs,
pch_header,
primary_module_map,
module_map,
purpose,
generate_module_map = True,
should_process_headers = True,
generate_module_map,
)

purpose = "{}_non_objc_arc".format(_get_purpose(common_variables))
non_arc_primary_module_map_fc = _build_feature_configuration(
common_variables,
for_swift_module_map = False,
support_parse_headers = False,
)
non_arc_extensions = _build_variable_extensions(
common_variables,
ctx,
Expand All @@ -434,24 +420,27 @@ def _cc_compile_and_link(
compilation_artifacts.private_hdrs,
public_hdrs,
pch_header,
primary_module_map,
module_map,
purpose,
generate_module_map = False,
should_process_headers = False,
)

objc_compilation_context = common_variables.objc_compilation_context

if extra_module_map != None and not extra_module_map.file().is_source:
if generate_module_map_for_swift:
_generate_extra_module_map(
common_variables,
extra_module_map,
intermediate_artifacts.swift_module_map,
public_hdrs,
compilation_artifacts.private_hdrs,
objc_compilation_context.public_textual_hdrs,
pch_header,
objc_compilation_context.cc_compilation_contexts,
extra_module_map_fc,
_build_feature_configuration(
common_variables,
for_swift_module_map = True,
support_parse_headers = False,
),
)

if link_type == "OBJC_ARCHIVE":
Expand Down
1 change: 0 additions & 1 deletion src/main/starlark/builtins_bzl/common/objc/objc_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def _objc_import_impl(ctx):
alwayslink = ctx.attr.alwayslink,
extra_import_libraries = ctx.files.archives,
empty_compilation_artifacts = True,
has_module_map = True,
)

(cc_compilation_context, _, _) = compilation_support.register_compile_and_archive_actions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ def _objc_library_impl(ctx):
runtime_deps = ctx.attr.runtime_deps,
linkopts = ctx.attr.linkopts,
alwayslink = ctx.attr.alwayslink,
has_module_map = True,
)
files = []
if common_variables.compilation_artifacts.archive != None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1181,11 +1181,6 @@ public void testLoadableBundleBinaryAddsRpathLinkOptWithBundleLoader() throws Ex
.contains("@loader_path/Frameworks");
}

@Test
public void testCustomModuleMap() throws Exception {
checkCustomModuleMap(getRuleType());
}

@Test
public void testMinimumOsDifferentTargets() throws Exception {
checkMinimumOsDifferentTargets(getRuleType(), "_lipobin", "_bin");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
import com.google.devtools.build.lib.rules.cpp.CppLinkAction;
import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
import com.google.devtools.build.lib.rules.cpp.CppModuleMapAction;
import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.common.options.OptionsParsingException;
Expand Down Expand Up @@ -782,41 +780,6 @@ private void checkBitcodeModeError(String... args) throws Exception {
assertThat(thrown).hasMessageThat().contains(INVALID_APPLE_BITCODE_OPTION_FORMAT);
}

@Test
public void testModuleNameAttributeChangesName() throws Exception {
RULE_TYPE.scratchTarget(scratch, "module_name", "'foo'");

ConfiguredTarget configuredTarget = getConfiguredTarget("//x:x");
Artifact moduleMap = getGenfilesArtifact("x.modulemaps/module.modulemap", configuredTarget);

CppModuleMapAction genMap = (CppModuleMapAction) getGeneratingAction(moduleMap);

CppModuleMap cppModuleMap = genMap.getCppModuleMap();
assertThat(cppModuleMap.getName()).isEqualTo("foo");
}

@Test
public void testModuleMapActionFiltersHeaders() throws Exception {
RULE_TYPE.scratchTarget(
scratch,
"srcs",
"['a.m', 'b.m', 'private.h', 'private.inc']",
"hdrs",
"['a.h', 'x.inc', 'foo.m', 'bar.mm']");

ConfiguredTarget configuredTarget = getConfiguredTarget("//x:x");
Artifact moduleMap = getGenfilesArtifact("x.modulemaps/module.modulemap", configuredTarget);

CppModuleMapAction genMap = (CppModuleMapAction) getGeneratingAction(moduleMap);

assertThat(Artifact.toRootRelativePaths(genMap.getPrivateHeaders())).isEmpty();
assertThat(Artifact.toRootRelativePaths(genMap.getPublicHeaders())).containsExactly("x/a.h");

// now check the generated name
CppModuleMap cppModuleMap = genMap.getCppModuleMap();
assertThat(cppModuleMap.getName()).isEqualTo("x_x");
}

@Test
public void testCompilationActionsWithCoptFmodules() throws Exception {
createLibraryTargetWriter("//objc:lib")
Expand Down Expand Up @@ -1845,11 +1808,6 @@ public void testDefaultEnabledFeatureIsUsed() throws Exception {
assertThat(compileAction.getArguments()).contains("-dummy");
}

@Test
public void testCustomModuleMap() throws Exception {
checkCustomModuleMap(RULE_TYPE);
}

@Test
public void testHeaderPassedToCcLib() throws Exception {
createLibraryTargetWriter("//objc:lib").setList("hdrs", "objc_hdr.h").write();
Expand Down Expand Up @@ -1918,8 +1876,6 @@ public void testDirectFields() throws Exception {
.containsExactly("bar.h", "bar.inc");
assertThat(baseArtifactNames(dependerProvider.getDirect(ObjcProvider.SOURCE)))
.containsExactly("bar.m", "bar_impl.h");
assertThat(Artifact.toRootRelativePaths(dependerProvider.getDirect(ObjcProvider.MODULE_MAP)))
.containsExactly("x/bar.modulemaps/module.modulemap");

ConfiguredTarget target = getConfiguredTarget("//x:bar");
CcCompilationContext ccCompilationContext =
Expand Down

0 comments on commit 31bec27

Please sign in to comment.