Skip to content

Commit

Permalink
Introduce option flag experimental_enable_tools_defaults_package.
Browse files Browse the repository at this point in the history
Default value is true, and behavior related to //tools/defaults package is not
changed. If set it to false, then in-memory Dfaultpacked will not be created.

RELNOTES:none
PiperOrigin-RevId: 205643628
  • Loading branch information
dbabkin authored and Copybara-Service committed Jul 23, 2018
1 parent dc1148e commit 258efc4
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.List;
Expand Down Expand Up @@ -161,6 +162,17 @@ public String getTypeDescription() {
)
public boolean checkOutputFiles;

@Option(
name = "experimental_enable_tools_defaults_package",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
"If true, Blaze constructs an in-memory //tools/defaults package based on the command"
+ " line options. If false, //tools/defaults is resolved as a regular package.")
public boolean experimentalInMemoryToolsDefaultsPackage;

/**
* A converter from strings containing comma-separated names of packages to lists of strings.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,14 +438,14 @@ public SkyValue compute(SkyKey key, Environment env) throws PackageFunctionExcep
Path buildFilePath = buildFileRootedPath.asPath();
String replacementContents = null;

if (!isDefaultsPackage(packageId)) {
buildFileValue = getBuildFileValue(env, buildFileRootedPath);
if (buildFileValue == null) {
if (isDefaultsPackage(packageId) && PrecomputedValue.isInMemoryToolsDefaults(env)) {
replacementContents = PrecomputedValue.DEFAULTS_PACKAGE_CONTENTS.get(env);
if (replacementContents == null) {
return null;
}
} else {
replacementContents = PrecomputedValue.DEFAULTS_PACKAGE_CONTENTS.get(env);
if (replacementContents == null) {
buildFileValue = getBuildFileValue(env, buildFileRootedPath);
if (buildFileValue == null) {
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
PathPackageLocator pkgLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env);

PackageIdentifier packageKey = (PackageIdentifier) skyKey.argument();
if (PackageFunction.isDefaultsPackage(packageKey)) {

if (PackageFunction.isDefaultsPackage(packageKey)
&& PrecomputedValue.isInMemoryToolsDefaults(env)) {
return PackageLookupValue.success(pkgLocator.getPathEntries().get(0), BuildFileName.BUILD);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ public static <T> Injected injected(Precomputed<T> precomputed, T value) {
return new Injected(precomputed, Suppliers.ofInstance(value));
}

public static final Precomputed<Boolean> ENABLE_DEFAULTS_PACKAGE =
new Precomputed<>(Key.create("enable_default_pkg"));

// TODO(dbabkin): better to move this code to PrecomputedValueUtils.
// It will gone soon after removing tools/defaults
public static boolean isInMemoryToolsDefaults(SkyFunction.Environment env)
throws InterruptedException {
Boolean enableDefaultsPackage = PrecomputedValue.ENABLE_DEFAULTS_PACKAGE.get(env);
return Preconditions.checkNotNull(enableDefaultsPackage);
}

public static final Precomputed<String> DEFAULTS_PACKAGE_CONTENTS =
new Precomputed<>(Key.create("default_pkg"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,14 @@ public void preparePackageLoading(
setShowLoadingProgress(packageCacheOptions.showLoadingProgress);
setDefaultVisibility(packageCacheOptions.defaultVisibility);
setSkylarkSemantics(skylarkSemanticsOptions.toSkylarkSemantics());
setupDefaultPackage(defaultsPackageContents);
if (packageCacheOptions.experimentalInMemoryToolsDefaultsPackage) {
setupDefaultPackage(defaultsPackageContents);
PrecomputedValue.ENABLE_DEFAULTS_PACKAGE.set(injectable(), true);
} else {
setupDefaultPackage("# //tools/defaults in-memory package is not enabled.");
PrecomputedValue.ENABLE_DEFAULTS_PACKAGE.set(injectable(), false);
}

setPackageLocator(pkgLocator);

syscalls.set(getPerBuildSyscallCache(packageCacheOptions.globbingThreads));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ public void inject(SkyKey key, SkyValue value) {
PrecomputedValue.DEFAULT_VISIBILITY.set(injectable, ConstantRuleVisibility.PRIVATE);
PrecomputedValue.SKYLARK_SEMANTICS.set(injectable, skylarkSemantics);
PrecomputedValue.DEFAULTS_PACKAGE_CONTENTS.set(injectable, defaultsPackageContents);
PrecomputedValue.ENABLE_DEFAULTS_PACKAGE.set(injectable, true);
return new ImmutableDiff(ImmutableList.of(), valuesToInject);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2018 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.analysis;

import static com.google.common.truth.Truth.assertThat;

import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/**
* Test for experimental_enable_tools_defaults_package flag. TODO(dbabkin): remove after
* //tools/defaults package gone.
*/
@RunWith(JUnit4.class)
public class EnableDefaultsPackageOptionTest extends BuildViewTestCase {

@Test
public void testEnableDefaultsPackageOptionWorks() throws Exception {
// do not need that as value is true by default.
// setPackageCacheOptions("--experimental_enable_tools_defaults_package=true");

ConfiguredTarget target = getConfiguredTarget("//tools/defaults:jdk");

assertThat(target.getLabel().toString()).isEqualTo("//tools/defaults:jdk");
}

@Test
public void testDisabledDefaultsPackageOptionWorks() throws Exception {

scratch.file(
"a/BUILD",
"filegroup(",
" name = 'my_filegroup',",
" srcs = ['//tools/defaults:jdk'],",
")");

reporter.removeHandler(failFastHandler);
setPackageCacheOptions("--experimental_enable_tools_defaults_package=false");
ConfiguredTarget target = getConfiguredTarget("//a:my_filegroup");

assertThat(target).isNull();
assertContainsEvent(
"no such package 'tools/defaults': "
+ "BUILD file not found on package path and referenced by '//a:my_filegroup'");
}

@Test
public void testFlipFlagOnFly() throws Exception {

setPackageCacheOptions("--experimental_enable_tools_defaults_package=false");
ConfiguredTarget defaultsJDKtarget = getConfiguredTarget("//tools/defaults:jdk");
assertThat(defaultsJDKtarget).isNull();

setPackageCacheOptions("--experimental_enable_tools_defaults_package=true");
defaultsJDKtarget = getConfiguredTarget("//tools/defaults:jdk");
assertThat(defaultsJDKtarget).isNotNull();
assertThat(defaultsJDKtarget.getLabel().toString()).isEqualTo("//tools/defaults:jdk");

setPackageCacheOptions("--experimental_enable_tools_defaults_package=false");
defaultsJDKtarget = getConfiguredTarget("//tools/defaults:jdk");
assertThat(defaultsJDKtarget).isNull();
}
}

0 comments on commit 258efc4

Please sign in to comment.