Skip to content

Commit

Permalink
Serialize Package#macroNamespaceViolatingTargets
Browse files Browse the repository at this point in the history
To ensure that after serialization/deserialization, targets that violate
macro namespace are still quarantined.

Working towards bazelbuild#19922

PiperOrigin-RevId: 668160971
Change-Id: Iec1cbce23958b1c57be98497ecc3f0d9e21a89de
  • Loading branch information
tetromino authored and copybara-github committed Aug 27, 2024
1 parent 2d12615 commit e811565
Showing 1 changed file with 33 additions and 9 deletions.
42 changes: 33 additions & 9 deletions src/main/java/com/google/devtools/build/lib/packages/Package.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,15 @@ public class Package {
*
* <p>Initialized by the builder in {@link #finishInit}.
*/
// TODO(#19922): ensure this map is serialized/deserialized; the builder might not initialize it
// in {@link #finishInit} when name conflict checking is disabled.
@Nullable private ImmutableMap<String, String> macroNamespaceViolatingTargets;

/**
* A map from names of targets declared in a symbolic macro to the (innermost) macro instance
* where they were declared.
*/
// TODO: #19922 - This likely subsumes macroNamespaceViolatingTargets, since we can just map the
// target to its macro and then check whether it is in the macro's namespace.
// TODO: #19922 - If this field were made serializable (currently it's not), it would subsume
// macroNamespaceViolatingTargets, since we can just map the target to its macro and then check
// whether it is in the macro's namespace.
//
// TODO: #19922 - Don't maintain this extra map of all macro-instantiated targets. We have a
// couple options:
Expand Down Expand Up @@ -634,16 +633,26 @@ public Rule getRule(String targetName) {
}

/**
* Throws {@link MacroNamespaceViolationException} if the given target (which must be a member of
* this package) violates macro naming rules.
* Returns a map from names of targets declared in a symbolic macro which violate macro naming
* rules, such as "lib%{name}-src.jar" implicit outputs in java rules, to the name of the macro
* instance where they were declared.
*/
public void checkMacroNamespaceCompliance(Target target) throws MacroNamespaceViolationException {
ImmutableMap<String, String> getMacroNamespaceViolatingTargets() {
Preconditions.checkNotNull(
macroNamespaceViolatingTargets,
"This method is only available after the package has been loaded.");
return macroNamespaceViolatingTargets;
}

/**
* Throws {@link MacroNamespaceViolationException} if the given target (which must be a member of
* this package) violates macro naming rules.
*/
public void checkMacroNamespaceCompliance(Target target) throws MacroNamespaceViolationException {
Preconditions.checkArgument(
this.equals(target.getPackage()), "Target must belong to this package");
@Nullable String macroNamespaceViolated = macroNamespaceViolatingTargets.get(target.getName());
@Nullable
String macroNamespaceViolated = getMacroNamespaceViolatingTargets().get(target.getName());
if (macroNamespaceViolated != null) {
throw new MacroNamespaceViolationException(
String.format(
Expand Down Expand Up @@ -1144,7 +1153,8 @@ private enum NameConflictCheckingPolicy {
* <p>This field is null if name conflict checking is disabled. The content of the map is
* manipulated only in {@link #checkRuleAndOutputs}.
*/
@Nullable private Map<String, String> macroNamespaceViolatingTargets = new HashMap<>();
@Nullable
private LinkedHashMap<String, String> macroNamespaceViolatingTargets = new LinkedHashMap<>();

/**
* A map from target name to the (innermost) macro instance that declared it. See {@link
Expand Down Expand Up @@ -2535,6 +2545,20 @@ private void checkTargetName(Target target) throws NameConflictException {
}
}

/**
* Add all given map entries to the builder's map from names of targets declared in a symbolic
* macro which violate macro naming rules to the name of the macro instance where they were
* declared.
*
* <p>Intended to be used for package deserialization.
*/
void putAllMacroNamespaceViolatingTargets(Map<String, String> macroNamespaceViolatingTargets) {
if (this.macroNamespaceViolatingTargets == null) {
this.macroNamespaceViolatingTargets = new LinkedHashMap<>();
}
this.macroNamespaceViolatingTargets.putAll(macroNamespaceViolatingTargets);
}

/**
* Throws {@link NameConflictException} if the given target's name matches that of an existing
* target in the package, or an existing macro in the package that is not its ancestor.
Expand Down

0 comments on commit e811565

Please sign in to comment.