Skip to content

Commit

Permalink
Add TargetDefinitionContext
Browse files Browse the repository at this point in the history
This adds an intermediate class between BazelStarlarkContext and Package.Builder, to prepare the way for a separate builder type for evaluating symbolic macros. A TargetDefinitionContext is anything that can consume the side-effect of defining a target, i.e. either a BUILD file's top-level logic, or a symbolic macro.

NameConflictException is migrated from Package.Builder to TargetDefinitionContext, since in principle it can happen during either BUILD file or symbolic macro evaluation.

Work toward #19922.

PiperOrigin-RevId: 602445896
Change-Id: I6ea14a118480418ae1da3c69973a508261c119f2
  • Loading branch information
brandjon authored and copybara-github committed Jan 29, 2024
1 parent 275000c commit 133dbe1
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
import com.google.devtools.build.lib.packages.MacroClass;
import com.google.devtools.build.lib.packages.MacroInstance;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.PredicateWithMessage;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
Expand All @@ -93,6 +92,7 @@
import com.google.devtools.build.lib.packages.StarlarkExportable;
import com.google.devtools.build.lib.packages.StarlarkProvider;
import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleFactory;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.BzlInitThreadContext;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import com.google.devtools.build.lib.packages.RuleFunction;
import com.google.devtools.build.lib.packages.StarlarkExportable;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import java.util.List;
Expand Down Expand Up @@ -94,7 +95,7 @@ public NoneType environmentGroup(
return Starlark.NONE;
} catch (LabelSyntaxException e) {
throw Starlark.errorf("environment group has invalid name: %s: %s", name, e.getMessage());
} catch (Package.NameConflictException e) {
} catch (NameConflictException e) {
throw Starlark.errorf("%s", e.getMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,6 @@ public class Package {
/** Sentinel value for package overhead being empty. */
private static final long PACKAGE_OVERHEAD_UNSET = -1;

/**
* An exception used when the name of a target or symbolic macro clashes with another entity
* defined in the package.
*
* <p>Common examples of conflicts include two targets or symbolic macros sharing the same name,
* and one output file being a prefix of another. See {@link #checkForExistingName} and {@link
* #checkRuleAndOutputs} for more details.
*/
public static final class NameConflictException extends Exception {
private NameConflictException(String message) {
super(message);
}
}

/**
* The collection of all targets defined in this package, indexed by name.
*
Expand Down Expand Up @@ -864,7 +850,7 @@ private static DetailedExitCode createDetailedCode(String errorMessage, Code cod
* A builder for {@link Package} objects. Only intended to be used by {@link PackageFactory} and
* {@link com.google.devtools.build.lib.skyframe.PackageFunction}.
*/
public static class Builder extends BazelStarlarkContext {
public static class Builder extends TargetDefinitionContext {

/**
* A bundle of options affecting package construction, that is not specific to any particular
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate.CannotPrecomputeDefaultsException;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Map;
Expand Down Expand Up @@ -291,7 +291,7 @@ public NoneType call(StarlarkThread thread, Tuple args, Dict<String, Object> kwa
.getSemantics()
.getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES),
thread.getCallStack());
} catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
} catch (RuleFactory.InvalidRuleException | NameConflictException e) {
throw new EvalException(e);
}
return Starlark.NONE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.io.FileSymlinkException;
import com.google.devtools.build.lib.packages.Globber.BadGlobException;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkNativeModuleApi;
Expand Down Expand Up @@ -557,7 +558,7 @@ public NoneType packageGroup(
return Starlark.NONE;
} catch (LabelSyntaxException e) {
throw Starlark.errorf("package group has invalid name: %s: %s", name, e.getMessage());
} catch (Package.NameConflictException e) {
} catch (NameConflictException e) {
throw new EvalException(e);
}
}
Expand Down Expand Up @@ -598,7 +599,7 @@ public NoneType exportsFiles(
}

pkgBuilder.setVisibilityAndLicense(inputFile, visibility, license);
} catch (Package.NameConflictException e) {
} catch (NameConflictException e) {
throw Starlark.errorf("%s", e.getMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2024 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.packages;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkThread;

/**
* A context object, usually stored in a {@link StarlarkThread}, upon which rules and symbolic
* macros can be instantiated.
*/
// TODO(#19922): Elevate some Package.Builder methods to this class.
public abstract class TargetDefinitionContext extends BazelStarlarkContext {

/**
* An exception used when the name of a target or symbolic macro clashes with another entity
* defined in the package.
*
* <p>Common examples of conflicts include two targets or symbolic macros sharing the same name,
* and one output file being a prefix of another. See {@link Package.Builder#checkForExistingName}
* and {@link Package.Builder#checkRuleAndOutputs} for more details.
*/
public static final class NameConflictException extends Exception {
public NameConflictException(String message) {
super(message);
}
}

protected TargetDefinitionContext(Phase phase, SymbolGenerator<?> symbolGenerator) {
super(phase, symbolGenerator);
}

/** Retrieves this object from a Starlark thread. Returns null if not present. */
@Nullable
public static TargetDefinitionContext fromOrNull(StarlarkThread thread) {
BazelStarlarkContext ctx = thread.getThreadLocal(BazelStarlarkContext.class);
return (ctx instanceof TargetDefinitionContext) ? (TargetDefinitionContext) ctx : null;
}

/**
* Retrieves this object from a Starlark thread. If not present, throws {@code EvalException} with
* an error message indicating that {@code what} can't be used in this Starlark environment.
*/
@CanIgnoreReturnValue
public static TargetDefinitionContext fromOrFail(StarlarkThread thread, String what)
throws EvalException {
@Nullable BazelStarlarkContext ctx = thread.getThreadLocal(BazelStarlarkContext.class);
if (!(ctx instanceof TargetDefinitionContext)) {
throw Starlark.errorf("%s can only be used while evaluating a BUILD file or macro", what);
}
return (TargetDefinitionContext) ctx;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -266,7 +266,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
thread.getCallStack());
RepositoryName.validateUserProvidedRepoName(rule.getName());
} catch (RuleFactory.InvalidRuleException
| Package.NameConflictException
| NameConflictException
| LabelSyntaxException e) {
throw new EvalException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Map;
import java.util.stream.Collectors;
Expand All @@ -50,7 +51,7 @@ public static Rule createAndAddRepositoryRule(
Map<String, Object> kwargs,
ImmutableList<StarlarkThread.CallStackEntry> callstack)
throws RuleFactory.InvalidRuleException,
Package.NameConflictException,
NameConflictException,
LabelSyntaxException,
InterruptedException {
BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
Expand Down Expand Up @@ -155,7 +156,7 @@ static void addBindRule(
Label virtual,
Label actual,
ImmutableList<StarlarkThread.CallStackEntry> callstack)
throws RuleFactory.InvalidRuleException, Package.NameConflictException, InterruptedException {
throws RuleFactory.InvalidRuleException, NameConflictException, InterruptedException {

Map<String, Object> attributes = Maps.newHashMap();
// Bound rules don't have a name field, but this works because we don't want more than one
Expand All @@ -170,8 +171,7 @@ static void addBindRule(
overwriteRule(pkg, rule);
}

private static void overwriteRule(Package.Builder pkg, Rule rule)
throws Package.NameConflictException {
private static void overwriteRule(Package.Builder pkg, Rule rule) throws NameConflictException {
Preconditions.checkArgument(rule.getOutputFiles().isEmpty());
Target old = pkg.getTarget(rule.getName());
if (old != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.starlarkbuildapi.WorkspaceGlobalsApi;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
Expand Down Expand Up @@ -142,7 +143,7 @@ public void bind(String name, Object actual, StarlarkThread thread)
(String) actual,
RepoContext.of(currentRepo, builder.getRepositoryMappingFor(currentRepo))),
thread.getCallStack());
} catch (InvalidRuleException | Package.NameConflictException | LabelSyntaxException e) {
} catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
throw Starlark.errorf("%s", e.getMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
import com.google.devtools.build.lib.packages.WorkspaceFactory;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey;
Expand Down

0 comments on commit 133dbe1

Please sign in to comment.