Always use exactly two components in Xcode version feature
When setting the standard Xcode version feature (see
commit eb952d08f76afa907bb34eaf7e1a69899102c523), always use exactly two
components for the version number. Grouping all point releases together
simplifies writing CROSSTOOLs.
Also fix a bug in MockObjcSupport that prevented Xcode version selection
in tests from working properly.
RELNOTES:
The standard `xcode_VERSION` feature now always uses exactly two
components in the version, even if you specify `--xcode_version` with
more or fewer than two.
PiperOrigin-RevId: 208877588
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
index 9919ff7..e063f3e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.rules.apple;
import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ComparisonChain;
@@ -169,6 +170,31 @@
}
/**
+ * Returns the string representation of this dotted version, padded or truncated to the specified
+ * number of components.
+ *
+ * <p>For example, a dotted version of "7.3.0" will return "7" if one is requested, "7.3" if two
+ * are requested, "7.3.0" if three are requested, and "7.3.0.0" if four are requested.
+ *
+ * @param numComponents a positive number of dot-separated numbers that should be present in the
+ * returned string representation
+ */
+ public String toStringWithComponents(int numComponents) {
+ Preconditions.checkArgument(numComponents > 0,
+ "Can't serialize as a version with %s components", numComponents);
+ ImmutableList.Builder<Component> stringComponents = ImmutableList.builder();
+ if (numComponents <= components.size()) {
+ stringComponents.addAll(components.subList(0, numComponents));
+ } else {
+ stringComponents.addAll(components);
+ for (int i = components.size(); i < numComponents; i++) {
+ stringComponents.add(ZERO_COMPONENT);
+ }
+ }
+ return Joiner.on('.').join(stringComponents.build());
+ }
+
+ /**
* Returns the string representation of this dotted version, padded to a minimum number of
* components if the string representation does not already contain that many components.
*
@@ -183,14 +209,7 @@
* the returned string representation
*/
public String toStringWithMinimumComponents(int numMinComponents) {
- ImmutableList.Builder<Component> stringComponents = ImmutableList.builder();
- stringComponents.addAll(components);
- int numComponents = Math.max(this.numOriginalComponents, numMinComponents);
- int zeroesToPad = numComponents - components.size();
- for (int i = 0; i < zeroesToPad; i++) {
- stringComponents.add(ZERO_COMPONENT);
- }
- return Joiner.on('.').join(stringComponents.build());
+ return toStringWithComponents(Math.max(this.numOriginalComponents, numMinComponents));
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index f7e7a4a..4093804 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -553,8 +553,11 @@
// next release.
activatedCrosstoolSelectables.add(NO_DSYM_ZIPS_FEATURE_NAME);
+ // Add a feature identifying the Xcode version so CROSSTOOL authors can enable flags for
+ // particular versions of Xcode. To ensure consistency across platforms, use exactly two
+ // components in the version number.
activatedCrosstoolSelectables.add(XCODE_VERSION_FEATURE_NAME_PREFIX
- + XcodeConfig.getXcodeVersion(ruleContext).toStringWithMinimumComponents(2));
+ + XcodeConfig.getXcodeVersion(ruleContext).toStringWithComponents(2));
activatedCrosstoolSelectables.addAll(ruleContext.getFeatures());
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MOCK_OSX_CROSSTOOL b/src/test/java/com/google/devtools/build/lib/packages/util/MOCK_OSX_CROSSTOOL
index 0986112..9bd8202 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MOCK_OSX_CROSSTOOL
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MOCK_OSX_CROSSTOOL
@@ -648,6 +648,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -663,6 +678,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -2389,6 +2419,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -2404,6 +2449,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -4125,6 +4185,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -4140,6 +4215,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -5861,6 +5951,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -5876,6 +5981,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -7601,6 +7721,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -7616,6 +7751,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -9347,6 +9497,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -9362,6 +9527,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -11096,6 +11276,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -11111,6 +11306,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -12872,6 +13082,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -12887,6 +13112,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -14618,6 +14858,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -14633,6 +14888,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -16384,6 +16654,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -16399,6 +16684,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -18153,6 +18453,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -18168,6 +18483,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
@@ -19949,6 +20279,21 @@
}
}
feature {
+ name: "xcode_5.0"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_5.0"
+ }
+ }
+ }
+ feature {
name: "xcode_5.8"
flag_set {
action: "preprocess-assemble"
@@ -19964,6 +20309,21 @@
}
}
feature {
+ name: "xcode_7.3"
+ flag_set {
+ action: "preprocess-assemble"
+ action: "c-compile"
+ action: "c++-compile"
+ action: "c++-header-parsing"
+ action: "c++-header-preprocessing"
+ action: "objc-compile"
+ action: "objc++-compile"
+ flag_group {
+ flag: "-DXCODE_FEATURE_FOR_TESTING=xcode_7.3"
+ }
+ }
+ }
+ feature {
name: "framework_paths"
flag_set {
action: "objc-compile"
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java
index 3b10f41..b9e7ce0 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java
@@ -116,7 +116,7 @@
")",
"xcode_config(name = 'host_xcodes',",
" default = ':version7_3_1',",
- " versions = [':version7_3_1', 'version5_0', 'version7_3', 'version5_8'])",
+ " versions = [':version7_3_1', ':version5_0', ':version7_3', ':version5_8', ':version5'])",
"xcode_version(",
" name = 'version7_3_1',",
" version = '" + DEFAULT_XCODE_VERSION + "',",
@@ -134,6 +134,10 @@
" name = 'version5_8',",
" version = '5.8',",
")",
+ "xcode_version(",
+ " name = 'version5',",
+ " version = '5',",
+ ")",
"objc_library(name = 'dummy_lib', srcs = ['objc_dummy.mm'])",
"alias(name = 'protobuf_lib', actual = '//objcproto:protobuf_lib')");
// If the bazel tools repository is not in the workspace, also create a workspace tools/objc
diff --git a/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java b/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java
index dbcf87a..285dc2f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java
@@ -60,6 +60,33 @@
}
@Test
+ public void testToStringWithComponents() throws Exception {
+ DottedVersion dottedVersion = DottedVersion.fromString("42.8");
+ assertThat(dottedVersion.toStringWithComponents(1)).isEqualTo("42");
+ assertThat(dottedVersion.toStringWithComponents(2)).isEqualTo("42.8");
+ assertThat(dottedVersion.toStringWithComponents(3)).isEqualTo("42.8.0");
+ assertThat(dottedVersion.toStringWithComponents(4)).isEqualTo("42.8.0.0");
+ }
+
+ @Test
+ public void testToStringWithComponents_trailingZero() throws Exception {
+ DottedVersion dottedVersion = DottedVersion.fromString("4.3alpha3.0");
+ assertThat(dottedVersion.toStringWithComponents(1)).isEqualTo("4");
+ assertThat(dottedVersion.toStringWithComponents(2)).isEqualTo("4.3alpha3");
+ assertThat(dottedVersion.toStringWithComponents(3)).isEqualTo("4.3alpha3.0");
+ assertThat(dottedVersion.toStringWithComponents(4)).isEqualTo("4.3alpha3.0.0");
+ assertThat(dottedVersion.toStringWithComponents(5)).isEqualTo("4.3alpha3.0.0.0");
+ }
+
+ @Test
+ public void testToStringWithComponents_zeroComponent() throws Exception {
+ DottedVersion zeroComponent = DottedVersion.fromString("0");
+ assertThat(zeroComponent.toStringWithComponents(1)).isEqualTo("0");
+ assertThat(zeroComponent.toStringWithComponents(2)).isEqualTo("0.0");
+ assertThat(zeroComponent.toStringWithComponents(3)).isEqualTo("0.0.0");
+ }
+
+ @Test
public void testToStringWithMinimumComponent() throws Exception {
DottedVersion dottedVersion = DottedVersion.fromString("42.8");
assertThat(dottedVersion.toStringWithMinimumComponents(0)).isEqualTo("42.8");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
index f0eb7ec..ce828e5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
@@ -1266,6 +1266,30 @@
}
@Test
+ public void testXcodeVersionFeatureTwoComponentsTooMany() throws Exception {
+ useConfiguration("--xcode_version=7.3.1");
+
+ createLibraryTargetWriter("//objc:lib")
+ .setAndCreateFiles("srcs", "a.m")
+ .write();
+ CommandAction action = compileAction("//objc:lib", "a.o");
+
+ assertThat(action.getArguments()).contains("-DXCODE_FEATURE_FOR_TESTING=xcode_7.3");
+ }
+
+ @Test
+ public void testXcodeVersionFeatureTwoComponentsTooFew() throws Exception {
+ useConfiguration("--xcode_version=5");
+
+ createLibraryTargetWriter("//objc:lib")
+ .setAndCreateFiles("srcs", "a.m")
+ .write();
+ CommandAction action = compileAction("//objc:lib", "a.o");
+
+ assertThat(action.getArguments()).contains("-DXCODE_FEATURE_FOR_TESTING=xcode_5.0");
+ }
+
+ @Test
public void testIosSdkVersionCannotBeDefinedButEmpty() throws Exception {
try {
useConfiguration("--ios_sdk_version=");
