Skip to content

Commit

Permalink
Rollback of commit b66898e.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

This was accidentally submitted (why did tappresubmit not complain about no LGTM?!)

*** Original change description ***

Allow objc_{library,binary} to depend on cc_library.

This is an early version of support for this feature, likely still missing
a number of edge cases. However the basic functionality should work.

To allow a dependency from objc to cc, the following flags will have to be
passed to bazel:

--experimental_enable_objc_cc_deps --experimental_disable_java --cpu=ios_i386 --crosstool_top=//tools/objc/crosstool:crosstool

The feature is also compatible with --ios_multi_cpus, with the familiar
values f...

***

--
MOS_MIGRATED_REVID=94118959
  • Loading branch information
aragos authored and hanwen committed May 21, 2015
1 parent b66898e commit 828c544
Show file tree
Hide file tree
Showing 16 changed files with 31 additions and 149 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses.JavaRule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;

/**
* Common attributes for Java rules.
Expand All @@ -36,7 +34,6 @@ public final class BazelJavaLibraryRule implements RuleDefinition {
public RuleClass build(Builder builder, final RuleDefinitionEnvironment env) {

return builder
.requiresConfigurationFragments(JavaConfiguration.class, CppConfiguration.class)
/* <!-- #BLAZE_RULE(java_library).IMPLICIT_OUTPUTS -->
<ul>
<li><code>lib<var>name</var>.jar</code>: A Java archive containing the class files.</li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleClass.PackageNameConstraint;
import com.google.devtools.build.lib.packages.TriState;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.rules.java.Jvm;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.vfs.PathFragment;

Expand Down Expand Up @@ -67,9 +65,6 @@ public static final class IjarBaseRule implements RuleDefinition {
@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
return builder
// Fail analysis if --disable_java is enabled.
.failIfMissingConfigurationFragment()
.requiresConfigurationFragments(JavaConfiguration.class, Jvm.class)
.add(attr("$ijar", LABEL).cfg(HOST).exec().value(env.getLabel("//tools/defaults:ijar")))
.setPreferredDependencyPredicate(JavaSemantics.JAVA_SOURCE)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.java;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.RedirectChaser;
Expand Down Expand Up @@ -48,10 +47,6 @@ public JavaConfiguration create(ConfigurationEnvironment env, BuildOptions build
throws InvalidConfigurationException {
JavaOptions javaOptions = buildOptions.get(JavaOptions.class);

if (javaOptions.disableJava) {
return null;
}

Label javaToolchain = RedirectChaser.followRedirects(env, javaOptions.javaToolchain,
"java_toolchain");
return create(javaOptions, javaToolchain, cpuSupplier.getJavaCpu(buildOptions, env));
Expand All @@ -61,10 +56,9 @@ public JavaConfiguration create(ConfigurationEnvironment env, BuildOptions build
public Class<? extends Fragment> creates() {
return JavaConfiguration.class;
}

@VisibleForTesting

public JavaConfiguration create(JavaOptions javaOptions, Label javaToolchain, String javaCpu)
throws InvalidConfigurationException {
throws InvalidConfigurationException {

boolean generateJavaDeps = javaOptions.javaDeps ||
javaOptions.experimentalJavaClasspath != JavaClasspathMode.OFF;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;

/**
* A base rule for building the java_import rule.
Expand All @@ -37,7 +36,6 @@ public class JavaImportBaseRule implements RuleDefinition {
@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
return builder
.requiresConfigurationFragments(JavaConfiguration.class, CppConfiguration.class)
.add(attr(":host_jdk", LABEL)
.cfg(HOST)
.value(JavaSemantics.HOST_JDK))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,6 @@ public JavaClasspathModeConverter() {
help = "Check the listed constraint.")
public List<String> checkedConstraints;

@Option(name = "experimental_disable_java",
defaultValue = "false",
category = "undocumented",
help = "Disables java support entirely.")
public boolean disableJava;

@Override
public FragmentOptions getHost(boolean fallback) {
JavaOptions host = (JavaOptions) getDefault();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,6 @@ public interface JavaSemantics {
new LateBoundLabel<BuildConfiguration>(JAVA_TOOLCHAIN_LABEL) {
@Override
public Label getDefault(Rule rule, BuildConfiguration configuration) {
if (!configuration.hasFragment(JavaConfiguration.class)) {
return null;
}
return configuration.getFragment(JavaConfiguration.class).getToolchainLabel();
}
};
Expand Down Expand Up @@ -135,9 +132,6 @@ public Label getDefault(Rule rule, BuildConfiguration configuration) {
new LateBoundLabel<BuildConfiguration>(JDK_LABEL) {
@Override
public Label getDefault(Rule rule, BuildConfiguration configuration) {
if (!configuration.hasFragment(Jvm.class)) {
return null;
}
return configuration.getFragment(Jvm.class).getJvmLabel();
}
};
Expand All @@ -154,9 +148,6 @@ public boolean useHostConfiguration() {

@Override
public Label getDefault(Rule rule, BuildConfiguration configuration) {
if (!configuration.hasFragment(Jvm.class)) {
return null;
}
return configuration.getFragment(Jvm.class).getJvmLabel();
}
};
Expand All @@ -169,9 +160,6 @@ public Label getDefault(Rule rule, BuildConfiguration configuration) {
new LateBoundLabel<BuildConfiguration>() {
@Override
public Label getDefault(Rule rule, BuildConfiguration configuration) {
if (!configuration.hasFragment(JavaConfiguration.class)) {
return null;
}
return configuration.getFragment(JavaConfiguration.class).getJavaLauncherLabel();
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public JavaToolchainRule(ImmutableList<String> defaultJavacJvmOpts) {
@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
return builder.setUndocumented()
.requiresConfigurationFragments(JavaConfiguration.class)
/* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(source_version) -->
The Java source version (e.g., '6' or '7'). It specifies which set of code structures
are allowed in the Java source code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ public JvmConfigurationLoader(JavaCpuSupplier cpuSupplier) {
public Jvm create(ConfigurationEnvironment env, BuildOptions buildOptions)
throws InvalidConfigurationException {
JavaOptions javaOptions = buildOptions.get(JavaOptions.class);

if (javaOptions.disableJava) {
return null;
}

String javaHome = javaOptions.javaBase;
String cpu = cpuSupplier.getJavaCpu(buildOptions, env);
if (cpu == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
Expand Down Expand Up @@ -171,10 +169,6 @@ private ObjcCommon common(RuleContext ruleContext) {
.setCompilationArtifacts(compilationArtifacts)
.addDefines(ruleContext.getTokenizedStringListAttr("defines"))
.addDepObjcProviders(ruleContext.getPrerequisites("deps", Mode.TARGET, ObjcProvider.class))
.addDepCcHeaderProviders(
ruleContext.getPrerequisites("deps", Mode.TARGET, CppCompilationContext.class))
.addDepCcLinkProviders(
ruleContext.getPrerequisites("deps", Mode.TARGET, CcLinkParamsProvider.class))
.addDepObjcProviders(
ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.class))
.addNonPropagatedDepObjcProviders(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
import com.google.devtools.build.lib.rules.objc.XcodeProvider.Builder;
import com.google.devtools.build.lib.shell.ShellUtils;
Expand Down Expand Up @@ -79,8 +78,6 @@ final class CompilationSupport {
static final ImmutableList<String> CLANG_COVERAGE_FLAGS =
ImmutableList.of("-fprofile-arcs", "-ftest-coverage", "-fprofile-dir=./coverage_output");

private static final String FRAMEWORK_SUFFIX = ".framework";

/**
* Iterable wrapper providing strong type safety for arguments to binary linking.
*/
Expand Down Expand Up @@ -285,37 +282,27 @@ private void registerLinkAction(ObjcProvider objcProvider, ExtraLinkArgs extraLi
Artifact linkedBinary =
ObjcRuleClasses.intermediateArtifacts(ruleContext).singleArchitectureBinary();

ImmutableList<Artifact> ccLibraries = ccLibraries(objcProvider);
ruleContext.registerAction(
ObjcRuleClasses.spawnOnDarwinActionBuilder()
.setMnemonic("ObjcLink")
.setShellCommand(ImmutableList.of("/bin/bash", "-c"))
.setCommandLine(
linkCommandLine(extraLinkArgs, objcProvider, linkedBinary, dsymBundle, ccLibraries))
.setCommandLine(linkCommandLine(extraLinkArgs, objcProvider, linkedBinary, dsymBundle))
.addOutput(linkedBinary)
.addOutputs(dsymBundle.asSet())
.addTransitiveInputs(objcProvider.get(LIBRARY))
.addTransitiveInputs(objcProvider.get(IMPORTED_LIBRARY))
.addTransitiveInputs(objcProvider.get(FRAMEWORK_FILE))
.addInputs(ccLibraries)
.addInputs(extraLinkInputs)
.build(ruleContext));
}

private ImmutableList<Artifact> ccLibraries(ObjcProvider objcProvider) {
ImmutableList.Builder<Artifact> ccLibraryBuilder = ImmutableList.builder();
for (LinkerInputs.LibraryToLink libraryToLink : objcProvider.get(ObjcProvider.CC_LIBRARY)) {
ccLibraryBuilder.add(libraryToLink.getArtifact());
}
return ccLibraryBuilder.build();
}
private static final String FRAMEWORK_SUFFIX = ".framework";

private CommandLine linkCommandLine(ExtraLinkArgs extraLinkArgs,
ObjcProvider objcProvider, Artifact linkedBinary, Optional<Artifact> dsymBundle,
ImmutableList<Artifact> ccLibraries) {
ObjcProvider objcProvider, Artifact linkedBinary, Optional<Artifact> dsymBundle) {
ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);

CustomCommandLine.Builder commandLine = CustomCommandLine.builder();
final CustomCommandLine.Builder commandLine = CustomCommandLine.builder();

if (objcProvider.is(USES_CPP)) {
commandLine
Expand All @@ -335,7 +322,6 @@ private CommandLine linkCommandLine(ExtraLinkArgs extraLinkArgs,
.addExecPath("-o", linkedBinary)
.addExecPaths(objcProvider.get(LIBRARY))
.addExecPaths(objcProvider.get(IMPORTED_LIBRARY))
.addExecPaths(ccLibraries)
.add(dylibPaths(objcProvider))
.addBeforeEach("-force_load", Artifact.toExecPaths(objcProvider.get(FORCE_LOAD_LIBRARY)))
.add(extraLinkArgs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,6 @@ public class ObjcCommandLineOptions extends FragmentOptions {
help = "Whether to add include path entries for every individual proto file.")
public boolean perProtoIncludes;

@Option(name = "experimental_enable_objc_cc_deps",
defaultValue = "false",
category = "undocumented",
help = "Allows objc_* rules to depend on cc_library and causes any objc dependencies to be "
+ "built with --cpu set to \"ios_<--ios_cpu>\" for any values in --ios_multi_cpu. For "
+ "most values of ios_cpu this means that this option requires the use of "
+ "--experimental_disable_java as java does not support these new --cpu values. Note "
+ "that this may affect genrules if they depend on java make variables.")
public boolean enableCcDeps;

// This option exists because two configurations are not allowed to have the same cache key
// (partially derived from options). Since we have multiple transitions (see
// getPotentialSplitTransitions below) that may result in the same configuration values at runtime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.BREAKPAD_FILE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.BUNDLE_FILE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.BUNDLE_IMPORT_DIR;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.CC_LIBRARY;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DEFINE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FLAG;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FORCE_LOAD_FOR_XCODEGEN;
Expand Down Expand Up @@ -58,8 +57,6 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.cpp.CcCommon;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -240,8 +237,6 @@ static class Builder {
private Iterable<Artifact> extraImportLibraries = ImmutableList.of();
private Optional<Artifact> linkedBinary = Optional.absent();
private Optional<Artifact> breakpadFile = Optional.absent();
private Iterable<CppCompilationContext> depCcHeaderProviders = ImmutableList.of();
private Iterable<CcLinkParamsProvider> depCcLinkProviders = ImmutableList.of();

Builder(RuleContext context) {
this.context = Preconditions.checkNotNull(context);
Expand Down Expand Up @@ -359,22 +354,6 @@ Builder setBreakpadFile(Artifact breakpadFile) {
return this;
}

/**
* Sets information from {@code cc_library} dependencies to be used during compilation.
*/
public Builder addDepCcHeaderProviders(Iterable<CppCompilationContext> depCcHeaderProviders) {
this.depCcHeaderProviders = Iterables.concat(this.depCcHeaderProviders, depCcHeaderProviders);
return this;
}

/**
* Sets information from {@code cc_library} dependencies to be used during linking.
*/
public Builder addDepCcLinkProviders(Iterable<CcLinkParamsProvider> depCcLinkProviders) {
this.depCcLinkProviders = Iterables.concat(this.depCcLinkProviders, depCcLinkProviders);
return this;
}

ObjcCommon build() {
Iterable<BundleableFile> bundleImports = BundleableFile.bundleImportsFromRule(context);

Expand All @@ -394,15 +373,6 @@ ObjcCommon build() {
.addTransitiveAndPropagate(depObjcProviders)
.addTransitiveWithoutPropagating(directDepObjcProviders);

for (CppCompilationContext headerProvider : depCcHeaderProviders) {
// TODO(bazel-team): Also account for custom include settings to go into header search paths
objcProvider.addTransitiveAndPropagate(HEADER, headerProvider.getDeclaredIncludeSrcs());
}
for (CcLinkParamsProvider linkProvider : depCcLinkProviders) {
objcProvider.addTransitiveAndPropagate(
CC_LIBRARY, linkProvider.getCcLinkParams(true, false).getLibraries());
}

if (compilationAttributes.isPresent()) {
CompilationAttributes attributes = compilationAttributes.get();
ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;

Expand Down Expand Up @@ -70,10 +68,6 @@ static ObjcCommon common(RuleContext ruleContext, Iterable<SdkFramework> extraSd
ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.class))
.addNonPropagatedDepObjcProviders(ruleContext.getPrerequisites("non_propagated_deps",
Mode.TARGET, ObjcProvider.class))
.addDepCcHeaderProviders(
ruleContext.getPrerequisites("deps", Mode.TARGET, CppCompilationContext.class))
.addDepCcLinkProviders(
ruleContext.getPrerequisites("deps", Mode.TARGET, CcLinkParamsProvider.class))
.setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
.setAlwayslink(alwayslink)
.addExtraImportLibraries(extraImportLibraries)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.xcode.xcodegen.proto.XcodeGenProtos.TargetControl;

Expand Down Expand Up @@ -160,7 +159,7 @@ private Key(Order order) {
public static final Key<Bundling> NESTED_BUNDLE = new Key<>(STABLE_ORDER);

/**
* Artifact containing information on debug symbols.
* Artifact containing information on debug symbols
*/
public static final Key<Artifact> DEBUG_SYMBOLS = new Key<>(STABLE_ORDER);

Expand All @@ -185,11 +184,6 @@ private Key(Order order) {
*/
public static final Key<Artifact> STRINGS = new Key<>(STABLE_ORDER);

/**
* Linking information from cc dependencies.
*/
public static final Key<LinkerInputs.LibraryToLink> CC_LIBRARY = new Key<>(LINK_ORDER);

/**
* Flags that apply to a transitive build dependency tree. Each item in the enum corresponds to a
* flag. If the item is included in the key {@link #FLAG}, then the flag is considered set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,7 @@ public static class CompilingRule implements RuleDefinition {
"objc_import",
"objc_framework",
"objc_proto_library",
"j2objc_library",
"cc_library");
"j2objc_library");

@Override
public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
Expand Down
Loading

0 comments on commit 828c544

Please sign in to comment.