diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/analysis')
7 files changed, 115 insertions, 55 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java index 0bccc72032..07f7a02190 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java @@ -45,6 +45,11 @@ public interface AnalysisEnvironment extends ActionRegistry { * Returns the artifact for the derived file {@code rootRelativePath}. * * <p>Creates the artifact if necessary and sets the root of that artifact to {@code root}. + * + * <p>This method can create artifacts anywhere in the output tree, thus making it possible for + * artifacts generated by two different rules to clash. To avoid this, use the methods + * {@code getUniqueDirectoryArtifact} and {@code getPackageRelativeArtifact} on + * {@link RuleContext}. */ Artifact getDerivedArtifact(PathFragment rootRelativePath, Root root); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java index 3ded47d104..9abcb2586a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java @@ -125,7 +125,7 @@ public final class AnalysisUtils { * <p>For example "//pkg:target" -> "pkg/<fragment>/target. */ public static PathFragment getUniqueDirectory(Label label, PathFragment fragment) { - return label.getPackageFragment().getRelative(fragment) + return label.getPackageIdentifier().getPathFragment().getRelative(fragment) .getRelative(label.getName()); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java index 2ac3bcc48c..d71998b86e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java @@ -88,9 +88,8 @@ public class PseudoAction<InfoType extends MessageLite> extends AbstractAction { } public static Artifact getDummyOutput(RuleContext ruleContext) { - return ruleContext.getAnalysisEnvironment().getDerivedArtifact( - ruleContext.getLabel().toPathFragment().replaceName( - ruleContext.getLabel().getName() + ".extra_action_dummy"), + return ruleContext.getPackageRelativeArtifact( + ruleContext.getLabel().getName() + ".extra_action_dummy", ruleContext.getConfiguration().getGenfilesDirectory()); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 8f5351fd8c..875e89ff74 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -358,10 +358,10 @@ public final class RuleContext extends TargetContext @Override public void attributeError(String attrName, String message) { reportError(rule.getAttributeLocation(attrName), - prefixAttributeMessage(Attribute.isImplicit(attrName) - ? "(an implicit dependency)" - : attrName, - message)); + prefixAttributeMessage(Attribute.isImplicit(attrName) + ? "(an implicit dependency)" + : attrName, + message)); } /** @@ -373,10 +373,10 @@ public final class RuleContext extends TargetContext @Override public void attributeWarning(String attrName, String message) { reportWarning(rule.getAttributeLocation(attrName), - prefixAttributeMessage(Attribute.isImplicit(attrName) - ? "(an implicit dependency)" - : attrName, - message)); + prefixAttributeMessage(Attribute.isImplicit(attrName) + ? "(an implicit dependency)" + : attrName, + message)); } private String prefixAttributeMessage(String attrName, String message) { @@ -424,8 +424,12 @@ public final class RuleContext extends TargetContext * signature. */ private Artifact internalCreateOutputArtifact(Target target) { + Preconditions.checkState( + target.getLabel().getPackageIdentifier().equals(getLabel().getPackageIdentifier()), + "Creating output artifact for target '%s' in different package than the rule '%s' " + + "being analyzed", target.getLabel(), getLabel()); Root root = getBinOrGenfilesDirectory(); - return getAnalysisEnvironment().getDerivedArtifact(Util.getWorkspaceRelativePath(target), root); + return getPackageRelativeArtifact(target.getName(), root); } /** @@ -440,6 +444,71 @@ public final class RuleContext extends TargetContext : getConfiguration().getGenfilesDirectory(); } + /** + * Creates an artifact in a directory that is unique to the package that contains the rule, + * thus guaranteeing that it never clashes with artifacts created by rules in other packages. + */ + public Artifact getPackageRelativeArtifact(String relative, Root root) { + return getPackageRelativeArtifact(new PathFragment(relative), root); + } + + /** + * Creates an artifact in a directory that is unique to the package that contains the rule, + * thus guaranteeing that it never clashes with artifacts created by rules in other packages. + */ + public Artifact getPackageRelativeArtifact(PathFragment relative, Root root) { + return getDerivedArtifact(getPackageDirectory().getRelative(relative), root); + } + + /** + * Returns the root-relative path fragment under which output artifacts of this rule should go. + * + * <p>Note that: + * <ul> + * <li>This doesn't guarantee that there are no clashes with rules in the same package. + * <li>If possible, {@link #getPackageRelativeArtifact(PathFragment, Root)} should be used + * instead of this method. + * </ul> + * + * Ideally, user-visible artifacts should all have corresponding output file targets, all others + * should go into a rule-specific directory. + * {@link #getUniqueDirectoryArtifact(String, PathFragment, Root)}) ensures that this is the case. + */ + public PathFragment getPackageDirectory() { + return getLabel().getPackageIdentifier().getPathFragment(); + } + + /** + * Creates an artifact under a given root with the given root-relative path. + * + * <p>Verifies that it is in the root-relative directory corresponding to the package of the rule, + * thus ensuring that it doesn't clash with other artifacts generated by other rules using this + * method. + */ + public Artifact getDerivedArtifact(PathFragment rootRelativePath, Root root) { + Preconditions.checkState(rootRelativePath.startsWith(getPackageDirectory()), + "Output artifact '%s' not under package directory '%s' for target '%s'", + rootRelativePath, getPackageDirectory(), getLabel()); + return getAnalysisEnvironment().getDerivedArtifact(rootRelativePath, root); + } + /** + * Creates an artifact in a directory that is unique to the rule, thus guaranteeing that it never + * clashes with artifacts created by other rules. + */ + public Artifact getUniqueDirectoryArtifact( + String uniqueDirectory, String relative, Root root) { + return getUniqueDirectoryArtifact(uniqueDirectory, new PathFragment(relative), root); + } + + public Artifact getUniqueDirectoryArtifact( + String uniqueDirectory, PathFragment relative, Root root) { + return getDerivedArtifact(getUniqueDirectory(uniqueDirectory).getRelative(relative), root); + } + + public PathFragment getArtifactPackagePrefix() { + return getLabel().getPackageIdentifier().getPathFragment(); + } + private Attribute getAttribute(String attributeName) { // TODO(bazel-team): We should check original rule for such attribute first, because aspect // can't contain empty attribute. Consider changing type of prerequisiteMap from @@ -982,9 +1051,7 @@ public final class RuleContext extends TargetContext * Only use from Skylark. Returns the implicit output artifact for a given output path. */ public Artifact getImplicitOutputArtifact(String path) { - Root root = getBinOrGenfilesDirectory(); - PathFragment packageFragment = getLabel().getPackageFragment(); - return getAnalysisEnvironment().getDerivedArtifact(packageFragment.getRelative(path), root); + return getPackageRelativeArtifact(path, getBinOrGenfilesDirectory()); } /** @@ -1061,7 +1128,7 @@ public final class RuleContext extends TargetContext */ public final Artifact getRelatedArtifact(PathFragment pathFragment, String extension) { PathFragment file = FileSystemUtils.replaceExtension(pathFragment, extension); - return getAnalysisEnvironment().getDerivedArtifact(file, getConfiguration().getBinDirectory()); + return getDerivedArtifact(file, getConfiguration().getBinDirectory()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index dd91cfb975..fcfce07af5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -88,7 +88,7 @@ public class RunfilesSupport { */ private RunfilesSupport(RuleContext ruleContext, Artifact executable, Runfiles runfiles, List<String> appendingArgs, boolean createSymlinks) { - owningExecutable = executable; + owningExecutable = Preconditions.checkNotNull(executable); this.createSymlinks = createSymlinks; // Adding run_under target to the runfiles manifest so it would become part @@ -123,19 +123,6 @@ public class RunfilesSupport { .build(); } - private RunfilesSupport(Runfiles runfiles, Artifact runfilesInputManifest, - Artifact runfilesManifest, Artifact runfilesMiddleman, Artifact sourcesManifest, - Artifact owningExecutable, boolean createSymlinks, ImmutableList<String> args) { - this.runfiles = runfiles; - this.runfilesInputManifest = runfilesInputManifest; - this.runfilesManifest = runfilesManifest; - this.runfilesMiddleman = runfilesMiddleman; - this.sourcesManifest = sourcesManifest; - this.owningExecutable = owningExecutable; - this.createSymlinks = createSymlinks; - this.args = args; - } - /** * Returns the executable owning this RunfilesSupport. Only use from Skylark. */ @@ -149,10 +136,6 @@ public class RunfilesSupport { * returns null. */ public PathFragment getRunfilesDirectoryExecPath() { - if (owningExecutable == null) { - return null; - } - PathFragment executablePath = owningExecutable.getExecPath(); return executablePath.getParentDirectory().getChild( executablePath.getBaseName() + RUNFILES_DIR_EXT); @@ -181,14 +164,14 @@ public class RunfilesSupport { return runfilesInputManifest; } - private Artifact createRunfilesInputManifestArtifact(ActionConstructionContext context) { + private Artifact createRunfilesInputManifestArtifact(RuleContext context) { // The executable may be null for emptyRunfiles PathFragment relativePath = (owningExecutable != null) ? owningExecutable.getRootRelativePath() - : Util.getWorkspaceRelativePath(context.getRule()); + : context.getPackageDirectory().getRelative(context.getLabel().getName()); String basename = relativePath.getBaseName(); PathFragment inputManifestPath = relativePath.replaceName(basename + ".runfiles_manifest"); - return context.getAnalysisEnvironment().getDerivedArtifact(inputManifestPath, + return context.getDerivedArtifact(inputManifestPath, context.getConfiguration().getBinDirectory()); } @@ -259,7 +242,6 @@ public class RunfilesSupport { /** * Returns the Sources manifest. - * This may be null if the owningRule has no executable. */ public Artifact getSourceManifest() { return sourcesManifest; @@ -302,7 +284,7 @@ public class RunfilesSupport { PathFragment outputManifestPath = runfilesDir.getRelative("MANIFEST"); BuildConfiguration config = context.getConfiguration(); - Artifact outputManifest = context.getAnalysisEnvironment().getDerivedArtifact( + Artifact outputManifest = context.getDerivedArtifact( outputManifestPath, config.getBinDirectory()); context.getAnalysisEnvironment().registerAction(new SymlinkTreeAction( context.getActionOwner(), inputManifest, outputManifest, /*filesetTree=*/false)); @@ -318,18 +300,14 @@ public class RunfilesSupport { */ private Artifact createSourceManifest(ActionConstructionContext context, Runfiles runfiles) { // Put the sources only manifest next to the MANIFEST file but call it SOURCES. - PathFragment runfilesDir = getRunfilesDirectoryExecPath(); - if (runfilesDir != null) { - PathFragment sourcesManifestPath = runfilesDir.getRelative("SOURCES"); - Artifact sourceOnlyManifest = context.getAnalysisEnvironment().getDerivedArtifact( - sourcesManifestPath, context.getConfiguration().getBinDirectory()); - context.getAnalysisEnvironment().registerAction( - SourceManifestAction.forRunfiles( - ManifestType.SOURCES_ONLY, context.getActionOwner(), sourceOnlyManifest, runfiles)); - return sourceOnlyManifest; - } else { - return null; - } + PathFragment executablePath = owningExecutable.getRootRelativePath(); + PathFragment sourcesManifestPath = executablePath.getParentDirectory().getChild( + executablePath.getBaseName() + ".runfiles.SOURCES"); + Artifact sourceOnlyManifest = context.getDerivedArtifact( + sourcesManifestPath, context.getConfiguration().getBinDirectory()); + context.getAnalysisEnvironment().registerAction(SourceManifestAction.forRunfiles( + ManifestType.SOURCES_ONLY, context.getActionOwner(), sourceOnlyManifest, runfiles)); + return sourceOnlyManifest; } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionConstructionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionConstructionContext.java index b7461e5f8f..71261a78f3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionConstructionContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ActionConstructionContext.java @@ -14,9 +14,12 @@ package com.google.devtools.build.lib.analysis.actions; import com.google.devtools.build.lib.actions.ActionOwner; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.vfs.PathFragment; /** * A temporary interface to allow migration from RuleConfiguredTarget to RuleContext. It bundles @@ -34,4 +37,13 @@ public interface ActionConstructionContext { /** The current analysis environment. */ AnalysisEnvironment getAnalysisEnvironment(); + + /** + * Creates an artifact under a given root with the given root-relative path. + * + * <p>Verifies that it is in the root-relative directory corresponding to the package of the rule, + * thus ensuring that it doesn't clash with other artifacts generated by other rules using this + * method. + */ + Artifact getDerivedArtifact(PathFragment rootRelativePath, Root root); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java index 1128617c78..4ffaa92c05 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java @@ -135,9 +135,8 @@ public class FileWriteAction extends AbstractFileWriteAction { */ public static Artifact createFile(RuleContext ruleContext, String fileName, CharSequence contents, boolean executable) { - Artifact scriptFileArtifact = ruleContext.getAnalysisEnvironment().getDerivedArtifact( - ruleContext.getTarget().getLabel().getPackageFragment().getRelative(fileName), - ruleContext.getConfiguration().getGenfilesDirectory()); + Artifact scriptFileArtifact = ruleContext.getPackageRelativeArtifact( + fileName, ruleContext.getConfiguration().getGenfilesDirectory()); ruleContext.registerAction(new FileWriteAction( ruleContext.getActionOwner(), scriptFileArtifact, contents, executable)); return scriptFileArtifact; |