Skip to content

Commit

Permalink
Extend the repository_rule function by a parameter configure
Browse files Browse the repository at this point in the history
    Setting this parameter to True (the default value is False) indicates
    to bazel that the repository serves a configure-like purpose, e.g.,
    inspecting the host environment searching for compilers or standard
    libraries.

    Change-Id: I57c97f2ae81b176bb40a06d60d65929585665a68
    PiperOrigin-RevId: 257588478
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent d7fd858 commit dc82c82
Show file tree
Hide file tree
Showing 14 changed files with 94 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.skylark.BazelStarlarkContext;
import com.google.devtools.build.lib.analysis.skylark.SymbolGenerator;
import com.google.devtools.build.lib.bazel.repository.RepositoryResolvedEvent;
import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.SymbolGenerator;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
Expand Down Expand Up @@ -133,8 +133,7 @@ public RepositoryDirectoryValue.Builder fetch(
/* toolsRepository = */ null,
/* fragmentNameToClass = */ null,
rule.getPackage().getRepositoryMapping(),
new SymbolGenerator<>(key),
/* analysisRuleLabel= */ null))
new SymbolGenerator<>(key)))
.build();
SkylarkRepositoryContext skylarkRepositoryContext =
new SkylarkRepositoryContext(
Expand Down Expand Up @@ -250,13 +249,7 @@ protected boolean isConfigure(Rule rule) {
return (Boolean) rule.getAttributeContainer().getAttr("$configure");
}

/**
* Static method to determine if for a starlark repository rule {@code isConfigure} holds true. It
* also checks that the rule is indeed a Starlark rule so that this class is the appropriate
* handler for the given rule. As, however, only Starklark rules can be configure rules, this
* method can also be used as a universal check.
*/
public static boolean isConfigureRule(Rule rule) {
public static boolean isConfigurelikeRule(Rule rule) {
return rule.getRuleClassObject().isSkylark()
&& ((Boolean) rule.getAttributeContainer().getAttr("$configure"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
package com.google.devtools.build.lib.bazel.repository.skylark;

import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
import static com.google.devtools.build.lib.packages.Type.STRING;
import static com.google.devtools.build.lib.packages.Type.STRING_LIST;
import static com.google.devtools.build.lib.syntax.SkylarkType.castMap;
import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import static com.google.devtools.build.lib.syntax.Type.STRING;
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;

import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.skylark.SkylarkAttr.Descriptor;
Expand Down Expand Up @@ -90,7 +90,7 @@ public BaseFunction repositoryRule(
}
builder.setConfiguredTargetFunction(implementation);
builder.setRuleDefinitionEnvironmentLabelAndHashCode(
(Label) funcallEnv.getGlobals().getLabel(), funcallEnv.getTransitiveContentHashCode());
funcallEnv.getGlobals().getLabel(), funcallEnv.getTransitiveContentHashCode());
builder.setWorkspaceOnly();
return new RepositoryRuleFunction(builder, ast.getLocation());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@
import com.google.devtools.build.lib.rules.apple.ApplePlatform;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkingContext.LinkOptions;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext.LinkOptions;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
Expand Down Expand Up @@ -267,7 +268,7 @@ public ConfiguredTarget create(RuleContext context)

public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake)
throws InterruptedException, RuleErrorException, ActionConflictException {
CcCommon.checkRuleLoadedThroughMacro(ruleContext);

semantics.validateDeps(ruleContext);
if (ruleContext.hasErrors()) {
return null;
Expand Down Expand Up @@ -1001,10 +1002,7 @@ private static NestedSet<Artifact> createDynamicLibrariesCopyActions(
RuleContext ruleContext, Iterable<Artifact> dynamicLibrariesForRuntime) {
NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
for (Artifact target : dynamicLibrariesForRuntime) {
if (!ruleContext
.getLabel()
.getPackageIdentifier()
.equals(target.getOwner().getPackageIdentifier())) {
if (!ruleContext.getLabel().getPackageName().equals(target.getOwner().getPackageName())) {
// SymlinkAction on file is actually copy on Windows.
Artifact copy = ruleContext.getBinArtifact(target.getFilename());
ruleContext.registerAction(SymlinkAction.toArtifact(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1061,30 +1061,4 @@ public static Map<String, NestedSet<Artifact>> createSaveFeatureStateArtifacts(
}
return outputGroupsBuilder.build();
}

public static void checkRuleLoadedThroughMacro(RuleContext ruleContext) {
if (!ruleContext.getFragment(CppConfiguration.class).loadCcRulesFromBzl()) {
return;
}

if (!hasValidTag(ruleContext) || !ruleContext.getRule().wasCreatedByMacro()) {
registerMigrationRuleError(ruleContext);
}
}

private static boolean hasValidTag(RuleContext ruleContext) {
return ruleContext
.attributes()
.get("tags", Type.STRING_LIST)
.contains("__CC_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__");
}

private static void registerMigrationRuleError(RuleContext ruleContext) {
ruleContext.ruleError(
"The native C++/Objc rules are deprecated. Please load "
+ ruleContext.getRule().getRuleClass()
+ " from the rules_cc repository. See http://github.com/bazelbuild/rules_cc and "
+ "https://github.com/bazelbuild/bazel/issues/7643. You can temporarily bypass this "
+ "error by setting --incompatible_load_cc_rules_from_bzl=false.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,6 @@ public SkylarkNestedSet getSkylarkDefines() {
String.class, NestedSetBuilder.wrap(Order.STABLE_ORDER, getDefines()));
}

@Override
public SkylarkNestedSet getSkylarkNonTransitiveDefines() {
return SkylarkNestedSet.of(
String.class, NestedSetBuilder.wrap(Order.STABLE_ORDER, getNonTransitiveDefines()));
}

@Override
public SkylarkNestedSet getSkylarkHeaders() {
return SkylarkNestedSet.of(Artifact.class, getDeclaredIncludeSrcs());
Expand Down Expand Up @@ -406,21 +400,14 @@ protected Set<Artifact> getHeaderModuleSrcs() {
}

/**
* Returns the set of defines needed to compile this target. This includes definitions from the
* transitive deps closure for the target. The order of the returned collection is deterministic.
* Returns the set of defines needed to compile this target (possibly empty
* but never null). This includes definitions from the transitive deps closure
* for the target. The order of the returned collection is deterministic.
*/
public ImmutableList<String> getDefines() {
return commandLineCcCompilationContext.defines;
}

/**
* Returns the set of defines needed to compile this target. This doesn't include definitions from
* the transitive deps closure for the target.
*/
ImmutableList<String> getNonTransitiveDefines() {
return commandLineCcCompilationContext.localDefines;
}

/**
* Returns a {@code CcCompilationContext} that is based on a given {@code CcCompilationContext}
* but returns empty sets for {@link #getDeclaredIncludeDirs()}.
Expand Down Expand Up @@ -494,21 +481,18 @@ static class CommandLineCcCompilationContext {
private final ImmutableList<PathFragment> systemIncludeDirs;
private final ImmutableList<PathFragment> frameworkIncludeDirs;
private final ImmutableList<String> defines;
private final ImmutableList<String> localDefines;

CommandLineCcCompilationContext(
ImmutableList<PathFragment> includeDirs,
ImmutableList<PathFragment> quoteIncludeDirs,
ImmutableList<PathFragment> systemIncludeDirs,
ImmutableList<PathFragment> frameworkIncludeDirs,
ImmutableList<String> defines,
ImmutableList<String> localDefines) {
ImmutableList<String> defines) {
this.includeDirs = includeDirs;
this.quoteIncludeDirs = quoteIncludeDirs;
this.systemIncludeDirs = systemIncludeDirs;
this.frameworkIncludeDirs = frameworkIncludeDirs;
this.defines = defines;
this.localDefines = localDefines;
}
}

Expand Down Expand Up @@ -540,7 +524,6 @@ public static class Builder {
private final NestedSetBuilder<Artifact> transitivePicModules = NestedSetBuilder.stableOrder();
private final Set<Artifact> directModuleMaps = new LinkedHashSet<>();
private final Set<String> defines = new LinkedHashSet<>();
private final Set<String> localDefines = new LinkedHashSet<>();
private CppModuleMap cppModuleMap;
private CppModuleMap verificationModuleMap;
private boolean propagateModuleMapAsActionInput = true;
Expand Down Expand Up @@ -750,12 +733,6 @@ public Builder addDefines(Iterable<String> defines) {
return this;
}

/** Adds multiple non-transitive defines. */
public Builder addNonTransitiveDefines(Iterable<String> defines) {
Iterables.addAll(this.localDefines, defines);
return this;
}

/** Sets the C++ module map. */
public Builder setCppModuleMap(CppModuleMap cppModuleMap) {
this.cppModuleMap = cppModuleMap;
Expand Down Expand Up @@ -829,8 +806,7 @@ public CcCompilationContext build(ActionOwner owner, MiddlemanFactory middlemanF
ImmutableList.copyOf(quoteIncludeDirs),
ImmutableList.copyOf(systemIncludeDirs),
ImmutableList.copyOf(frameworkIncludeDirs),
ImmutableList.copyOf(defines),
ImmutableList.copyOf(localDefines)),
ImmutableList.copyOf(defines)),
// TODO(b/110873917): We don't have the middle man compilation prerequisite, therefore, we
// use the compilation prerequisites as they were passed to the builder, i.e. we use every
// header instead of a middle man.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,6 @@ private CcCompilationContext initializeCcCompilationContext() {

ccCompilationContextBuilder.addSystemIncludeDirs(systemIncludeDirs);
ccCompilationContextBuilder.addFrameworkIncludeDirs(frameworkIncludeDirs);
ccCompilationContextBuilder.addQuoteIncludeDirs(quoteIncludeDirs);

for (PathFragment includeDir : includeDirs) {
ccCompilationContextBuilder.addIncludeDir(includeDir);
Expand Down Expand Up @@ -996,6 +995,7 @@ private CcCompilationContext initializeCcCompilationContext() {
}
}
ccCompilationContextBuilder.setPurpose(purpose);
ccCompilationContextBuilder.addQuoteIncludeDirs(quoteIncludeDirs);
return ccCompilationContextBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,20 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.Depset;
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 com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcDebugInfoContextApi;
import java.util.Collection;
import java.util.Objects;

/**
* A struct that stores .dwo files which can be combined into a .dwp in the packaging step. See
* https://gcc.gnu.org/wiki/DebugFission for details.
*/
@Immutable
public final class CcDebugInfoContext implements CcDebugInfoContextApi {
public final class CcDebugInfoContext {

public static final CcDebugInfoContext EMPTY =
new CcDebugInfoContext(
Expand All @@ -44,7 +42,7 @@ public CcDebugInfoContext(
}

/** Merge multiple {@link CcDebugInfoContext}s into one. */
public static CcDebugInfoContext merge(Collection<CcDebugInfoContext> contexts) {
public static CcDebugInfoContext merge(ImmutableList<CcDebugInfoContext> contexts) {
NestedSetBuilder<Artifact> transitiveDwoFiles = NestedSetBuilder.stableOrder();
NestedSetBuilder<Artifact> transitivePicDwoFiles = NestedSetBuilder.stableOrder();

Expand Down Expand Up @@ -80,16 +78,6 @@ public NestedSet<Artifact> getTransitivePicDwoFiles() {
return transitivePicDwoFiles;
}

@Override
public Depset getStarlarkTransitiveFiles() {
return Depset.of(Artifact.TYPE, getTransitiveDwoFiles());
}

@Override
public Depset getStarlarkTransitivePicFiles() {
return Depset.of(Artifact.TYPE, getTransitivePicDwoFiles());
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
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.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.syntax.Type;
import java.util.Map;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -70,7 +71,6 @@ private static NoPicAndPicStaticLibrary create(@Nullable Artifact staticLibrary)
@Override
public ConfiguredTarget create(RuleContext ruleContext)
throws InterruptedException, RuleErrorException, ActionConflictException {
CcCommon.checkRuleLoadedThroughMacro(ruleContext);

boolean systemProvided = ruleContext.attributes().get("system_provided", Type.BOOLEAN);
CcToolchainProvider ccToolchain =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.skylarkbuildapi.cpp.CcInfoApi;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Type;
Expand All @@ -69,9 +70,6 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
/** A string constant for the name of dynamic library output group. */
public static final String DYNAMIC_LIBRARY_OUTPUT_GROUP_NAME = "dynamic_library";

/** A string constant for the name of Windows def file output group. */
public static final String DEF_FILE_OUTPUT_GROUP_NAME = "def_file";

private final CppSemantics semantics;

protected CcLibrary(CppSemantics semantics) {
Expand Down Expand Up @@ -113,7 +111,7 @@ public static void init(
boolean linkStatic,
boolean addDynamicRuntimeInputArtifactsToRunfiles)
throws RuleErrorException, InterruptedException {
CcCommon.checkRuleLoadedThroughMacro(ruleContext);

semantics.validateDeps(ruleContext);
if (ruleContext.hasErrors()) {
return;
Expand Down Expand Up @@ -309,31 +307,28 @@ public static void init(
// If user specifies a custom DEF file, then we use it.
Artifact defFile = common.getWinDefFile();

Artifact defParser = common.getDefParser();
Artifact generatedDefFile = null;
if (defParser != null) {
try {
generatedDefFile =
CppHelper.createDefFileActions(
ruleContext,
defParser,
ccCompilationOutputs.getObjectFiles(false),
ccToolchain
.getFeatures()
.getArtifactNameForCategory(
ArtifactCategory.DYNAMIC_LIBRARY, ruleContext.getLabel().getName()));
targetBuilder.addOutputGroup(DEF_FILE_OUTPUT_GROUP_NAME, generatedDefFile);
} catch (EvalException e) {
throw ruleContext.throwWithRuleError(e.getMessage());
}
}

// If no DEF file is specified and the windows_export_all_symbols feature is enabled, parse
// object files to generate DEF file and use it to export symbols - if we have a parser.
// Otherwise, use no DEF file.
if (defFile == null
&& CppHelper.shouldUseGeneratedDefFile(ruleContext, featureConfiguration)) {
defFile = generatedDefFile;
Artifact defParser = common.getDefParser();
if (defParser != null) {
try {
defFile =
CppHelper.createDefFileActions(
ruleContext,
defParser,
ccCompilationOutputs.getObjectFiles(false),
ccToolchain
.getFeatures()
.getArtifactNameForCategory(
ArtifactCategory.DYNAMIC_LIBRARY, ruleContext.getLabel().getName()));
} catch (EvalException e) {
ruleContext.throwWithRuleError(e.getMessage());
throw new IllegalStateException("Should not be reached");
}
}
}

if (defFile != null) {
Expand Down
Loading

0 comments on commit dc82c82

Please sign in to comment.