Skip to content

Commit

Permalink
@AutoCodec all the *ConfiguredTarget classes except for RuleConfigure…
Browse files Browse the repository at this point in the history
…dTarget. RuleConfiguredTarget is harder, and will be handled in a follow-up.

Also remove duplicate field from InputFileConfiguredTarget and unused parameter in EnvironmentGroupConfiguredTarget constructor.

Largely punt on FilesetOutputConfiguredTarget for now, but will handle soon.

PiperOrigin-RevId: 186829768
  • Loading branch information
janakdr authored and Copybara-Service committed Feb 23, 2018
1 parent 6966a2e commit 14e549c
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ public final ConfiguredTarget createConfiguredTarget(
PackageGroup packageGroup = (PackageGroup) target;
return new PackageGroupConfiguredTarget(targetContext, packageGroup);
} else if (target instanceof EnvironmentGroup) {
return new EnvironmentGroupConfiguredTarget(targetContext, (EnvironmentGroup) target);
return new EnvironmentGroupConfiguredTarget(targetContext);
} else {
throw new AssertionError("Unexpected target class: " + target.getClass().getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,19 @@ public abstract class AbstractConfiguredTarget
private static final String DEFAULT_RUNFILES_FIELD = "default_runfiles";

public AbstractConfiguredTarget(Label label, BuildConfiguration configuration) {
this(label, configuration, NestedSetBuilder.emptySet(Order.STABLE_ORDER));
}

protected AbstractConfiguredTarget(
Label label, BuildConfiguration configuration, NestedSet<PackageGroupContents> visibility) {
this.label = label;
this.configuration = configuration;
this.visibility = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
this.visibility = visibility;
}

@Deprecated // For callers to be serializable, they shouldn't have a TargetContext constructor.
public AbstractConfiguredTarget(TargetContext targetContext) {
this.label = targetContext.getTarget().getLabel();
this.configuration = targetContext.getConfiguration();
this.visibility = targetContext.getVisibility();
this(targetContext.getLabel(), targetContext.getConfiguration(), targetContext.getVisibility());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,26 @@

import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.analysis.TargetContext;
import com.google.devtools.build.lib.packages.EnvironmentGroup;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;

/**
* Dummy ConfiguredTarget for environment groups. Contains no functionality, since
* environment groups are not really first-class Targets.
* Dummy ConfiguredTarget for environment groups. Contains no functionality, since environment
* groups are not really first-class Targets.
*/
@AutoCodec
public final class EnvironmentGroupConfiguredTarget extends AbstractConfiguredTarget {
public EnvironmentGroupConfiguredTarget(TargetContext targetContext, EnvironmentGroup envGroup) {
super(targetContext);
Preconditions.checkArgument(targetContext.getConfiguration() == null);
@AutoCodec.Instantiator
@AutoCodec.VisibleForSerialization
EnvironmentGroupConfiguredTarget(Label label) {
super(label, null);
}

public EnvironmentGroupConfiguredTarget(TargetContext targetContext) {
this(targetContext.getLabel());
Preconditions.checkState(targetContext.getConfiguration() == null, targetContext);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.LicensesProvider;
import com.google.devtools.build.lib.analysis.TargetContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap;
import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder;
import com.google.devtools.build.lib.analysis.VisibilityProvider;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.fileset.FilesetProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.cmdline.Label;
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.packages.Info;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.util.FileType;

Expand All @@ -43,8 +45,12 @@ public abstract class FileConfiguredTarget extends AbstractConfiguredTarget
private final Artifact artifact;
private final TransitiveInfoProviderMap providers;

FileConfiguredTarget(TargetContext targetContext, Artifact artifact) {
super(targetContext);
FileConfiguredTarget(
Label label,
BuildConfiguration configuration,
NestedSet<PackageGroupContents> visibility,
Artifact artifact) {
super(label, configuration, visibility);
NestedSet<Artifact> filesToBuild = NestedSetBuilder.create(Order.STABLE_ORDER, artifact);
this.artifact = artifact;
FileProvider fileProvider = new FileProvider(filesToBuild);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,17 @@ public FilesetOutputConfiguredTarget(
TransitiveInfoCollection generatingRule,
Artifact outputArtifact,
@Nullable ImmutableList<FilesetTraversalParams> traversals) {
super(targetContext, outputFile, generatingRule, outputArtifact);
super(
targetContext.getLabel(),
targetContext.getConfiguration(),
targetContext.getVisibility(),
outputArtifact,
generatingRule);
Preconditions.checkState(
outputFile.getLabel().equals(targetContext.getLabel()),
"mismatch: %s %s",
outputFile,
targetContext);
FilesetProvider provider = generatingRule.getProvider(FilesetProvider.class);
Preconditions.checkArgument(provider != null);
filesetInputManifest = provider.getFilesetInputManifest();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,53 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.TargetContext;
import com.google.devtools.build.lib.cmdline.Label;
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.packages.InputFile;
import com.google.devtools.build.lib.packages.License;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;

/**
* A ConfiguredTarget for an InputFile.
*
* All InputFiles for the same target are equivalent, so configuration does not
* play any role here and is always set to <b>null</b>.
* <p>All InputFiles for the same target are equivalent, so configuration does not play any role
* here and is always set to <b>null</b>.
*/
@AutoCodec
public final class InputFileConfiguredTarget extends FileConfiguredTarget implements SkylarkValue {
private final Artifact artifact;
private final NestedSet<TargetLicense> licenses;

public InputFileConfiguredTarget(TargetContext targetContext, InputFile inputFile,
Artifact artifact) {
super(targetContext, artifact);
Preconditions.checkArgument(targetContext.getTarget() == inputFile, getLabel());
Preconditions.checkArgument(getConfiguration() == null, getLabel());
this.artifact = artifact;
@Instantiator
@VisibleForSerialization
InputFileConfiguredTarget(
Label label,
NestedSet<PackageGroupContents> visibility,
Artifact artifact,
NestedSet<TargetLicense> licenses) {
super(label, null, visibility, artifact);
this.licenses = licenses;
}

if (inputFile.getLicense() != License.NO_LICENSE) {
licenses = NestedSetBuilder.create(Order.LINK_ORDER,
new TargetLicense(getLabel(), inputFile.getLicense()));
} else {
licenses = NestedSetBuilder.emptySet(Order.LINK_ORDER);
}
public InputFileConfiguredTarget(
TargetContext targetContext, InputFile inputFile, Artifact artifact) {
this(inputFile.getLabel(), targetContext.getVisibility(), artifact, makeLicenses(inputFile));
Preconditions.checkArgument(getConfiguration() == null, getLabel());
Preconditions.checkArgument(targetContext.getTarget() == inputFile, getLabel());
}

@Override
public Artifact getArtifact() {
return artifact;
private static NestedSet<TargetLicense> makeLicenses(InputFile inputFile) {
License license = inputFile.getLicense();
return license == License.NO_LICENSE
? NestedSetBuilder.emptySet(Order.LINK_ORDER)
: NestedSetBuilder.create(
Order.LINK_ORDER, new TargetLicense(inputFile.getLabel(), license));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,21 @@
import com.google.devtools.build.lib.analysis.TargetContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProviderImpl;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.util.Pair;

/**
* A ConfiguredTarget for an OutputFile.
*/
/** A ConfiguredTarget for an OutputFile. */
@AutoCodec
public class OutputFileConfiguredTarget extends FileConfiguredTarget
implements InstrumentedFilesProvider {

Expand All @@ -39,8 +44,24 @@ public class OutputFileConfiguredTarget extends FileConfiguredTarget
public OutputFileConfiguredTarget(
TargetContext targetContext, OutputFile outputFile,
TransitiveInfoCollection generatingRule, Artifact outputArtifact) {
super(targetContext, outputArtifact);
this(
targetContext.getLabel(),
targetContext.getConfiguration(),
targetContext.getVisibility(),
outputArtifact,
generatingRule);
Preconditions.checkArgument(targetContext.getTarget() == outputFile);
}

@Instantiator
@VisibleForSerialization
OutputFileConfiguredTarget(
Label label,
BuildConfiguration configuration,
NestedSet<PackageGroupContents> visibility,
Artifact artifact,
TransitiveInfoCollection generatingRule) {
super(label, configuration, visibility, artifact);
this.generatingRule = Preconditions.checkNotNull(generatingRule);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.build.lib.analysis.configuredtargets;

import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.PackageSpecificationProvider;
Expand All @@ -30,6 +29,8 @@
import com.google.devtools.build.lib.packages.PackageGroup;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;

/**
* Dummy ConfiguredTarget for package groups. Contains no functionality, since
Expand All @@ -42,10 +43,25 @@ public final class PackageGroupConfiguredTarget extends AbstractConfiguredTarget

private final NestedSet<PackageGroupContents> packageSpecifications;

@VisibleForSerialization
@Instantiator
PackageGroupConfiguredTarget(
Label label,
NestedSet<PackageGroupContents> visibility,
NestedSet<PackageGroupContents> packageSpecifications) {
super(label, null, visibility);
this.packageSpecifications = packageSpecifications;
}

public PackageGroupConfiguredTarget(TargetContext targetContext, PackageGroup packageGroup) {
super(targetContext);
Preconditions.checkArgument(targetContext.getConfiguration() == null);
this(
targetContext.getLabel(),
targetContext.getVisibility(),
getPackageSpecifications(targetContext, packageGroup));
}

private static NestedSet<PackageGroupContents> getPackageSpecifications(
TargetContext targetContext, PackageGroup packageGroup) {
NestedSetBuilder<PackageGroupContents> builder = NestedSetBuilder.stableOrder();
for (Label label : packageGroup.getIncludes()) {
TransitiveInfoCollection include = targetContext.maybeFindDirectPrerequisite(
Expand All @@ -67,7 +83,7 @@ public PackageGroupConfiguredTarget(TargetContext targetContext, PackageGroup pa
}

builder.add(packageGroup.getPackageSpecifications());
packageSpecifications = builder.build();
return builder.build();
}

@Override
Expand Down

0 comments on commit 14e549c

Please sign in to comment.