diff options
author | Daniel Wagner-Hall <danielwh@google.com> | 2015-03-10 14:41:40 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2015-03-10 17:27:06 +0000 |
commit | c00a757d613306f1f9b8cfc6b11745bff934b877 (patch) | |
tree | 34884567295a3e3d7f15a46f2a900c06444f6533 /src/main/java/com/google/devtools/build | |
parent | c513e114a0a5f97891433b80bf442c5e9acaf22b (diff) |
Allow ios_test to override the GCOV environment variable
--
MOS_MIGRATED_REVID=88218457
Diffstat (limited to 'src/main/java/com/google/devtools/build')
9 files changed, 112 insertions, 8 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java index 51122e2af3..5380cf7ca9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java @@ -15,12 +15,15 @@ package com.google.devtools.build.lib.analysis; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.OutputFile; import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider; import com.google.devtools.build.lib.rules.test.InstrumentedFilesProviderImpl; +import java.util.Map; + /** * A ConfiguredTarget for an OutputFile. */ @@ -64,6 +67,11 @@ public class OutputFileConfiguredTarget extends FileConfiguredTarget .getInstrumentationMetadataFiles(); } + @Override + public Map<String, String> getExtraEnv() { + return ImmutableMap.of(); + } + /** * Returns the corresponding provider from the generating rule, if it is non-null, or {@code * defaultValue} otherwise. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index 9cd41c3609..376045c382 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java @@ -163,9 +163,16 @@ public final class RuleConfiguredTargetBuilder { "Having more than 50 shards is indicative of poor test organization. " + "Please reduce the number of shards."); } - final TestParams testParams = new TestActionBuilder(ruleContext) + TestActionBuilder testActionBuilder = new TestActionBuilder(ruleContext); + InstrumentedFilesProvider instrumentedFilesProvider = + findProvider(InstrumentedFilesProvider.class); + if (instrumentedFilesProvider != null) { + testActionBuilder + .setInstrumentedFiles(instrumentedFilesProvider) + .setExtraEnv(instrumentedFilesProvider.getExtraEnv()); + } + final TestParams testParams = testActionBuilder .setFilesToRunProvider(filesToRunProvider) - .setInstrumentedFiles(findProvider(InstrumentedFilesProvider.class)) .setExecutionRequirements(findProvider(ExecutionInfoProvider.class)) .setShardCount(explicitShardCount) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java index 3473e4b742..bc2b862cbd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java @@ -19,10 +19,13 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.CompilationMode; +import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.xcode.common.Platform; import java.util.List; +import javax.annotation.Nullable; + /** * A compiler configuration containing flags required for Objective-C compilation. */ @@ -50,7 +53,18 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment { private final List<String> copts; private final CompilationMode compilationMode; - ObjcConfiguration(ObjcCommandLineOptions objcOptions, BuildConfiguration.Options options) { + // We only load this label if coverage mode is enabled. That is know as part of the + // BuildConfiguration. This label needs to be part of a configuration because only configurations + // can conditionally cause loading. + // This is referenced from a late bound attribute, and if loading wasn't forced in a + // configuration, the late bound attribute will fail to be initialized because it hasn't been + // loaded. + @Nullable private final Label gcovLabel; + + ObjcConfiguration( + ObjcCommandLineOptions objcOptions, + BuildConfiguration.Options options, + @Nullable Label gcovLabel) { this.iosSdkVersion = Preconditions.checkNotNull(objcOptions.iosSdkVersion, "iosSdkVersion"); this.iosMinimumOs = Preconditions.checkNotNull(objcOptions.iosMinimumOs, "iosMinimumOs"); this.iosSimulatorVersion = @@ -61,6 +75,7 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment { this.runMemleaks = objcOptions.runMemleaks; this.copts = ImmutableList.copyOf(objcOptions.copts); this.compilationMode = Preconditions.checkNotNull(options.compilationMode, "compilationMode"); + this.gcovLabel = gcovLabel; } public String getIosSdkVersion() { @@ -131,6 +146,14 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment { return copts; } + /** + * Returns the label of the gcov binary, used to get test coverage data. Null iff not in coverage + * mode. + */ + @Nullable public Label getGcovLabel() { + return gcovLabel; + } + @Override public String getName() { return "Objective-C"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java index 19713a346a..58b4f175c9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java @@ -15,10 +15,14 @@ package com.google.devtools.build.lib.rules.objc; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.packages.NoSuchPackageException; +import com.google.devtools.build.lib.packages.NoSuchTargetException; +import com.google.devtools.build.lib.syntax.Label; /** * A loader that creates ObjcConfiguration instances based on Objective-C configurations and @@ -28,8 +32,21 @@ public class ObjcConfigurationLoader implements ConfigurationFragmentFactory { @Override public ObjcConfiguration create(ConfigurationEnvironment env, BuildOptions buildOptions) throws InvalidConfigurationException { + Options options = buildOptions.get(BuildConfiguration.Options.class); + Label gcovLabel = null; + if (options.collectCodeCoverage) { + try { + // TODO(danielwh): Replace this with something from an objc_toolchain when it exists + gcovLabel = Label.parseAbsolute("//third_party/gcov:gcov_for_xcode"); + // Force the label to be loaded + env.getTarget(gcovLabel); + } catch (Label.SyntaxException | NoSuchPackageException | NoSuchTargetException e) { + throw new InvalidConfigurationException("Error parsing or loading objc coverage label: " + + e.getMessage(), e); + } + } return new ObjcConfiguration(buildOptions.get(ObjcCommandLineOptions.class), - buildOptions.get(BuildConfiguration.Options.class)); + options, gcovLabel); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java index b1f956c54c..166c7e27aa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java @@ -18,6 +18,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import java.util.Map; + /** * A provider of instrumented file sources and instrumentation metadata. */ @@ -32,4 +34,9 @@ public interface InstrumentedFilesProvider extends TransitiveInfoProvider { * Returns a collection of instrumentation metadata files. */ NestedSet<Artifact> getInstrumentationMetadataFiles(); + + /** + * Returns environment variables which should be set for coverage to function. + */ + Map<String, String> getExtraEnv(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java index ad5f333197..5bf2155a55 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java @@ -13,11 +13,14 @@ // limitations under the License. package com.google.devtools.build.lib.rules.test; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import java.util.Map; + /** * An implementation class for the InstrumentedFilesProvider interface. */ @@ -31,20 +34,27 @@ public final class InstrumentedFilesProviderImpl implements InstrumentedFilesPro public NestedSet<Artifact> getInstrumentationMetadataFiles() { return NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER); } + @Override + public Map<String, String> getExtraEnv() { + return ImmutableMap.of(); + } }; private final NestedSet<Artifact> instrumentedFiles; private final NestedSet<Artifact> instrumentationMetadataFiles; + private final ImmutableMap<String, String> extraEnv; public InstrumentedFilesProviderImpl(NestedSet<Artifact> instrumentedFiles, - NestedSet<Artifact> instrumentationMetadataFiles) { + NestedSet<Artifact> instrumentationMetadataFiles, Map<String, String> extraEnv) { this.instrumentedFiles = instrumentedFiles; this.instrumentationMetadataFiles = instrumentationMetadataFiles; + this.extraEnv = ImmutableMap.copyOf(extraEnv); } public InstrumentedFilesProviderImpl(InstrumentedFilesCollector collector) { this.instrumentedFiles = collector.getInstrumentedFiles(); this.instrumentationMetadataFiles = collector.getInstrumentationMetadataFiles(); + this.extraEnv = ImmutableMap.of(); } @Override @@ -56,4 +66,9 @@ public final class InstrumentedFilesProviderImpl implements InstrumentedFilesPro public NestedSet<Artifact> getInstrumentationMetadataFiles() { return instrumentationMetadataFiles; } + + @Override + public Map<String, String> getExtraEnv() { + return extraEnv; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java index aff00873a7..fca0e6d462 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java @@ -120,7 +120,7 @@ public class StandaloneTestStrategy extends TestStrategy { BuildConfiguration config = action.getConfiguration(); vars.putAll(config.getDefaultShellEnvironment()); - vars.putAll(config.getTestEnv()); + vars.putAll(action.getTestEnv()); vars.put("TEST_SRCDIR", runfilesDir.getRelative(action.getRunfilesPrefix()).getPathString()); // TODO(bazel-team): set TEST_TMPDIR. diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java index 2b3b698828..46ba583db0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.test; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; @@ -39,6 +40,7 @@ import com.google.devtools.common.options.EnumConverter; import java.util.Collection; import java.util.List; +import java.util.Map; import javax.annotation.Nullable; @@ -53,9 +55,11 @@ public final class TestActionBuilder { private ExecutionInfoProvider executionRequirements; private InstrumentedFilesProvider instrumentedFiles; private int explicitShardCount; + private Map<String, String> extraEnv; public TestActionBuilder(RuleContext ruleContext) { this.ruleContext = ruleContext; + this.extraEnv = ImmutableMap.of(); } /** @@ -110,6 +114,12 @@ public final class TestActionBuilder { return this; } + public TestActionBuilder setExtraEnv(@Nullable Map<String, String> extraEnv) { + this.extraEnv = extraEnv == null + ? ImmutableMap.<String, String> of() : ImmutableMap.copyOf(extraEnv); + return this; + } + /** * Set the explicit shard count. Note that this may be overridden by the sharding strategy. */ @@ -255,7 +265,7 @@ public final class TestActionBuilder { ruleContext.getActionOwner(), inputs, testLog, cacheStatus, coverageArtifact, microCoverageArtifact, - testProperties, executionSettings, + testProperties, extraEnv, executionSettings, shard, run, config, ruleContext.getWorkspaceName())); results.add(cacheStatus); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java index 52fa09cc27..13ec62cedc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.test; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -40,6 +41,8 @@ import com.google.devtools.common.options.TriState; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.HashMap; +import java.util.Map; import java.util.logging.Level; import javax.annotation.Nullable; @@ -85,6 +88,8 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private boolean checkedCaching = false; private boolean unconditionalExecution = false; + private ImmutableMap<String, String> testEnv; + private static ImmutableList<Artifact> list(Artifact... artifacts) { ImmutableList.Builder<Artifact> builder = ImmutableList.builder(); for (Artifact artifact : artifacts) { @@ -110,6 +115,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa Artifact coverageArtifact, Artifact microCoverageArtifact, TestTargetProperties testProperties, + Map<String, String> extraTestEnv, TestTargetExecutionSettings executionSettings, int shardNum, int runNumber, @@ -150,6 +156,10 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa this.undeclaredOutputsAnnotationsPath = undeclaredOutputsAnnotationsDir.getChild("ANNOTATIONS"); this.testInfrastructureFailure = baseDir.getChild(namePrefix + ".infrastructure_failure"); this.workspaceName = workspaceName; + + Map<String, String> mergedTestEnv = new HashMap<>(configuration.getTestEnv()); + mergedTestEnv.putAll(extraTestEnv); + this.testEnv = ImmutableMap.copyOf(mergedTestEnv); } public BuildConfiguration getConfiguration() { @@ -182,7 +192,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa f.addString(executionSettings.getTestFilter() == null ? "" : executionSettings.getTestFilter()); RunUnder runUnder = executionSettings.getRunUnder(); f.addString(runUnder == null ? "" : runUnder.getValue()); - f.addStringMap(configuration.getTestEnv()); + f.addStringMap(getTestEnv()); f.addString(testProperties.getSize().toString()); f.addString(testProperties.getTimeout().toString()); f.addStrings(testProperties.getTags()); @@ -389,6 +399,13 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa return testLog; } + /** + * Returns all environment variables which must be set in order to run this test. + */ + public Map<String, String> getTestEnv() { + return testEnv; + } + public ResolvedPaths resolve(Path execRoot) { return new ResolvedPaths(execRoot); } |