From 45197b72926ffac133c7ae1465909e4a857b2e65 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 22 Nov 2022 10:25:27 -0800 Subject: [PATCH] Subtract imported_library in subtractSubtrees This is needed for some upcoming changes to migrate linking info to CcInfo. During the migration we will put the linking info in both CcInfo and ObjcProvider, and it is possible in some code path (through swift_library, fwiw) for an imported_library to become a regular library. For avoid_deps to work, avoid_deps needs to add imported_library to the list of libraries to subtract. PiperOrigin-RevId: 490275745 Change-Id: I013950a2ffe988013ca1613d8b7802ca379d3170 --- .../build/lib/rules/objc/ObjcProvider.java | 3 + .../objc/AppleBinaryStarlarkApiTest.java | 5 ++ .../rules/objc/AppleDynamicLibraryTest.java | 6 ++ .../lib/rules/objc/ObjcRuleTestCase.java | 61 +++++++++++++++++++ 4 files changed, 75 insertions(+) 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 9e93f264d6acde..f2f23d25a1398a 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 @@ -560,6 +560,9 @@ public ObjcProvider subtractSubtrees( for (Artifact libraryToAvoid : avoidProvider.getPropagable(LIBRARY).toList()) { avoidLibrariesSet.add(libraryToAvoid.getRunfilesPath()); } + for (Artifact libraryToAvoid : avoidProvider.getPropagable(IMPORTED_LIBRARY).toList()) { + avoidLibrariesSet.add(libraryToAvoid.getRunfilesPath()); + } } ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder(semantics); for (Key key : items.keySet()) { diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java index 3845ceeea10a7d..8095716c15be3e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java @@ -565,6 +565,11 @@ public void testAvoidDepsObjects_avoidViaCcLibrary() throws Exception { checkAvoidDepsObjects_avoidViaCcLibrary(getRuleType()); } + @Test + public void testAvoidDepsSubtractsImportedLibrary() throws Exception { + checkAvoidDepsSubtractsImportedLibrary(getRuleType()); + } + @Test public void testBundleLoaderIsCorrectlyPassedToTheLinker() throws Exception { checkBundleLoaderIsCorrectlyPassedToTheLinker(getRuleType()); diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java index a30e40b0036d1f..de11f4d5824bd3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java @@ -118,6 +118,12 @@ public void testAvoidDepsObjects_avoidViaCcLibrary() throws Exception { checkAvoidDepsObjects_avoidViaCcLibrary(RULE_TYPE); } + @Override + @Test + public void testAvoidDepsSubtractsImportedLibrary() throws Exception { + checkAvoidDepsSubtractsImportedLibrary(RULE_TYPE); + } + @Override @Test public void testLipoBinaryAction() throws Exception { 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 7103c36711f8d6..6459c6522c0ef6 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 @@ -1777,6 +1777,67 @@ public void checkAvoidDepsObjects_avoidViaCcLibrary(RuleType ruleType) throws Ex .isNull(); } + public void checkAvoidDepsSubtractsImportedLibrary(RuleType ruleType) throws Exception { + if (!ruleType.getRuleTypeName().equals("apple_binary_starlark")) { + addAppleBinaryStarlarkRule(scratch); + } + + ruleType.scratchTarget( + scratch, "deps", "['//libs:objc_lib']", "avoid_deps", "['//libs:objc_avoid_lib']"); + + scratch.file( + "libs/defs.bzl", + "def _custom_library_impl(ctx):", + " return [", + " apple_common.new_objc_provider(", + " library=depset([ctx.file.library]),", + " ), CcInfo()", + " ]", + "custom_library = rule(", + " _custom_library_impl,", + " attrs={'library': attr.label(allow_single_file=True)},", + ")", + "def _custom_static_framework_import_impl(ctx):", + " return [", + " apple_common.new_objc_provider(", + " imported_library=depset([ctx.file.library]),", + " ), CcInfo()", + " ]", + "custom_static_framework_import = rule(", + " _custom_static_framework_import_impl,", + " attrs={'library': attr.label(allow_single_file=True)},", + ")"); + + scratch.file( + "libs/BUILD", + "load('//test_starlark:apple_binary_starlark.bzl', 'apple_binary_starlark')", + "load(':defs.bzl', 'custom_library', 'custom_static_framework_import')", + "objc_library(", + " name = 'objc_lib',", + " srcs = ['a.m'],", + " deps = [':framework_library'],", + ")", + "custom_library(", + " name = 'framework_library',", + " library = 'buzzbuzz.framework/buzzbuzz',", + ")", + "objc_library(", + " name = 'objc_avoid_lib',", + " srcs = ['b.m'],", + " deps = [':framework'],", + ")", + "custom_static_framework_import(", + " name = 'framework',", + " library = 'buzzbuzz.framework/buzzbuzz',", + ")"); + + Artifact binArtifact = + getFirstArtifactEndingWith(lipoBinAction("//x:x").getInputs(), "x/x_bin"); + Action action = getGeneratingAction(binArtifact); + assertThat(Artifact.toRootRelativePaths(action.getInputs())) + .doesNotContain("libs/buzzbuzz.framework/buzzbuzz"); + } + public void checkFilesToCompileOutputGroup(RuleType ruleType) throws Exception { ruleType.scratchTarget(scratch); ConfiguredTarget target = getConfiguredTarget("//x:x");