From 2c9014741a6fc2e55760be75c79a0845ce2e6d0d Mon Sep 17 00:00:00 2001 From: rosica Date: Fri, 26 Oct 2018 06:44:10 -0700 Subject: [PATCH] Remove ConfigurationEnvironment#getFragment() RELNOTES: None. PiperOrigin-RevId: 218846882 --- .../config/ConfigurationEnvironment.java | 10 ---- .../PackageProviderForConfigurations.java | 5 -- .../cpp/CrosstoolConfigurationLoader.java | 2 +- .../ConfigurationFragmentFunction.java | 13 +---- .../build/lib/skyframe/SkyframeExecutor.java | 2 +- ...ramePackageLoaderWithValueEnvironment.java | 21 +------- .../config/BuildConfigurationTest.java | 53 ------------------- .../lib/analysis/mock/BazelAnalysisMock.java | 6 --- .../build/lib/analysis/util/AnalysisMock.java | 8 --- .../lib/analysis/util/BuildViewTestCase.java | 1 - 10 files changed, 5 insertions(+), 116 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java index d7303f26867172..7947e5e7169771 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.analysis.config; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -70,10 +69,6 @@ Target getTarget(Label label) */ @Deprecated Path getPath(Package pkg, String fileName) throws InterruptedException; - - /** Returns fragment based on fragment class and build options. */ - T getFragment(BuildOptions buildOptions, Class fragmentType) - throws InvalidConfigurationException, InterruptedException; /** * An implementation backed by a {@link PackageProvider} instance. */ @@ -95,10 +90,5 @@ public Target getTarget(final Label label) public Path getPath(Package pkg, String fileName) { return pkg.getPackageDirectory().getRelative(fileName); } - - @Override - public T getFragment(BuildOptions buildOptions, Class fragmentType) { - throw new UnsupportedOperationException(); - } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java b/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java index ca3c5daad93952..9b6a84cb3822ed 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.analysis.config; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.ExtendedEventHandler; @@ -35,10 +34,6 @@ public interface PackageProviderForConfigurations { */ void addDependency(Package pkg, String fileName) throws LabelSyntaxException, IOException, InterruptedException; - - /** Returns fragment based on fragment type and build options. */ - T getFragment(BuildOptions buildOptions, Class fragmentType) - throws InvalidConfigurationException, InterruptedException; /** * Returns true if any dependency is missing (value of some node hasn't been evaluated yet). diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java index cafbdaf6b453a2..2eed4bb8106f80 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoader.java @@ -215,7 +215,7 @@ private static CrosstoolRelease findCrosstoolConfiguration( /** Reads a crosstool file. */ @Nullable - public static CrosstoolRelease readCrosstool(ConfigurationEnvironment env, Label crosstoolTop) + static CrosstoolRelease readCrosstool(ConfigurationEnvironment env, Label crosstoolTop) throws InvalidConfigurationException, InterruptedException { crosstoolTop = RedirectChaser.followRedirects(env, crosstoolTop, "crosstool_top"); if (crosstoolTop == null) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java index 9a9f449610a479..0d6e0e61a4cbd8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; -import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.ConfigurationFragmentValue.ConfigurationFragmentKey; import com.google.devtools.build.lib.vfs.Path; @@ -41,13 +40,10 @@ */ public final class ConfigurationFragmentFunction implements SkyFunction { private final Supplier> configurationFragments; - private final RuleClassProvider ruleClassProvider; public ConfigurationFragmentFunction( - Supplier> configurationFragments, - RuleClassProvider ruleClassProvider) { + Supplier> configurationFragments) { this.configurationFragments = configurationFragments; - this.ruleClassProvider = ruleClassProvider; } @Override @@ -59,7 +55,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept ConfigurationFragmentFactory factory = getFactory(configurationFragmentKey.getFragmentType()); try { PackageProviderForConfigurations packageProvider = - new SkyframePackageLoaderWithValueEnvironment(env, ruleClassProvider); + new SkyframePackageLoaderWithValueEnvironment(env); ConfigurationEnvironment confEnv = new ConfigurationBuilderEnvironment(packageProvider); Fragment fragment = factory.create(confEnv, buildOptions); @@ -119,11 +115,6 @@ public Path getPath(Package pkg, String fileName) throws InterruptedException { return result; } - @Override - public T getFragment(BuildOptions buildOptions, Class fragmentType) - throws InvalidConfigurationException, InterruptedException { - return packageProvider.getFragment(buildOptions, fragmentType); - } } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 2f9dfaa724c0a0..edbbce4d683a99 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -527,7 +527,7 @@ private ImmutableMap skyFunctions( new BuildConfigurationFunction(directories, ruleClassProvider, defaultBuildOptions)); map.put( SkyFunctions.CONFIGURATION_FRAGMENT, - new ConfigurationFragmentFunction(configurationFragments, ruleClassProvider)); + new ConfigurationFragmentFunction(configurationFragments)); map.put(SkyFunctions.WORKSPACE_NAME, new WorkspaceNameFunction()); map.put(SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(ruleClassProvider)); map.put( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java index a674abb8f5a707..abd6cd42e74f8e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java @@ -14,9 +14,6 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.actions.FileValue; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -25,7 +22,6 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; -import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.SkyframePackageLoader; import com.google.devtools.build.lib.vfs.RootedPath; @@ -41,12 +37,9 @@ */ class SkyframePackageLoaderWithValueEnvironment implements PackageProviderForConfigurations { private final SkyFunction.Environment env; - private final RuleClassProvider ruleClassProvider; - SkyframePackageLoaderWithValueEnvironment( - SkyFunction.Environment env, RuleClassProvider ruleClassProvider) { + SkyframePackageLoaderWithValueEnvironment(SkyFunction.Environment env) { this.env = env; - this.ruleClassProvider = ruleClassProvider; } @Override @@ -82,18 +75,6 @@ public void addDependency(Package pkg, String fileName) } } - @Override - public T getFragment(BuildOptions buildOptions, Class fragmentType) - throws InvalidConfigurationException, InterruptedException { - ConfigurationFragmentValue fragmentNode = (ConfigurationFragmentValue) env.getValueOrThrow( - ConfigurationFragmentValue.key(buildOptions, fragmentType, ruleClassProvider), - InvalidConfigurationException.class); - if (fragmentNode == null) { - return null; - } - return fragmentType.cast(fragmentNode.getFragment()); - } - @Override public boolean valuesMissing() { return env.valuesMissing(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java index d56890a8b24e43..56618e948141a4 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.rules.cpp.CppConfiguration; -import com.google.devtools.build.lib.rules.java.JavaConfiguration; import com.google.devtools.build.lib.rules.objc.J2ObjcConfiguration; import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester; @@ -198,58 +197,6 @@ public void testTargetEnvironment() throws Exception { assertThat(noEnvsConfig.getTargetEnvironments()).isEmpty(); } - @SafeVarargs - @SuppressWarnings("unchecked") - private final ConfigurationFragmentFactory createMockFragment( - final Class creates, final Class... dependsOn) { - return new ConfigurationFragmentFactory() { - - @Override - public Class creates() { - return creates; - } - - @Override - public ImmutableSet> requiredOptions() { - return ImmutableSet.of(); - } - - @Override - public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions) - throws InvalidConfigurationException, InterruptedException { - for (Class fragmentType : dependsOn) { - env.getFragment(buildOptions, fragmentType); - } - return new Fragment() {}; - } - }; - } - - @Test - public void testCycleInFragments() throws Exception { - configurationFragmentFactories = ImmutableList.of( - createMockFragment(CppConfiguration.class, JavaConfiguration.class), - createMockFragment(JavaConfiguration.class, CppConfiguration.class)); - try { - createCollection(); - fail(); - } catch (IllegalStateException e) { - // expected - } - } - - @Test - public void testMissingFragment() throws Exception { - configurationFragmentFactories = ImmutableList.of( - createMockFragment(CppConfiguration.class, JavaConfiguration.class)); - try { - createCollection(); - fail(); - } catch (RuntimeException e) { - // expected - } - } - @Test public void testGlobalMakeVariableOverride() throws Exception { assertThat(create().getMakeEnvironment()).containsEntry("COMPILATION_MODE", "fastbuild"); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index a1e7f3555fd6f6..9626bb618881ae 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -44,7 +44,6 @@ import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.List; public final class BazelAnalysisMock extends AnalysisMock { @@ -326,11 +325,6 @@ public ConfiguredRuleClassProvider createRuleClassProvider() { return TestRuleClassProvider.getRuleClassProvider(true); } - @Override - public Collection getOptionOverrides() { - return ImmutableList.of(); - } - @Override public boolean isThisBazel() { return true; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java index 588e526a08aa03..c159faabde1d1f 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java @@ -42,7 +42,6 @@ import com.google.devtools.common.options.InvocationPolicyEnforcer; import java.io.IOException; import java.lang.reflect.Field; -import java.util.Collection; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -109,8 +108,6 @@ public String getDefaultsPackageContent() { @Override public abstract ConfiguredRuleClassProvider createRuleClassProvider(); - public abstract Collection getOptionOverrides(); - public abstract boolean isThisBazel(); public abstract MockCcSupport ccSupport(); @@ -191,11 +188,6 @@ public MockPythonSupport pySupport() { return delegate.pySupport(); } - @Override - public Collection getOptionOverrides() { - return delegate.getOptionOverrides(); - } - @Override public ImmutableMap getSkyFunctions( BlazeDirectories directories) { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 3e37583cb1783a..91bc6fe55daf18 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -353,7 +353,6 @@ protected final BuildConfigurationCollection createConfigurations(String... args allArgs.add("--stamp"); // Stamp is now defaulted to false. allArgs.add("--experimental_extended_sanity_checks"); allArgs.add("--features=cc_include_scanning"); - allArgs.addAll(getAnalysisMock().getOptionOverrides()); optionsParser.parse(allArgs); optionsParser.parse(args);