Skip to content

Commit

Permalink
Remove code after --incompatible_disable_sysroot_from_configuration flip
Browse files Browse the repository at this point in the history
#6565

RELNOTES: None
PiperOrigin-RevId: 222514769
  • Loading branch information
hlopko authored and Copybara-Service committed Nov 22, 2018
1 parent 68c57f0 commit d37e18e
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@
public class BazelRulesModule extends BlazeModule {
/** This is where deprecated options go to die. */
public static class GraveyardOptions extends OptionsBase {
@Option(
name = "incompatible_disable_sysroot_from_configuration",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.DEPRECATED,
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "Deprecated no-op.")
public boolean disableSysrootFromConfiguration;

@Option(
name = "incompatible_provide_cc_toolchain_info_from_cc_toolchain_suite",
defaultValue = "true",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ private static LabelLateBoundDefault<CppConfiguration> getSysrootAttribute() {
return LabelLateBoundDefault.fromTargetConfiguration(
CppConfiguration.class,
null,
(rules, attributes, cppConfig) -> cppConfig.getSysrootLabel());
(rules, attributes, cppConfig) -> cppConfig.getLibcTopLabel());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.License;
import com.google.devtools.build.lib.packages.NativeProvider;
import com.google.devtools.build.lib.syntax.Type;
Expand All @@ -61,6 +62,7 @@ public class CcToolchainAttributesProvider extends ToolchainInfo implements HasC
private final NestedSet<Artifact> ar;
private final NestedSet<Artifact> link;
private final NestedSet<Artifact> dwp;
private final Label libcTopAttribute;
private final NestedSet<Artifact> libc;
private final NestedSet<Artifact> libcMiddleman;
private final NestedSet<Artifact> fullInputsForCrosstool;
Expand Down Expand Up @@ -126,7 +128,6 @@ public CcToolchainAttributesProvider(
.addTransitive(crosstoolMiddleman)
.addTransitive(libcMiddleman)
.build();
;
this.fullInputsForLink = fullInputsForLink(ruleContext, link, libcMiddleman, isAppleToolchain);
NestedSet<Artifact> coverageAttribute =
getOptionalMiddlemanOrFiles(ruleContext, "coverage_files");
Expand All @@ -150,6 +151,7 @@ public CcToolchainAttributesProvider(
this.fdoPrefetch =
ruleContext.getPrerequisite(
":fdo_prefetch_hints", Mode.TARGET, FdoPrefetchHintsProvider.PROVIDER);
this.libcTopAttribute = ruleContext.attributes().get("libc_top", BuildType.LABEL);
this.libcTop = ruleContext.getPrerequisite(CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET);
this.moduleMap = ruleContext.getPrerequisite("module_map", Mode.HOST);
this.moduleMapArtifact = ruleContext.getPrerequisiteArtifact("module_map", Mode.HOST);
Expand Down Expand Up @@ -346,6 +348,10 @@ public NestedSet<Artifact> getLibc() {
return libc;
}

public Label getLibcTopAttribute() {
return libcTopAttribute;
}

public String getCompiler() {
return compiler;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,13 @@ private static void reportInvalidOptions(RuleContext ruleContext, CppToolchainIn
+ "dwp for the test executable, use '--fission=yes' with a toolchain that supports "
+ "Fission to build statically.");
}

if (config.getLibcTopLabel() != null && toolchain.getDefaultSysroot() == null) {
ruleContext.ruleError(
"The selected toolchain "
+ toolchain.getToolchainIdentifier()
+ " does not support setting --grte_top (it doesn't specify builtin_sysroot).");
}
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,9 @@ private static Label getLabel(AttributeMap attributes, String attrName, Label de
return cppOptionLibcTop;
}

// Should we use the version from the configuration?
Label cppConfigSysrootLabel =
cppConfig.disableSysrootFromConfiguration() ? null : cppConfig.getSysrootLabel();

// Look up the value from the attribute.
// This avoids analyzing the label from the CROSSTOOL if the attribute is set.
return getLabel(attributes, "libc_top", cppConfigSysrootLabel);
return getLabel(attributes, "libc_top", /* defaultValue= */ null);
});

private static final LabelLateBoundDefault<?> FDO_OPTIMIZE_VALUE =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,6 @@ public String toString() {
private final PathFragment fdoPath;
private final Label fdoOptimizeLabel;

private final Label sysrootLabel;

private final ImmutableList<String> conlyopts;

private final ImmutableList<String> copts;
Expand Down Expand Up @@ -237,7 +235,6 @@ static CppConfiguration create(CppConfigurationParameters params)
Preconditions.checkNotNull(params.commonOptions.cpu),
params.fdoPath,
params.fdoOptimizeLabel,
params.sysrootLabel,
ImmutableList.copyOf(cppOptions.conlyoptList),
ImmutableList.copyOf(cppOptions.coptList),
ImmutableList.copyOf(cppOptions.cxxoptList),
Expand All @@ -260,7 +257,6 @@ private CppConfiguration(
String desiredCpu,
PathFragment fdoPath,
Label fdoOptimizeLabel,
Label sysrootLabel,
ImmutableList<String> conlyopts,
ImmutableList<String> copts,
ImmutableList<String> cxxopts,
Expand All @@ -278,7 +274,6 @@ private CppConfiguration(
this.desiredCpu = desiredCpu;
this.fdoPath = fdoPath;
this.fdoOptimizeLabel = fdoOptimizeLabel;
this.sysrootLabel = sysrootLabel;
this.conlyopts = conlyopts;
this.copts = copts;
this.cxxopts = cxxopts;
Expand Down Expand Up @@ -375,10 +370,6 @@ public CompilationMode getCompilationMode() {
return compilationMode;
}

public Label getSysrootLabel() {
return sysrootLabel;
}


public boolean hasSharedLinkOption() {
return linkopts.contains("-shared");
Expand Down Expand Up @@ -716,8 +707,4 @@ boolean enableCcToolchainConfigInfoFromSkylark() {
public Label getLibcTopLabel() {
return cppOptions.libcTopLabel;
}

public boolean disableSysrootFromConfiguration() {
return cppOptions.disableSysrootFromConfiguration;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.Map;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -88,7 +87,6 @@ public static class CppConfigurationParameters {
protected final Label ccToolchainLabel;
protected final PathFragment fdoPath;
protected final Label fdoOptimizeLabel;
protected final Label sysrootLabel;
protected final CcToolchainConfigInfo ccToolchainConfigInfo;
protected final String transformedCpu;
protected final String compiler;
Expand All @@ -103,7 +101,6 @@ public static class CppConfigurationParameters {
Label fdoOptimizeLabel,
Label crosstoolTop,
Label ccToolchainLabel,
Label sysrootLabel,
CcToolchainConfigInfo ccToolchainConfigInfo) {
this.transformedCpu = transformedCpu;
this.compiler = compiler;
Expand All @@ -114,7 +111,6 @@ public static class CppConfigurationParameters {
this.fdoOptimizeLabel = fdoOptimizeLabel;
this.crosstoolTop = crosstoolTop;
this.ccToolchainLabel = ccToolchainLabel;
this.sysrootLabel = sysrootLabel;
this.ccToolchainConfigInfo = ccToolchainConfigInfo;
}
}
Expand Down Expand Up @@ -195,8 +191,6 @@ protected CppConfigurationParameters createParameters(
cToolchain, crosstoolTopLabel.getPackageIdentifier().getPathUnderExecRoot());
CcToolchainConfigInfo ccToolchainConfigInfo = CcToolchainConfigInfo.fromToolchain(cToolchain);

Label sysrootLabel = getSysrootLabel(cToolchain, cppOptions.libcTopLabel);

String ccToolchainSuiteProtoAttributeValue =
StringUtil.emptyToNull(
NonconfigurableAttributeMapper.of((Rule) crosstoolTop).get("proto", Type.STRING));
Expand Down Expand Up @@ -239,7 +233,6 @@ protected CppConfigurationParameters createParameters(
fdoProfileLabel,
crosstoolTopLabel,
ccToolchainLabel,
sysrootLabel,
ccToolchainConfigInfo);
}

Expand Down Expand Up @@ -293,33 +286,4 @@ static String getMissingCcToolchainErrorMessage(
}
return errorMessage;
}

@Nullable
public static Label getSysrootLabel(CToolchain toolchain, Label libcTopLabel)
throws InvalidConfigurationException {
PathFragment defaultSysroot =
CppConfiguration.computeDefaultSysroot(toolchain.getBuiltinSysroot());

if ((libcTopLabel != null) && (defaultSysroot == null)) {
throw new InvalidConfigurationException(
"The selected toolchain "
+ toolchain.getToolchainIdentifier()
+ " does not support setting --grte_top.");
}

if (libcTopLabel != null) {
return libcTopLabel;
}

if (!toolchain.getDefaultGrteTop().isEmpty()) {
try {
Label grteTopLabel =
new CppOptions.LibcTopLabelConverter().convert(toolchain.getDefaultGrteTop());
return grteTopLabel;
} catch (OptionsParsingException e) {
throw new InvalidConfigurationException(e.getMessage(), e);
}
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -780,22 +780,6 @@ public Label getFdoPrefetchHintsLabel() {
+ "`create_link_variables`. Use list instead.")
public boolean disableDepsetInUserFlags;

// TODO(--incompatible_disable_systool_from_configration): Deprecate the feature and remove.
@Option(
name = "incompatible_disable_sysroot_from_configuration",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If true, cc_toolchain will no longer determine the sysroot from the CROSSTOOL "
+ "(default_grte_top property_ during configuration, instead only using the "
+ "libc_top attribute.")
public boolean disableSysrootFromConfiguration;

@Override
public FragmentOptions getHost() {
CppOptions host = (CppOptions) getDefault();
Expand Down Expand Up @@ -835,8 +819,6 @@ public FragmentOptions getHost() {
host.fdoProfileLabel = null;
host.inmemoryDotdFiles = inmemoryDotdFiles;

host.disableSysrootFromConfiguration = disableSysrootFromConfiguration;

return host;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,4 +275,44 @@ public void testOptionalGcovTool() throws Exception {
ccToolchainProvider.addGlobalMakeVariables(builder);
assertThat(builder.build().get("GCOVTOOL")).isNotNull();
}

@Test
public void testUnsupportedSysrootErrorMessage() throws Exception {
scratch.file(
"a/BUILD",
"filegroup(name='empty') ",
"filegroup(name='everything')",
"cc_toolchain_suite(",
" name = 'a',",
" toolchains = { 'k8': ':b' },",
")",
"cc_toolchain(",
" name = 'b',",
" cpu = 'banana',",
" all_files = ':empty',",
" ar_files = ':empty',",
" as_files = ':empty',",
" compiler_files = ':empty',",
" dwp_files = ':empty',",
" linker_files = ':empty',",
" strip_files = ':empty',",
" objcopy_files = ':empty',",
" dynamic_runtime_libs = [':empty'],",
" static_runtime_libs = [':empty'],",
" proto = \"\"\"",
" toolchain_identifier: \"a\"",
" host_system_name: \"a\"",
" target_system_name: \"a\"",
" target_cpu: \"a\"",
" target_libc: \"a\"",
" compiler: \"a\"",
" abi_version: \"a\"",
" abi_libc_version: \"a\"",
// Not specifying `builtin_sysroot` means the toolchain doesn't support --grte_top,
"\"\"\")");
reporter.removeHandler(failFastHandler);
useConfiguration("--grte_top=//a", "--cpu=k8", "--host_cpu=k8");
getConfiguredTarget("//a:a");
assertContainsEvent("does not support setting --grte_top");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -877,44 +877,6 @@ public void testSysroot_fromCrosstool_unset() throws Exception {
assertThat(toolchainProvider.getSysroot()).isEqualTo("/usr/grte/v1");
}

@Test
public void testSysroot_fromCrosstool() throws Exception {
scratch.file("a/BUILD", "cc_toolchain_alias(name = 'b')");
scratch.file("libc1/BUILD", "filegroup(name = 'everything', srcs = ['header1.h'])");
scratch.file("libc1/header1.h", "#define FOO 1");

getAnalysisMock()
.ccSupport()
.setupCrosstool(
mockToolsConfig,
CrosstoolConfig.CToolchain.newBuilder().setDefaultGrteTop("//libc1").buildPartial());
useConfiguration("--incompatible_disable_sysroot_from_configuration=false");
ConfiguredTarget target = getConfiguredTarget("//a:b");
CcToolchainProvider toolchainProvider =
(CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);

assertThat(toolchainProvider.getSysroot()).isEqualTo("libc1");
}

@Test
public void testSysroot_fromCrosstool_disabled() throws Exception {
scratch.file("a/BUILD", "cc_toolchain_alias(name = 'b')");
scratch.file("libc1/BUILD", "filegroup(name = 'everything', srcs = ['header1.h'])");
scratch.file("libc1/header1.h", "#define FOO 1");

getAnalysisMock()
.ccSupport()
.setupCrosstool(
mockToolsConfig,
CrosstoolConfig.CToolchain.newBuilder().setDefaultGrteTop("//libc1").buildPartial());
useConfiguration("--incompatible_disable_sysroot_from_configuration");
ConfiguredTarget target = getConfiguredTarget("//a:b");
CcToolchainProvider toolchainProvider =
(CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);

assertThat(toolchainProvider.getSysroot()).isEqualTo("/usr/grte/v1");
}

@Test
public void testSysroot_fromCcToolchain() throws Exception {
scratch.file(
Expand Down Expand Up @@ -999,6 +961,7 @@ public void testSysroot_fromFlag() throws Exception {
" compiler: \"a\"",
" abi_version: \"a\"",
" abi_libc_version: \"a\"",
" builtin_sysroot: \":empty\"",
"\"\"\",",
" libc_top = '//libc2:everything')");
scratch.file("libc1/BUILD", "filegroup(name = 'everything', srcs = ['header.h'])");
Expand All @@ -1013,8 +976,7 @@ public void testSysroot_fromFlag() throws Exception {
.setupCrosstool(
mockToolsConfig,
CrosstoolConfig.CToolchain.newBuilder().setDefaultGrteTop("//libc1").buildPartial());
useConfiguration(
"--cpu=k8", "--grte_top=//libc3", "--incompatible_disable_sysroot_from_configuration");
useConfiguration("--cpu=k8", "--grte_top=//libc3");
ConfiguredTarget target = getConfiguredTarget("//a:a");
CcToolchainProvider ccToolchainProvider =
(CcToolchainProvider) target.get(CcToolchainProvider.PROVIDER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1961,7 +1961,6 @@ public void testSysrootArgSpecifiedWithGrteTopFlag() throws Exception {
useConfiguration(
"--cpu=ios_x86_64",
"--ios_cpu=x86_64",
"--incompatible_disable_sysroot_from_configuration",
"--apple_grte_top=//x");
scratch.file(
"x/BUILD",
Expand Down

0 comments on commit d37e18e

Please sign in to comment.