From 4a0cc3b3f297f8df60022ae977e170148a4c7ae4 Mon Sep 17 00:00:00 2001 From: plf Date: Wed, 21 Jul 2021 04:46:20 -0700 Subject: [PATCH] Rewrite ObjcCommon in Starlark RELNOTES:none PiperOrigin-RevId: 385983265 --- .../lib/rules/objc/AppleStarlarkCommon.java | 34 ++- .../lib/rules/objc/CompilationAttributes.java | 55 ++++- .../build/lib/rules/objc/ObjcCommon.java | 39 ---- .../build/lib/rules/objc/ObjcProvider.java | 6 +- .../objc/ObjcProviderStarlarkConverters.java | 7 + .../build/lib/rules/objc/ObjcRuleClasses.java | 4 +- .../lib/rules/objc/ObjcStarlarkInternal.java | 75 +++++- .../common/objc/compilation_support.bzl | 26 ++- .../builtins_bzl/common/objc/objc_common.bzl | 217 ++++++++++++++++++ .../builtins_bzl/common/objc/objc_import.bzl | 1 - .../builtins_bzl/common/objc/objc_library.bzl | 8 +- .../lib/rules/objc/ObjcRuleTestCase.java | 2 +- .../integration/discard_graph_edges_test.sh | 4 +- 13 files changed, 409 insertions(+), 69 deletions(-) create mode 100644 src/main/starlark/builtins_bzl/common/objc/objc_common.bzl diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java index eeda9a1d8acfe1..834b6500f2109d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java @@ -39,7 +39,9 @@ import com.google.devtools.build.lib.rules.apple.DottedVersion; import com.google.devtools.build.lib.rules.apple.XcodeConfigInfo; import com.google.devtools.build.lib.rules.apple.XcodeVersionProperties; +import com.google.devtools.build.lib.rules.cpp.CcModule; import com.google.devtools.build.lib.rules.cpp.CppSemantics; +import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag; import com.google.devtools.build.lib.starlarkbuildapi.SplitTransitionProviderApi; import com.google.devtools.build.lib.starlarkbuildapi.apple.AppleCommonApi; import java.util.Map; @@ -193,12 +195,34 @@ public ObjcProvider newObjcProvider(Dict kwargs, StarlarkThread ObjcProvider.Key key = ObjcProvider.getStarlarkKeyForString(entry.getKey()); if (key != null) { resultBuilder.addElementsFromStarlark(key, entry.getValue()); - } else if (entry.getKey().equals("strict_include")) { - resultBuilder.addStrictIncludeFromStarlark(entry.getValue()); - } else if (entry.getKey().equals("providers")) { - resultBuilder.addProvidersFromStarlark(entry.getValue()); } else { - throw Starlark.errorf(BAD_KEY_ERROR, entry.getKey()); + switch (entry.getKey()) { + case "cc_library": + CcModule.checkPrivateStarlarkificationAllowlist(thread); + resultBuilder.uncheckedAddTransitive( + ObjcProvider.CC_LIBRARY, + ObjcProviderStarlarkConverters.convertToJava( + ObjcProvider.CC_LIBRARY, entry.getValue())); + break; + case "linkstamp": + CcModule.checkPrivateStarlarkificationAllowlist(thread); + resultBuilder.uncheckedAddTransitive( + ObjcProvider.LINKSTAMP, + ObjcProviderStarlarkConverters.convertToJava( + ObjcProvider.LINKSTAMP, entry.getValue())); + break; + case "flag": + resultBuilder.add(ObjcProvider.FLAG, Flag.USES_CPP); + break; + case "strict_include": + resultBuilder.addStrictIncludeFromStarlark(entry.getValue()); + break; + case "providers": + resultBuilder.addProvidersFromStarlark(entry.getValue()); + break; + default: + throw Starlark.errorf(BAD_KEY_ERROR, entry.getKey()); + } } } return resultBuilder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java index 5c4b8c5e8bfd16..a29e374dd98acf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; +import net.starlark.java.annot.Param; import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.Sequence; import net.starlark.java.eval.StarlarkList; @@ -351,6 +352,11 @@ public Depset hdrsForStarlark() { return Depset.of(Artifact.TYPE, hdrs()); } + @StarlarkMethod(name = "textual_hdrs", documented = false, structField = true) + public Depset textualHdrsForStarlark() { + return Depset.of(Artifact.TYPE, textualHdrs()); + } + /** * Returns the headers that cannot be compiled individually. */ @@ -376,6 +382,17 @@ public Depset includesForStarlark() { .collect(ImmutableList.toImmutableList()))); } + @StarlarkMethod(name = "sdk_includes", documented = false, structField = true) + public Depset sdkIncludesForStarlark() { + return Depset.of( + ElementType.STRING, + NestedSetBuilder.wrap( + Order.COMPILE_ORDER, + sdkIncludes().toList().stream() + .map(PathFragment::getSafePathString) + .collect(ImmutableList.toImmutableList()))); + } + /** * Returns the paths for SDK includes. */ @@ -390,13 +407,20 @@ public NestedSet sdkFrameworks() { return this.sdkFrameworks; } - @StarlarkMethod(name = "sdk_framework", documented = false, structField = true) - public Depset sdkFramework() { + @StarlarkMethod(name = "sdk_frameworks", documented = false, structField = true) + public Depset sdkFrameworksForStarlark() { return (Depset) ObjcProviderStarlarkConverters.convertToStarlark( ObjcProvider.SDK_FRAMEWORK, sdkFrameworks()); } + @StarlarkMethod(name = "weak_sdk_frameworks", documented = false, structField = true) + public Depset weakSdkFrameworksForStarlark() { + return (Depset) + ObjcProviderStarlarkConverters.convertToStarlark( + ObjcProvider.SDK_FRAMEWORK, weakSdkFrameworks()); + } + /** * Returns the SDK frameworks to be linked weakly. */ @@ -404,6 +428,11 @@ public NestedSet weakSdkFrameworks() { return this.weakSdkFrameworks; } + @StarlarkMethod(name = "sdk_dylibs", documented = false, structField = true) + public Depset sdkDylibsForStarlark() { + return Depset.of(ElementType.STRING, sdkDylibs); + } + /** * Returns the SDK Dylibs to link against. */ @@ -411,6 +440,23 @@ public NestedSet sdkDylibs() { return this.sdkDylibs; } + @StarlarkMethod( + name = "header_search_paths", + documented = false, + parameters = { + @Param(name = "genfiles_dir", positional = false, named = true), + }) + public Depset headerSearchPathsForStarlark(String genfilesDir) { + return Depset.of( + ElementType.STRING, + NestedSetBuilder.stableOrder() + .addAll( + headerSearchPaths(PathFragment.create(genfilesDir)).toList().stream() + .map(PathFragment::toString) + .collect(ImmutableList.toImmutableList())) + .build()); + } + /** * Returns the exec paths of all header search paths that should be added to this target and * dependers on this target, obtained from the {@code includes} attribute. @@ -454,6 +500,11 @@ public ImmutableList linkInputs() { return this.linkInputs; } + @StarlarkMethod(name = "defines", documented = false, structField = true) + public Sequence getDefinesForStarlark() { + return StarlarkList.immutableCopyOf(defines()); + } + /** Returns the defines. */ public ImmutableList defines() { return this.defines; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java index b2bb1a98801b4a..8f033aec8e8ba9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java @@ -59,7 +59,6 @@ import java.util.Iterator; import java.util.List; import java.util.Set; -import javax.annotation.Nullable; import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.StarlarkSemantics; import net.starlark.java.eval.StarlarkValue; @@ -512,23 +511,6 @@ public Optional getCompiledArchive() { return Optional.absent(); } - /** - * Returns the compiled {@code .a} file, or null if this object contains no {@link - * CompilationArtifacts} or the compilation information has no sources. - */ - @StarlarkMethod( - name = "compiled_archive", - documented = false, - structField = true, - allowReturnNones = true) - @Nullable - public Artifact getCompiledArchiveForStarlark() { - if (compilationArtifacts.isPresent() && compilationArtifacts.get().getArchive().isPresent()) { - return compilationArtifacts.get().getArchive().get(); - } - return null; - } - /** * Returns effective compilation options that do not arise from the crosstool. */ @@ -569,27 +551,6 @@ static Optional nearestContainerMatching( return Optional.absent(); } - /** - * Similar to {@link #nearestContainerMatching(FileType, Artifact)}, but returns the container - * closest to the root that matches the given type. - */ - static Optional farthestContainerMatching(FileType type, Artifact artifact) { - PathFragment container = artifact.getExecPath(); - Optional lastMatch = Optional.absent(); - do { - if (type.matches(container)) { - lastMatch = Optional.of(container); - } - container = container.getParentDirectory(); - } while (container != null); - return lastMatch; - } - - static Iterable notInContainerErrors( - Iterable artifacts, FileType containerType) { - return notInContainerErrors(artifacts, ImmutableList.of(containerType)); - } - static Iterable notInContainerErrors( Iterable artifacts, Iterable containerTypes) { Set errors = new HashSet<>(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java index 489200f7f5c86e..bb6e544cf29f2b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java @@ -400,11 +400,11 @@ public Depset weakSdkFramework() { @VisibleForTesting static final ImmutableList> KEYS_NOT_IN_STARLARK = ImmutableList.>of( - // LibraryToLink not exposed to Starlark. - CC_LIBRARY, // Flag enum is not exposed to Starlark. FLAG, - // Linkstamp is not exposed to Starlark. See commentary at its definition. + // cc_library is handled specially. + CC_LIBRARY, + // linkstamp is handled specially. LINKSTAMP, // Strict include is handled specially. STRICT_INCLUDE); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderStarlarkConverters.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderStarlarkConverters.java index 986d8d32e25892..5485a87640dc14 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderStarlarkConverters.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderStarlarkConverters.java @@ -21,6 +21,8 @@ import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.rules.cpp.CcLinkingContext.Linkstamp; +import com.google.devtools.build.lib.rules.cpp.LibraryToLink; import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key; import com.google.devtools.build.lib.vfs.PathFragment; import net.starlark.java.eval.EvalException; @@ -49,6 +51,11 @@ public static Object convertToStarlark(Key javaKey, NestedSet javaValue) { /** Returns a value for a java ObjcProvider given a key and a corresponding Starlark value. */ public static NestedSet convertToJava(Key javaKey, Object starlarkValue) throws EvalException { + if (javaKey.getType().equals(LibraryToLink.class)) { + return Depset.noneableCast(starlarkValue, LibraryToLink.class, "cc_library"); + } else if (javaKey.getType().equals(Linkstamp.class)) { + return Depset.noneableCast(starlarkValue, Linkstamp.class, "linkstamp"); + } return CONVERTERS.get(javaKey.getType()).valueForJava(javaKey, starlarkValue); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java index 0b9037a70109e7..d7f370b7a57506 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java @@ -253,9 +253,7 @@ public Metadata getMetadata() { } } - /** - * Iff a file matches this type, it is considered to use C++. - */ + /** Iff a file matches this type, it is considered to use C++. */ static final FileType CPP_SOURCES = FileType.of(".cc", ".cpp", ".mm", ".cxx", ".C"); static final FileType NON_CPP_SOURCES = FileType.of(".m", ".c"); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java index 53159cc6b1f9d6..d1814f235cb7ad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.rules.objc; +import static com.google.common.collect.ImmutableList.toImmutableList; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.docgen.annot.DocCategory; @@ -25,9 +27,11 @@ import com.google.devtools.build.lib.packages.NativeInfo; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.rules.cpp.CcCompilationContext; import com.google.devtools.build.lib.rules.cpp.CppSemantics; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.shell.ShellUtils.TokenizationException; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; @@ -76,10 +80,11 @@ public CompilationAttributes createCompilationAttributes(StarlarkRuleContext sta builder, starlarkRuleContext.getRuleContext()); CompilationAttributes.Builder.addSdkAttributesFromRuleContext( builder, starlarkRuleContext.getRuleContext()); - List copts = + Sequence copts = expandToolchainAndRuleContextVariables( starlarkRuleContext, - starlarkRuleContext.getRuleContext().attributes().get("copts", Type.STRING_LIST)); + StarlarkList.immutableCopyOf( + starlarkRuleContext.getRuleContext().attributes().get("copts", Type.STRING_LIST))); CompilationAttributes.Builder.addCompileOptionsFromRuleContext( builder, starlarkRuleContext.getRuleContext(), copts); CompilationAttributes.Builder.addModuleOptionsFromRuleContext( @@ -88,8 +93,15 @@ public CompilationAttributes createCompilationAttributes(StarlarkRuleContext sta return builder.build(); } - private List expandToolchainAndRuleContextVariables( - StarlarkRuleContext starlarkRuleContext, Iterable flags) throws EvalException { + @StarlarkMethod( + name = "expand_toolchain_and_ctx_variables", + documented = false, + parameters = { + @Param(name = "ctx", positional = false, named = true), + @Param(name = "flags", positional = false, defaultValue = "[]", named = true), + }) + public Sequence expandToolchainAndRuleContextVariables( + StarlarkRuleContext starlarkRuleContext, Sequence flags) throws EvalException { ImmutableMap toolchainMap = starlarkRuleContext .getRuleContext() @@ -98,7 +110,7 @@ private List expandToolchainAndRuleContextVariables( ImmutableMap starlarkRuleContextMap = ImmutableMap.builder().putAll(starlarkRuleContext.var()).build(); List expandedFlags = new ArrayList<>(); - for (String flag : flags) { + for (String flag : Sequence.cast(flags, String.class, "flags")) { String expandedFlag = expandFlag(flag, toolchainMap, starlarkRuleContextMap); try { ShellUtils.tokenize(expandedFlags, expandedFlag); @@ -106,7 +118,7 @@ private List expandToolchainAndRuleContextVariables( throw new EvalException(e); } } - return expandedFlags; + return StarlarkList.immutableCopyOf(expandedFlags); } private String expandFlag( @@ -280,4 +292,55 @@ public InstrumentedFilesInfo createInstrumentedFilesInfo( starlarkRuleContext.getRuleContext(), Sequence.cast(objectFiles, Artifact.class, "object_files").getImmutableList()); } + + @StarlarkMethod( + name = "create_compilation_context", + documented = false, + parameters = { + @Param(name = "public_hdrs", positional = false, defaultValue = "[]", named = true), + @Param(name = "public_textual_hdrs", positional = false, defaultValue = "[]", named = true), + @Param(name = "private_hdrs", positional = false, defaultValue = "[]", named = true), + @Param(name = "providers", positional = false, defaultValue = "[]", named = true), + @Param( + name = "direct_cc_compilation_contexts", + positional = false, + defaultValue = "[]", + named = true), + @Param( + name = "cc_compilation_contexts", + positional = false, + defaultValue = "[]", + named = true), + @Param(name = "defines", positional = false, defaultValue = "[]", named = true), + @Param(name = "includes", positional = false, defaultValue = "[]", named = true), + }) + public ObjcCompilationContext createCompilationContext( + Sequence publicHdrs, + Sequence publicTextualHdrs, + Sequence privateHdrs, + Sequence providers, + Sequence directCcCompilationContexts, + Sequence ccCompilationContexts, + Sequence defines, + Sequence includes) + throws InterruptedException, EvalException { + return ObjcCompilationContext.builder() + .addPublicHeaders(Sequence.cast(publicHdrs, Artifact.class, "public_hdrs")) + .addPublicTextualHeaders( + Sequence.cast(publicTextualHdrs, Artifact.class, "public_textual_hdrs")) + .addPrivateHeaders(Sequence.cast(privateHdrs, Artifact.class, "private_hdrs")) + .addObjcProviders(Sequence.cast(providers, ObjcProvider.class, "providers")) + .addDirectCcCompilationContexts( + Sequence.cast( + directCcCompilationContexts, CcCompilationContext.class, "cc_compilation_contexts")) + .addCcCompilationContexts( + Sequence.cast( + ccCompilationContexts, CcCompilationContext.class, "cc_compilation_contexts")) + .addDefines(Sequence.cast(defines, String.class, "defines")) + .addIncludes( + Sequence.cast(includes, String.class, "includes").stream() + .map(PathFragment::create) + .collect(toImmutableList())) + .build(); + } } diff --git a/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl b/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl index 045a6dc862b337..3ec8ca4e9d0ad9 100644 --- a/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl @@ -15,6 +15,7 @@ """Utility methods used for creating objc_* rules actions""" load("@_builtins//:common/cc/cc_helper.bzl", "cc_helper") +load("@_builtins//:common/objc/objc_common.bzl", "objc_common") objc_internal = _builtins.internal.objc_internal cc_common = _builtins.toplevel.cc_common @@ -60,7 +61,25 @@ def _build_common_variables( compilation_artifacts = objc_internal.create_compilation_artifacts() else: compilation_artifacts = objc_internal.create_compilation_artifacts(ctx = ctx) - objc_common = objc_internal.create_common( + + # objc_common = objc_internal.create_common( + # purpose = "COMPILE_AND_LINK", + # ctx = ctx, + # compilation_attributes = compilation_attributes, + # compilation_artifacts = compilation_artifacts, + # deps = deps, + # runtime_deps = runtime_deps, + # intermediate_artifacts = intermediate_artifacts, + # alwayslink = ctx.attr.alwayslink, + # has_module_map = True, + # extra_import_libraries = extra_import_libraries, + # linkopts = linkopts, + # ) + + # objc_provider = objc_common.objc_provider + # objc_compilation_context = objc_common.objc_compilation_context + + (objc_provider, objc_compilation_context) = objc_common.create_context_and_provider( purpose = "COMPILE_AND_LINK", ctx = ctx, compilation_attributes = compilation_attributes, @@ -74,17 +93,18 @@ def _build_common_variables( linkopts = linkopts, ) - return objc_common, struct( + return struct( ctx = ctx, intermediate_artifacts = intermediate_artifacts, compilation_attributes = compilation_attributes, compilation_artifacts = compilation_artifacts, - objc_compilation_context = objc_common.objc_compilation_context, + objc_compilation_context = objc_compilation_context, toolchain = toolchain, use_pch = use_pch, disable_layering_check = disable_layering_check, disable_parse_headers = disable_parse_hdrs, objc_config = ctx.fragments.objc, + objc_provider = objc_provider, ) def _build_feature_configuration(common_variables, for_swift_module_map, support_parse_headers): diff --git a/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl b/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl new file mode 100644 index 00000000000000..bfc3a3b5e4d1ae --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl @@ -0,0 +1,217 @@ +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Builds the Objective-C provider""" + +objc_internal = _builtins.internal.objc_internal +CcInfo = _builtins.toplevel.CcInfo +apple_common = _builtins.toplevel.apple_common + +def _create_context_and_provider( + purpose, + ctx, + compilation_attributes, + compilation_artifacts, + intermediate_artifacts, + alwayslink, + has_module_map, + extra_import_libraries, + deps, + runtime_deps, + linkopts): + objc_providers = [] + cc_compilation_contexts_for_direct_fields = [] + cc_linking_contexts = [] + cc_compilation_contexts = [] + cc_linkstamp_contexts = [] + for dep in deps: + if apple_common.Objc in dep: + objc_providers.append(dep[apple_common.Objc]) + elif CcInfo in dep: + cc_compilation_contexts_for_direct_fields.append(dep[CcInfo].compilation_context) + cc_linking_contexts.append(dep[CcInfo].linking_context) + + if CcInfo in dep: + cc_compilation_contexts.append(dep[CcInfo].compilation_context) + cc_linkstamp_contexts.append(dep[CcInfo].linking_context) + + runtime_objc_providers = [] + for runtime_dep in runtime_deps: + if apple_common.Objc in runtime_dep: + runtime_objc_providers.append(runtime_dep[apple_common.Objc]) + if CcInfo in runtime_dep: + cc_compilation_contexts.append(runtime_dep[CcInfo].cc_compilation_context) + + link_order_keys = [ + "imported_library", + "cc_library", + "library", + "force_load_library", + "linkopt", + ] + objc_provider_kwargs = { + "imported_library": [depset(direct = extra_import_libraries, order = "topological")], + "header": [], + "weak_sdk_framework": [], + "sdk_dylib": [], + "linkopt": [], + "cc_library": [], + "library": [], + "providers": objc_providers, + "sdk_framework": [], + "linkstamp": [], + "force_load_library": [], + "umbrella_header": [], + "module_map": [], + "source": [], + } + + objc_compilation_context_kwargs = { + "providers": objc_providers + runtime_objc_providers, + "cc_compilation_contexts": cc_compilation_contexts, + "public_hdrs": [], + "private_hdrs": [], + "public_textual_hdrs": [], + "defines": [], + "includes": [], + } + + for header_provider in cc_compilation_contexts_for_direct_fields: + objc_provider_kwargs["header"].extend(header_provider.headers.to_list()) + + for link_provider in cc_linking_contexts: + link_opts = [] + libraries_to_link = [] + for linker_input in link_provider.linker_inputs.to_list(): + link_opts.extend(linker_input.user_link_flags) + libraries_to_link.extend(linker_input.libraries) + _add_linkopts(objc_provider_kwargs, link_opts) + + objc_provider_kwargs["cc_library"].append(depset(direct = libraries_to_link, order = "topological")) + + _add_linkopts(objc_provider_kwargs, objc_internal.expand_toolchain_and_ctx_variables(ctx = ctx, flags = linkopts)) + + for cc_linkstamp_context in cc_linkstamp_contexts: + objc_provider_kwargs["linkstamp"].extend(cc_linkstamp_context.linkstamps().to_list()) + + if compilation_attributes != None: + sdk_dir = apple_common.apple_toolchain().sdk_dir() + usr_include_dir = sdk_dir + "/usr/include/" + sdk_includes = [] + + for sdk_include in compilation_attributes.sdk_includes.to_list(): + sdk_includes.append(usr_include_dir + sdk_include) + + objc_provider_kwargs["sdk_framework"].extend(compilation_attributes.sdk_frameworks.to_list()) + objc_provider_kwargs["weak_sdk_framework"].extend(compilation_attributes.weak_sdk_frameworks.to_list()) + objc_provider_kwargs["sdk_dylib"].extend(compilation_attributes.sdk_dylibs.to_list()) + hdrs = compilation_attributes.hdrs.to_list() + objc_provider_kwargs["header"].extend(hdrs) + textual_hdrs = compilation_attributes.textual_hdrs.to_list() + objc_provider_kwargs["header"].extend(textual_hdrs) + + objc_compilation_context_kwargs["public_hdrs"].extend(hdrs) + objc_compilation_context_kwargs["public_textual_hdrs"].extend(textual_hdrs) + objc_compilation_context_kwargs["defines"].extend(compilation_attributes.defines) + objc_compilation_context_kwargs["includes"].extend(compilation_attributes.header_search_paths(genfiles_dir = ctx.genfiles_dir.path).to_list()) + objc_compilation_context_kwargs["includes"].extend(sdk_includes) + + if compilation_artifacts != None: + all_sources = [] + all_sources.extend(compilation_artifacts.srcs) + all_sources.extend(compilation_artifacts.non_arc_srcs) + all_sources.extend(compilation_artifacts.private_hdrs) + + if compilation_artifacts.archive != None: + objc_provider_kwargs["library"] = [depset([compilation_artifacts.archive], order = "topological")] + objc_provider_kwargs["source"].extend(all_sources) + objc_provider_kwargs["header"].extend(compilation_artifacts.additional_hdrs.to_list()) + + objc_compilation_context_kwargs["public_hdrs"].extend(compilation_artifacts.additional_hdrs.to_list()) + objc_compilation_context_kwargs["private_hdrs"].extend(compilation_artifacts.private_hdrs) + + uses_cpp = False + arc_and_non_arc_srcs = [] + arc_and_non_arc_srcs.extend(compilation_artifacts.srcs) + arc_and_non_arc_srcs.extend(compilation_artifacts.non_arc_srcs) + for source_file in arc_and_non_arc_srcs: + uses_cpp = uses_cpp or _is_cpp_source(source_file) + + if uses_cpp: + objc_provider_kwargs["flag"] = ["uses_cpp"] + + if alwayslink: + direct = [] + if compilation_artifacts != None: + if compilation_artifacts.archive != None: + direct.append(compilation_artifacts.archive) + + objc_provider_kwargs["force_load_library"] = [depset(direct = direct, transitive = objc_provider_kwargs["force_load_library"], order = "topological")] + + if has_module_map: + module_map = intermediate_artifacts.swift_module_map + umbrella_header = module_map.umbrella_header() + if umbrella_header != None: + objc_provider_kwargs["umbrella_header"].append(umbrella_header) + + objc_provider_kwargs["module_map"].append(module_map.file()) + + objc_provider_kwargs_built = {} + for k, v in objc_provider_kwargs.items(): + if k == "providers": + objc_provider_kwargs_built[k] = v + elif k in link_order_keys: + objc_provider_kwargs_built[k] = depset(transitive = v, order = "topological") + else: + objc_provider_kwargs_built[k] = depset(v) + + objc_compilation_context = objc_internal.create_compilation_context( + **objc_compilation_context_kwargs + ) + + return (apple_common.new_objc_provider(**objc_provider_kwargs_built), objc_compilation_context) + +def _is_cpp_source(source_file): + return source_file.extension in ["cc", "cpp", "mm", "cxx", "C"] + +def _add_linkopts(objc_provider_kwargs, link_opts): + framework_link_opts = {} + non_framework_link_opts = [] + i = 0 + skip_next = False + for arg in link_opts: + if skip_next: + skip_next = False + i += 1 + continue + if arg == "-framework" and i < len(link_opts) - 1: + framework_link_opts[link_opts[i + 1]] = True + skip_next = True + else: + non_framework_link_opts.append(arg) + i += 1 + + objc_provider_kwargs["sdk_framework"].extend(framework_link_opts.keys()) + objc_provider_kwargs["linkopt"].append(depset(direct = non_framework_link_opts, order = "topological")) + +def _compilation_contexts_from_cc_infos(cc_infos): + cc_compilation_contexts = [] + for cc_info in cc_infos: + cc_compilation_contexts.append(cc_info.compilation_context) + + return cc_compilation_contexts + +objc_common = struct( + create_context_and_provider = _create_context_and_provider, +) diff --git a/src/main/starlark/builtins_bzl/common/objc/objc_import.bzl b/src/main/starlark/builtins_bzl/common/objc/objc_import.bzl index 9a6e31fa9e3057..9150b41e62e98c 100644 --- a/src/main/starlark/builtins_bzl/common/objc/objc_import.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/objc_import.bzl @@ -35,7 +35,6 @@ def _objc_import_impl(ctx): compilation_artifacts = objc_internal.create_compilation_artifacts(), ) - compilation_attributes = objc_internal.create_compilation_attributes(ctx = ctx) compilation_support = objc_internal.create_compilation_support( ctx = ctx, semantics = semantics.get_semantics(), diff --git a/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl b/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl index 4fc834092edee5..581a667d3bfdcf 100644 --- a/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl @@ -133,7 +133,7 @@ def _objc_library_impl(ctx): cc_toolchain = cc_helper.find_cpp_toolchain(ctx) - (objc_common, common_variables) = compilation_support.build_common_variables( + common_variables = compilation_support.build_common_variables( ctx, cc_toolchain, True, @@ -146,8 +146,8 @@ def _objc_library_impl(ctx): ctx.attr.linkopts, ) files = [] - if objc_common.compiled_archive != None: - files.append(objc_common.compiled_archive) + if common_variables.compilation_artifacts.archive != None: + files.append(common_variables.compilation_artifacts.archive) (cc_compilation_context, compilation_outputs, output_group_info) = compilation_support.register_compile_and_archive_actions( common_variables, @@ -159,7 +159,7 @@ def _objc_library_impl(ctx): j2objc_providers = objc_internal.j2objc_providers_from_deps(ctx = ctx) - objc_provider = objc_common.objc_provider + objc_provider = common_variables.objc_provider feature_configuration = compilation_support.build_feature_configuration(common_variables, False, True) linking_context = _build_linking_context(ctx, feature_configuration, cc_toolchain, objc_provider, common_variables) cc_info = CcInfo( diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java index 906cff409ad43a..3d2ccb4d5f96d6 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java @@ -1236,7 +1236,7 @@ protected void checkObjcLibraryLinkoptsArePropagatedToLinkAction(RuleType ruleTy // Frameworks included in linkopts by the user should get placed together with no duplicates. // (They may duplicate the ones inserted by the CROSSTOOL feature, but we don't test that here.) assertThat(Joiner.on(" ").join(linkAction("//x").getArguments())) - .contains("-framework F1 -framework F2"); + .contains(" -framework F1 -framework F2"); // Linkopts should also be grouped together. assertThat(Joiner.on(" ").join(linkAction("//x").getArguments())) .contains("-another-opt -one-more-opt -Wl,--other-opt"); diff --git a/src/test/shell/integration/discard_graph_edges_test.sh b/src/test/shell/integration/discard_graph_edges_test.sh index 3d243e2b8aee85..9c814d543f4327 100755 --- a/src/test/shell/integration/discard_graph_edges_test.sh +++ b/src/test/shell/integration/discard_graph_edges_test.sh @@ -276,8 +276,8 @@ function test_packages_cleared() { [[ "$glob_count" -le 1 ]] \ || fail "glob count $glob_count too high" module_count="$(extract_histogram_count "$histo_file" 'eval.Module$')" - [[ "$module_count" -lt 25 ]] \ - || fail "Module count $module_count too high" # was 22 + [[ "$module_count" -lt 100 ]] \ + || fail "Module count $module_count too high" ct_count="$(extract_histogram_count "$histo_file" \ 'RuleConfiguredTarget$')" [[ "$ct_count" -le 1 ]] \