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 c97c9f5c033f85..3ab96fa1df305e 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 @@ -22,6 +22,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -39,6 +40,7 @@ import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext; import com.google.devtools.build.lib.skylarkbuildapi.apple.ObjcProviderApi; import com.google.devtools.build.lib.syntax.EvalUtils; +import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.vfs.PathFragment; @@ -342,10 +344,25 @@ public enum Flag { } private final StarlarkSemantics semantics; + + // Items which are propagated transitively to dependents. private final ImmutableMap, NestedSet> items; + /** + * This is intended to be used by clients which need to collect transitive information without + * paying the O(n^2) behavior to flatten it during analysis time. + * + *

For example, IDEs may use this to identify all direct header files for a target and fetch + * all transitive headers from its dependencies by recursing through this field. + */ + private final ImmutableListMultimap, ?> directItems; + // Items which should not be propagated to dependents. private final ImmutableMap, NestedSet> nonPropagatedItems; + + // Items which should be passed to strictly direct dependers, but not transitive dependers. + private final ImmutableMap, NestedSet> strictDependencyItems; + /** All keys in ObjcProvider that will be passed in the corresponding Skylark provider. */ static final ImmutableList> KEYS_FOR_SKYLARK = ImmutableList.>of( @@ -401,6 +418,22 @@ public enum Flag { XCDATAMODEL, XIB); + /** + * Keys that should be kept as directItems. This is limited to a few keys that have larger + * performance implications when flattened in a transitive fashion and/or require non-transitive + * access (e.g. what module map did a target generate?). + * + *

Keys: + * + *

+ */ + static final ImmutableSet> KEYS_FOR_DIRECT = + ImmutableSet.>of(HEADER, MODULE_MAP, SOURCE); + @Override public NestedSet assetCatalog() { return get(ASSET_CATALOG); @@ -448,6 +481,11 @@ public NestedSet header() { return get(HEADER); } + @Override + public SkylarkList directHeaders() { + return getDirect(HEADER); + } + @Override public NestedSet importedLibrary() { return get(IMPORTED_LIBRARY); @@ -513,6 +551,11 @@ public NestedSet moduleMap() { return get(MODULE_MAP); } + @Override + public SkylarkList directModuleMaps() { + return getDirect(MODULE_MAP); + } + @Override public NestedSet multiArchDynamicLibraries() { return get(MULTI_ARCH_DYNAMIC_LIBRARIES); @@ -549,6 +592,11 @@ public NestedSet source() { return get(SOURCE); } + @Override + public SkylarkList directSources() { + return getDirect(SOURCE); + } + @Override public NestedSet staticFrameworkFile() { return get(STATIC_FRAMEWORK_FILE); @@ -652,9 +700,6 @@ static boolean isDeprecatedResourceKey(Key key) { return DEPRECATED_RESOURCE_KEYS.contains(key); } - // Items which should be passed to strictly direct dependers, but not transitive dependers. - private final ImmutableMap, NestedSet> strictDependencyItems; - /** Skylark constructor and identifier for ObjcProvider. */ public static final BuiltinProvider SKYLARK_CONSTRUCTOR = new Constructor(); @@ -662,12 +707,14 @@ private ObjcProvider( StarlarkSemantics semantics, ImmutableMap, NestedSet> items, ImmutableMap, NestedSet> nonPropagatedItems, - ImmutableMap, NestedSet> strictDependencyItems) { + ImmutableMap, NestedSet> strictDependencyItems, + ImmutableListMultimap, ?> directItems) { super(SKYLARK_CONSTRUCTOR, Location.BUILTIN); this.semantics = semantics; this.items = Preconditions.checkNotNull(items); this.nonPropagatedItems = Preconditions.checkNotNull(nonPropagatedItems); this.strictDependencyItems = Preconditions.checkNotNull(strictDependencyItems); + this.directItems = Preconditions.checkNotNull(directItems); } /** @@ -689,6 +736,15 @@ public NestedSet get(Key key) { return builder.build(); } + /** All direct artifacts, bundleable files, etc. of the type specified by {@code key}. */ + @SuppressWarnings({"rawtypes", "unchecked"}) + public SkylarkList getDirect(Key key) { + if (directItems.containsKey(key)) { + return SkylarkList.createImmutable((List) directItems.get(key)); + } + return SkylarkList.createImmutable(ImmutableList.of()); + } + /** * Returns all keys that have at least one value in this provider (values may be propagable, * non-propagable, or strict). @@ -998,6 +1054,10 @@ public static final class Builder { private final Map, NestedSetBuilder> nonPropagatedItems = new HashMap<>(); private final Map, NestedSetBuilder> strictDependencyItems = new HashMap<>(); + // Only includes items or lists added directly, never flattens any NestedSets. + private final ImmutableListMultimap.Builder, ?> directItems = + new ImmutableListMultimap.Builder<>(); + public Builder(StarlarkSemantics semantics) { this.starlarkSemantics = semantics; } @@ -1013,8 +1073,14 @@ private void uncheckedAddAll(Key key, Iterable toAdd, Map, NestedSetBuild } @SuppressWarnings({"rawtypes", "unchecked"}) - private void uncheckedAddTransitive(Key key, NestedSet toAdd, - Map, NestedSetBuilder> set) { + private void uncheckedAddAllDirect( + Key key, Iterable toAdd, ImmutableListMultimap.Builder, ?> builder) { + builder.putAll(key, (Iterable) toAdd); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private void uncheckedAddTransitive( + Key key, NestedSet toAdd, Map, NestedSetBuilder> set) { maybeAddEmptyBuilder(set, key); set.get(key).addTransitive(toAdd); } @@ -1088,6 +1154,9 @@ public Builder addAsDirectDeps(ObjcProvider provider) { */ public Builder add(Key key, E toAdd) { uncheckedAddAll(key, ImmutableList.of(toAdd), this.items); + if (ObjcProvider.KEYS_FOR_DIRECT.contains(key)) { + uncheckedAddAllDirect(key, ImmutableList.of(toAdd), this.directItems); + } return this; } @@ -1096,6 +1165,9 @@ public Builder add(Key key, E toAdd) { */ public Builder addAll(Key key, Iterable toAdd) { uncheckedAddAll(key, toAdd, this.items); + if (ObjcProvider.KEYS_FOR_DIRECT.contains(key)) { + uncheckedAddAllDirect(key, toAdd, this.directItems); + } return this; } @@ -1124,11 +1196,15 @@ public Builder addAllForDirectDependents(Key key, Iterable t } /** - * Add elements in toAdd with the given key from skylark. An error is thrown if toAdd is not - * an appropriate SkylarkNestedSet. + * Add elements in toAdd with the given key from skylark. An error is thrown if toAdd is not an + * appropriate SkylarkNestedSet. */ - void addElementsFromSkylark(Key key, Object toAdd) { - uncheckedAddAll(key, ObjcProviderSkylarkConverters.convertToJava(key, toAdd), this.items); + void addElementsFromSkylark(Key key, Object skylarkToAdd) { + Iterable toAdd = ObjcProviderSkylarkConverters.convertToJava(key, skylarkToAdd); + uncheckedAddAll(key, toAdd, this.items); + if (ObjcProvider.KEYS_FOR_DIRECT.contains(key)) { + uncheckedAddAllDirect(key, toAdd, this.directItems); + } } /** @@ -1203,7 +1279,8 @@ public ObjcProvider build() { starlarkSemantics, propagatedBuilder.build(), nonPropagatedBuilder.build(), - strictDependencyBuilder.build()); + strictDependencyBuilder.build(), + directItems.build()); } } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java index ee1c9238bd4e56..55540050e21093 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; +import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.StarlarkSemantics.FlagIdentifier; @@ -97,6 +98,14 @@ public interface ObjcProviderApi extends SkylarkValue ) public NestedSet header(); + @SkylarkCallable( + name = "direct_headers", + structField = true, + doc = + "Header files from this target directly (no transitive headers). " + + "These may be either public or private headers.") + public SkylarkList directHeaders(); + @SkylarkCallable(name = "imported_library", structField = true, doc = "Imported precompiled static libraries (.a files) to be linked into the binary." @@ -183,6 +192,14 @@ public interface ObjcProviderApi extends SkylarkValue ) public NestedSet moduleMap(); + @SkylarkCallable( + name = "direct_module_maps", + structField = true, + doc = + "Module map files from this target directly (no transitive module maps). " + + "Used to enforce proper use of private header files and for Swift compilation.") + public SkylarkList directModuleMaps(); + @SkylarkCallable(name = "multi_arch_dynamic_libraries", structField = true, doc = "Combined-architecture dynamic libraries to include in the final bundle." @@ -227,6 +244,12 @@ public interface ObjcProviderApi extends SkylarkValue ) public NestedSet source(); + @SkylarkCallable( + name = "direct_sources", + structField = true, + doc = "All direct source files from this target (no transitive files).") + public SkylarkList directSources(); + @SkylarkCallable( name = "static_framework_file", structField = true, diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeObjcProvider.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeObjcProvider.java index 2a668ed38738a7..03c3ace91a2553 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeObjcProvider.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/apple/FakeObjcProvider.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.skylarkbuildapi.FileApi; import com.google.devtools.build.lib.skylarkbuildapi.apple.ObjcProviderApi; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; +import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; /** @@ -70,6 +71,11 @@ public NestedSet header() { return null; } + @Override + public SkylarkList directHeaders() { + return null; + } + @Override public NestedSet importedLibrary() { return null; @@ -135,6 +141,11 @@ public NestedSet moduleMap() { return null; } + @Override + public SkylarkList directModuleMaps() { + return null; + } + @Override public NestedSet multiArchDynamicLibraries() { return null; @@ -170,6 +181,11 @@ public NestedSet source() { return null; } + @Override + public SkylarkList directSources() { + return null; + } + @Override public NestedSet staticFrameworkFile() { return null; diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BUILD b/src/test/java/com/google/devtools/build/lib/rules/objc/BUILD index a37cb525294ffc..e1553485e232cf 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BUILD @@ -26,6 +26,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/lib/rules/objc", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:bundlemerge_java_proto", "//src/main/protobuf:plmerge_java_proto", diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java index 3f2727760da0c9..f5ae1924e4a809 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java @@ -19,12 +19,18 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key; +import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.lang.reflect.Field; import java.lang.reflect.Modifier; -import java.util.ArrayList; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -38,12 +44,111 @@ private static ObjcProvider.Builder objcProviderBuilder() { return new ObjcProvider.Builder(StarlarkSemantics.DEFAULT_SEMANTICS); } + private static ImmutableList> getAllKeys() throws Exception { + ImmutableList.Builder> builder = new ImmutableList.Builder<>(); + for (Field field : ObjcProvider.class.getDeclaredFields()) { + if (Modifier.isStatic(field.getModifiers()) && field.getType() == ObjcProvider.Key.class) { + builder.add((ObjcProvider.Key) field.get(null)); + } + } + return builder.build(); + } + + private static Artifact createArtifact(String path) { + return new Artifact( + PathFragment.create(path), + ArtifactRoot.asSourceRoot(Root.absoluteRoot(new InMemoryFileSystem()))); + } + @Test public void emptyProvider() { ObjcProvider empty = objcProviderBuilder().build(); assertThat(empty.get(ObjcProvider.SDK_DYLIB)).isEmpty(); } + @Test + public void directFieldsDontPropagateTransitively() { + Artifact leafArtifact = createArtifact("/main.m"); + ObjcProvider leaf = objcProviderBuilder().add(ObjcProvider.SOURCE, leafArtifact).build(); + + Artifact rootArtifact = createArtifact("/root.m"); + ObjcProvider root = + objcProviderBuilder() + .add(ObjcProvider.SOURCE, rootArtifact) + .addTransitiveAndPropagate(leaf) + .build(); + assertThat(root.getDirect(ObjcProvider.SOURCE)).containsExactly(rootArtifact); + } + + @Test + public void directFieldsSingleAdd() { + Artifact source = createArtifact("/main.m"); + Artifact header = createArtifact("/Foo.h"); + Artifact module = createArtifact("/module.modulemap"); + ObjcProvider provider = + objcProviderBuilder() + .add(ObjcProvider.SOURCE, source) + .add(ObjcProvider.HEADER, header) + .add(ObjcProvider.MODULE_MAP, module) + .build(); + assertThat(provider.getDirect(ObjcProvider.SOURCE)).containsExactly(source); + assertThat(provider.getDirect(ObjcProvider.HEADER)).containsExactly(header); + assertThat(provider.getDirect(ObjcProvider.MODULE_MAP)).containsExactly(module); + } + + @Test + public void directFieldsAddAll() { + ImmutableList artifacts = + ImmutableList.of(createArtifact("/foo"), createArtifact("/bar")); + ObjcProvider provider = + objcProviderBuilder() + .addAll(ObjcProvider.SOURCE, artifacts) + .addAll(ObjcProvider.HEADER, artifacts) + .addAll(ObjcProvider.MODULE_MAP, artifacts) + .build(); + assertThat(provider.getDirect(ObjcProvider.SOURCE)).containsExactlyElementsIn(artifacts); + assertThat(provider.getDirect(ObjcProvider.HEADER)).containsExactlyElementsIn(artifacts); + assertThat(provider.getDirect(ObjcProvider.MODULE_MAP)).containsExactlyElementsIn(artifacts); + } + + @Test + public void directFieldsAddFromSkylark() { + ImmutableList artifacts = + ImmutableList.of(createArtifact("/foo"), createArtifact("/bar")); + SkylarkNestedSet set = + SkylarkNestedSet.of(Artifact.class, NestedSetBuilder.wrap(Order.STABLE_ORDER, artifacts)); + ObjcProvider.Builder builder = objcProviderBuilder(); + builder.addElementsFromSkylark(ObjcProvider.SOURCE, set); + builder.addElementsFromSkylark(ObjcProvider.HEADER, set); + builder.addElementsFromSkylark(ObjcProvider.MODULE_MAP, set); + ObjcProvider provider = builder.build(); + assertThat(provider.getDirect(ObjcProvider.SOURCE)).containsExactlyElementsIn(artifacts); + assertThat(provider.getDirect(ObjcProvider.HEADER)).containsExactlyElementsIn(artifacts); + assertThat(provider.getDirect(ObjcProvider.MODULE_MAP)).containsExactlyElementsIn(artifacts); + } + + @Test + @SuppressWarnings("unchecked") + public void directFieldsLimitedToCertainKeys() throws Exception { + ObjcProvider.Builder builder = objcProviderBuilder(); + ImmutableList values = ImmutableList.of("dummy", "fooey"); + + List> allKeys = getAllKeys(); + for (ObjcProvider.Key key : allKeys) { + // Use a List without a generic type to trick the compiler into allowing strings. + builder.addAll(key, (List) values); + } + ObjcProvider provider = builder.build(); + + for (ObjcProvider.Key key : allKeys) { + if (ObjcProvider.KEYS_FOR_DIRECT.contains(key)) { + assertThat(provider.getDirect(key)).containsExactlyElementsIn(values); + } else { + assertThat(provider.getDirect(key)).isEmpty(); + } + } + } + @Test public void onlyPropagatesProvider() { ObjcProvider onlyPropagates = objcProviderBuilder() @@ -101,19 +206,12 @@ public void strictDependencyDoesNotPropagateMoreThanOneLevelOnSkylark() { @Test public void keysExportedToSkylark() throws Exception { - List keyFields = new ArrayList<>(); - for (Field field : ObjcProvider.class.getDeclaredFields()) { - if (Modifier.isStatic(field.getModifiers()) && field.getType() == ObjcProvider.Key.class) { - keyFields.add(field); - } - } ImmutableSet> allRegisteredKeys = ImmutableSet.>builder() .addAll(ObjcProvider.KEYS_FOR_SKYLARK) .addAll(ObjcProvider.KEYS_NOT_IN_SKYLARK) .build(); - for (Field field : keyFields) { - ObjcProvider.Key key = (Key) field.get(null); + for (ObjcProvider.Key key : getAllKeys()) { assertWithMessage("Key %s must either be exposed to skylark or explicitly blacklisted", key.getSkylarkKeyName()).that(allRegisteredKeys).contains(key); }