Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flip --incompatible_disable_objc_library_transition #19433

Closed
wants to merge 1 commit into from
Closed

Flip --incompatible_disable_objc_library_transition #19433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ public final class BuildLanguageOptions extends OptionsBase {

@Option(
name = "incompatible_disable_objc_library_transition",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
Expand Down Expand Up @@ -913,7 +913,7 @@ public StarlarkSemantics toStarlarkSemantics() {
public static final String INCOMPATIBLE_OBJC_PROVIDER_REMOVE_LINKING_INFO =
"-incompatible_objc_provider_remove_linking_info";
public static final String INCOMPATIBLE_DISABLE_OBJC_LIBRARY_TRANSITION =
"-incompatible_disable_objc_library_transition";
"+incompatible_disable_objc_library_transition";
public static final String INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES =
"-incompatible_fail_on_unknown_attributes";

Expand Down
2 changes: 2 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,7 @@ protected Artifact j2objcArchive(String j2objcLibraryTarget, String javaTargetNa

@Test
public void testJ2ObjcInformationExportedFromObjcLibrary() throws Exception {
setBuildLanguageOptions("--incompatible_disable_objc_library_transition=false");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flipping it back on these 2 was a cop out, it would be great if someone who knew the impact of j2objc changes here could review and recommend the right solution

scratch.file("app/lib.m");
scratch.file(
"app/BUILD",
Expand Down Expand Up @@ -720,6 +721,7 @@ public void testJ2ObjcInformationExportedFromObjcLibrary() throws Exception {

@Test
public void testJ2ObjcInfoExportedInObjcLibraryFromRuntimeDeps() throws Exception {
setBuildLanguageOptions("--incompatible_disable_objc_library_transition=false");
scratch.file("app/lib.m");
scratch.file(
"app/BUILD",
Expand Down
13 changes: 8 additions & 5 deletions src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ public void testAllowVariousNonBlacklistedTypesInHeaders() throws Exception {

@Test
public void testAppleSdkVersionEnv() throws Exception {
useConfiguration("--apple_platform_type=ios");
useConfiguration("--apple_platform_type=ios", "--cpu=ios_x86_64");
createLibraryTargetWriter("//objc:lib")
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
.setAndCreateFiles("hdrs", "c.h")
Expand All @@ -1028,7 +1028,9 @@ public void testAppleSdkVersionEnv() throws Exception {

@Test
public void testNonDefaultAppleSdkVersionEnv() throws Exception {
useConfiguration("--apple_platform_type=ios", "--ios_sdk_version=8.1");
useConfiguration(
"--apple_platform_type=ios", "--ios_sdk_version=8.1",
"--cpu=ios_x86_64");

createLibraryTargetWriter("//objc:lib")
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
Expand Down Expand Up @@ -1109,7 +1111,7 @@ public void testSdkIncludesUsedInCompileAction() throws Exception {

@Test
public void testCompilationActionsWithPch() throws Exception {
useConfiguration("--apple_platform_type=ios");
useConfiguration("--apple_platform_type=ios", "--cpu=ios_x86_64");
scratch.file("objc/foo.pch");
createLibraryTargetWriter("//objc:lib")
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
Expand Down Expand Up @@ -1411,7 +1413,7 @@ public void testUsesDotdPruning() throws Exception {

@Test
public void testAppleSdkDefaultPlatformEnv() throws Exception {
useConfiguration("--apple_platform_type=ios");
useConfiguration("--apple_platform_type=ios", "--cpu=ios_x86_64");
createLibraryTargetWriter("//objc:lib")
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
.setAndCreateFiles("hdrs", "c.h")
Expand Down Expand Up @@ -1567,6 +1569,7 @@ public void testFilesToCompileOutputGroup() throws Exception {
public void testDefaultEnabledFeatureIsUsed() throws Exception {
// Although using --cpu=ios_x86_64, it transitions to darwin_x86_64, so the actual
// cc_toolchain in use will be the darwin_x86_64 one.
setBuildLanguageOptions("--incompatible_disable_objc_library_transition=false");
MockObjcSupport.setupCcToolchainConfig(
mockToolsConfig, MockObjcSupport.darwinX86_64().withFeatures("default_feature"));
useConfiguration("--cpu=ios_x86_64");
Expand Down Expand Up @@ -2075,7 +2078,7 @@ public void testPassesDependenciesStaticLibrariesInCcInfo() throws Exception {
.flatMap(List::stream)
.map(LibraryToLink::getStaticLibrary)
.collect(toImmutableList())))
.contains("/ x/libbaz.a");
.containsExactly("bin x/libbar.a", "bin x/libfoo.a", "bin x/libbaz.a");
}

@Test
Expand Down
9 changes: 7 additions & 2 deletions src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,18 +933,22 @@ protected void checkClangCoptsForCompilationMode(
switch (codeCoverageMode) {
case NONE:
useConfiguration(
"--apple_platform_type=ios", "--compilation_mode=" + compilationModeFlag(mode));
"--cpu=ios_x86_64",
"--apple_platform_type=ios",
"--compilation_mode=" + compilationModeFlag(mode));
break;
case GCOV:
allExpectedCoptsBuilder.addAll(CompilationSupport.CLANG_GCOV_COVERAGE_FLAGS);
useConfiguration(
"--cpu=ios_x86_64",
"--apple_platform_type=ios",
"--collect_code_coverage",
"--compilation_mode=" + compilationModeFlag(mode));
break;
case LLVMCOV:
allExpectedCoptsBuilder.addAll(CompilationSupport.CLANG_LLVM_COVERAGE_FLAGS);
useConfiguration(
"--cpu=ios_x86_64",
"--apple_platform_type=ios",
"--collect_code_coverage",
"--experimental_use_llvm_covmap",
Expand All @@ -967,7 +971,8 @@ protected void checkClangCoptsForDebugModeWithoutGlib(RuleType ruleType) throws
.addAll(ObjcConfiguration.DBG_COPTS);

useConfiguration(
"--apple_platform_type=ios", "--compilation_mode=dbg", "--objc_debug_with_GLIBCXX=false");
"--apple_platform_type=ios", "--compilation_mode=dbg", "--objc_debug_with_GLIBCXX=false",
"--cpu=ios_x86_64");
scratch.file("x/a.m");
ruleType.scratchTarget(scratch, "srcs", "['a.m']");

Expand Down
Loading