Skip to content

Commit

Permalink
Remove support for loose headers.
Browse files Browse the repository at this point in the history
Loose headers were disabled a long time ago so all this is dead code.

RELNOTES: None.
PiperOrigin-RevId: 575181594
Change-Id: Ibb81d5f1a0126016674228855f3df2e398c23fd3
  • Loading branch information
lberki authored and copybara-github committed Oct 20, 2023
1 parent 6b912c5 commit 7f2b802
Show file tree
Hide file tree
Showing 21 changed files with 17 additions and 360 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.google.devtools.build.lib.rules.cpp.CppActionNames;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import net.starlark.java.eval.EvalException;
Expand Down Expand Up @@ -113,12 +112,6 @@ public void finalizeCompileActionBuilder(
}
}

@Override
public HeadersCheckingMode determineStarlarkHeadersCheckingMode(
RuleContext ruleContext, CppConfiguration cppConfig, CcToolchainProvider toolchain) {
return HeadersCheckingMode.STRICT;
}

@Override
public boolean allowIncludeScanning() {
return true;
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.cpp;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;

import com.google.common.base.Joiner;
Expand All @@ -34,22 +33,19 @@
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.SourceCategory;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.CollidingProvidesException;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -429,91 +425,7 @@ private List<String> getDefinesFromAttribute(String attr) throws InterruptedExce

@StarlarkMethod(name = "loose_include_dirs", structField = true, documented = false)
public Sequence<String> getLooseIncludeDirsForStarlark() {
return StarlarkList.immutableCopyOf(
getLooseIncludeDirs().stream().map(PathFragment::toString).collect(toImmutableList()));
}

/**
* Determines a list of loose include directories that are only allowed to be referenced when
* headers checking is {@link HeadersCheckingMode#LOOSE}.
*/
Set<PathFragment> getLooseIncludeDirs() {
ImmutableSet.Builder<PathFragment> result = ImmutableSet.builder();
// The package directory of the rule contributes includes. Note that this also covers all
// non-subpackage sub-directories.
PathFragment rulePackage =
ruleContext
.getLabel()
.getPackageIdentifier()
.getExecPath(ruleContext.getConfiguration().isSiblingRepositoryLayout());
result.add(rulePackage);

if (ruleContext.getRule().isAttributeValueExplicitlySpecified("includes")) {
PathFragment packageFragment =
ruleContext
.getLabel()
.getPackageIdentifier()
.getExecPath(ruleContext.getConfiguration().isSiblingRepositoryLayout());
// For now, anything with an 'includes' needs a blanket declaration
result.add(packageFragment.getRelative("**"));
}
return result.build();
}

List<PathFragment> getSystemIncludeDirs() throws InterruptedException {
boolean siblingRepositoryLayout = ruleContext.getConfiguration().isSiblingRepositoryLayout();
List<PathFragment> result = new ArrayList<>();
PackageIdentifier packageIdentifier = ruleContext.getLabel().getPackageIdentifier();
PathFragment packageExecPath = packageIdentifier.getExecPath(siblingRepositoryLayout);
PathFragment packageSourceRoot = packageIdentifier.getPackagePath(siblingRepositoryLayout);
for (String includesAttr : ruleContext.getExpander().list("includes")) {
if (includesAttr.startsWith("/")) {
ruleContext.attributeWarning("includes",
"ignoring invalid absolute path '" + includesAttr + "'");
continue;
}
PathFragment includesPath = packageExecPath.getRelative(includesAttr);
if (!siblingRepositoryLayout && includesPath.containsUplevelReferences()) {
ruleContext.attributeError("includes",
"Path references a path above the execution root.");
}
if (includesPath.isEmpty()) {
ruleContext.attributeError(
"includes",
"'"
+ includesAttr
+ "' resolves to the workspace root, which would allow this rule and all of its "
+ "transitive dependents to include any file in your workspace. Please include only"
+ " what you need");
} else if (!includesPath.startsWith(packageExecPath)) {
ruleContext.attributeWarning(
"includes",
"'"
+ includesAttr
+ "' resolves to '"
+ includesPath
+ "' not below the relative path of its package '"
+ packageExecPath
+ "'. This will be an error in the future");
}
result.add(includesPath);
// We don't need to perform the above checks against outIncludesPath again since any errors
// must have manifested in includesPath already.
PathFragment outIncludesPath = packageSourceRoot.getRelative(includesAttr);
if (ruleContext.getConfiguration().hasSeparateGenfilesDirectory()) {
result.add(ruleContext.getGenfilesFragment().getRelative(outIncludesPath));
}
result.add(ruleContext.getBinFragment().getRelative(outIncludesPath));
}
return result;
}

/**
* Returns all additional linker inputs specified in the |additional_linker_inputs| attribute of
* the rule.
*/
List<Artifact> getAdditionalLinkerInputs() {
return ruleContext.getPrerequisiteArtifacts("additional_linker_inputs").list();
return StarlarkList.empty();
}

public String getPurpose(CppSemantics semantics) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif

private final CommandLineCcCompilationContext commandLineCcCompilationContext;

private final NestedSet<PathFragment> looseHdrsDirs;
private final NestedSet<Artifact> declaredIncludeSrcs;

/** Module maps from direct dependencies. */
Expand All @@ -87,8 +86,6 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
// Derived from depsContexts.
private final NestedSet<Artifact> compilationPrerequisites;

private final CppConfiguration.HeadersCheckingMode headersCheckingMode;

// Each pair maps the Bazel generated paths of virtual include headers back to their original path
// relative to the workspace directory.
// For example it can map
Expand All @@ -109,7 +106,6 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
private CcCompilationContext(
CommandLineCcCompilationContext commandLineCcCompilationContext,
NestedSet<Artifact> compilationPrerequisites,
NestedSet<PathFragment> looseHdrsDirs,
NestedSet<Artifact> declaredIncludeSrcs,
NestedSet<Artifact> nonCodeInputs,
HeaderInfo headerInfo,
Expand All @@ -119,12 +115,10 @@ private CcCompilationContext(
ImmutableList<CppModuleMap> exportingModuleMaps,
CppModuleMap cppModuleMap,
boolean propagateModuleMapAsActionInput,
CppConfiguration.HeadersCheckingMode headersCheckingMode,
NestedSet<Tuple> virtualToOriginalHeaders,
NestedSet<Artifact> headerTokens) {
Preconditions.checkNotNull(commandLineCcCompilationContext);
this.commandLineCcCompilationContext = commandLineCcCompilationContext;
this.looseHdrsDirs = looseHdrsDirs;
this.declaredIncludeSrcs = declaredIncludeSrcs;
this.directModuleMaps = directModuleMaps;
this.exportingModuleMaps = exportingModuleMaps;
Expand All @@ -135,7 +129,6 @@ private CcCompilationContext(
this.nonCodeInputs = nonCodeInputs;
this.compilationPrerequisites = compilationPrerequisites;
this.propagateModuleMapAsActionInput = propagateModuleMapAsActionInput;
this.headersCheckingMode = headersCheckingMode;
this.virtualToOriginalHeaders = virtualToOriginalHeaders;
this.transitiveHeaderCount = -1;
this.headerTokens = headerTokens;
Expand Down Expand Up @@ -349,14 +342,6 @@ public ImmutableList<PathFragment> getExternalIncludeDirs() {
return commandLineCcCompilationContext.externalIncludeDirs;
}

/**
* Returns the immutable set of declared include directories, relative to a "-I" or "-iquote"
* directory" (possibly empty but never null).
*/
public NestedSet<PathFragment> getLooseHdrsDirs() {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}

/**
* Returns the immutable set of headers that have been declared in the {@code srcs} or {@code
* hdrs} attribute (possibly empty but never null).
Expand Down Expand Up @@ -653,7 +638,6 @@ public static CcCompilationContext createWithExtraHeaderTokens(
return new CcCompilationContext(
ccCompilationContext.commandLineCcCompilationContext,
ccCompilationContext.compilationPrerequisites,
ccCompilationContext.looseHdrsDirs,
ccCompilationContext.declaredIncludeSrcs,
ccCompilationContext.nonCodeInputs,
ccCompilationContext.headerInfo,
Expand All @@ -663,7 +647,6 @@ public static CcCompilationContext createWithExtraHeaderTokens(
ccCompilationContext.exportingModuleMaps,
ccCompilationContext.cppModuleMap,
ccCompilationContext.propagateModuleMapAsActionInput,
ccCompilationContext.headersCheckingMode,
ccCompilationContext.virtualToOriginalHeaders,
headerTokens.build());
}
Expand Down Expand Up @@ -731,7 +714,6 @@ public static class Builder {
new TransitiveSetHelper<>();
private final TransitiveSetHelper<PathFragment> externalIncludeDirs =
new TransitiveSetHelper<>();
private final NestedSetBuilder<PathFragment> looseHdrsDirs = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Artifact> declaredIncludeSrcs = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Artifact> nonCodeInputs = NestedSetBuilder.stableOrder();
private final HeaderInfo.Builder headerInfoBuilder = new HeaderInfo.Builder();
Expand All @@ -742,8 +724,6 @@ public static class Builder {
private final Set<String> localDefines = new LinkedHashSet<>();
private CppModuleMap cppModuleMap;
private boolean propagateModuleMapAsActionInput = true;
private CppConfiguration.HeadersCheckingMode headersCheckingMode =
CppConfiguration.HeadersCheckingMode.STRICT;
private final NestedSetBuilder<Tuple> virtualToOriginalHeaders = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Artifact> headerTokens = NestedSetBuilder.stableOrder();

Expand Down Expand Up @@ -793,7 +773,6 @@ private void mergeDependentCcCompilationContext(
systemIncludeDirs.addTransitive(otherCcCompilationContext.getSystemIncludeDirs());
frameworkIncludeDirs.addTransitive(otherCcCompilationContext.getFrameworkIncludeDirs());
externalIncludeDirs.addTransitive(otherCcCompilationContext.getExternalIncludeDirs());
looseHdrsDirs.addTransitive(otherCcCompilationContext.getLooseHdrsDirs());
declaredIncludeSrcs.addTransitive(otherCcCompilationContext.getDeclaredIncludeSrcs());
headerInfoBuilder.addDep(otherCcCompilationContext.headerInfo);

Expand Down Expand Up @@ -968,13 +947,6 @@ public Builder addExternalIncludeDirs(Iterable<PathFragment> externalIncludeDirs
return this;
}

/** Add a single declared include dir, relative to a "-I" or "-iquote" directory". */
@CanIgnoreReturnValue
public Builder addLooseHdrsDir(PathFragment dir) {
looseHdrsDirs.add(dir);
return this;
}

/**
* Adds a header that has been declared in the {@code src} or {@code headers attribute}. The
* header will also be added to the compilation prerequisites.
Expand Down Expand Up @@ -1095,13 +1067,6 @@ Builder setPicHeaderModule(DerivedArtifact picHeaderModule) {
return this;
}

@CanIgnoreReturnValue
public Builder setHeadersCheckingMode(
CppConfiguration.HeadersCheckingMode headersCheckingMode) {
this.headersCheckingMode = headersCheckingMode;
return this;
}

@CanIgnoreReturnValue
public Builder addVirtualToOriginalHeaders(NestedSet<Tuple> virtualToOriginalHeaders) {
this.virtualToOriginalHeaders.addTransitive(virtualToOriginalHeaders);
Expand Down Expand Up @@ -1139,7 +1104,6 @@ public CcCompilationContext build() {
allDefines.getMergedResult(),
ImmutableList.copyOf(localDefines)),
constructedPrereq,
looseHdrsDirs.build(),
declaredIncludeSrcs.build(),
nonCodeInputs.build(),
headerInfo,
Expand All @@ -1149,7 +1113,6 @@ public CcCompilationContext build() {
ImmutableList.copyOf(exportingModuleMaps),
cppModuleMap,
propagateModuleMapAsActionInput,
headersCheckingMode,
virtualToOriginalHeaders.build(),
headerTokens.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import com.google.devtools.build.lib.rules.cpp.CcCommon.Language;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.VariablesExtension;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CompilationInfoApi;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Pair;
Expand Down Expand Up @@ -276,14 +275,11 @@ public CcCompilationContext getCcCompilationContext() {
private final Set<String> localDefines = new LinkedHashSet<>();
private final List<CcCompilationContext> deps = new ArrayList<>();
private final List<CcCompilationContext> implementationDeps = new ArrayList<>();
private Set<PathFragment> looseIncludeDirs = ImmutableSet.of();
private final List<PathFragment> systemIncludeDirs = new ArrayList<>();
private final List<PathFragment> quoteIncludeDirs = new ArrayList<>();
private final List<PathFragment> includeDirs = new ArrayList<>();
private final List<PathFragment> frameworkIncludeDirs = new ArrayList<>();

private HeadersCheckingMode headersCheckingMode = HeadersCheckingMode.STRICT;

private final SourceCategory sourceCategory;
private final List<VariablesExtension> variablesExtensions = new ArrayList<>();
private CcToolchainVariables prebuiltParent;
Expand Down Expand Up @@ -638,14 +634,6 @@ public CcCompilationHelper addImplementationDepsCcCompilationContexts(
return this;
}

/**
* Sets the given directories to by loose include directories that are only allowed to be
* referenced when headers checking is {@link HeadersCheckingMode#LOOSE}.
*/
public void setLooseIncludeDirs(Set<PathFragment> looseIncludeDirs) {
this.looseIncludeDirs = Preconditions.checkNotNull(looseIncludeDirs);
}

/**
* Adds the given directories to the system include directories (they are passed with {@code
* "-isystem"} to the compiler); these are also passed to dependent rules.
Expand Down Expand Up @@ -709,13 +697,6 @@ public CcCompilationHelper setPropagateModuleMapToCompileAction(boolean propagat
return this;
}

/** Sets the given headers checking mode. The default is {@link HeadersCheckingMode#LOOSE}. */
@CanIgnoreReturnValue
public CcCompilationHelper setHeadersCheckingMode(HeadersCheckingMode headersCheckingMode) {
this.headersCheckingMode = Preconditions.checkNotNull(headersCheckingMode);
return this;
}

/** Whether to generate no-PIC actions. */
@CanIgnoreReturnValue
public CcCompilationHelper setGenerateNoPicAction(boolean generateNoPicAction) {
Expand Down Expand Up @@ -814,8 +795,6 @@ private Tuple callStarlarkInit(RuleContext ruleContext)
/* public_textual_headers */ StarlarkList.immutableCopyOf(publicTextualHeaders),
/* private_headers_artifacts */ StarlarkList.immutableCopyOf(privateHeaders),
/* additional_inputs */ StarlarkList.immutableCopyOf(additionalInputs),
/* headers_checking_mode */ headersCheckingMode.name(),
/* loose_include_dirs */ convertPathFragmentsToStarlarkList(looseIncludeDirs),
/* separate_module_headers */ StarlarkList.immutableCopyOf(separateModuleHeaders),
/* generate_module_map */ generateModuleMap,
/* generate_pic_action */ generatePicAction,
Expand Down
Loading

0 comments on commit 7f2b802

Please sign in to comment.