Check that most output artifacts are under a directory determined by the repository and package of the rule being analyzed. Currently this directory is PACKAGE for rules in the main repository and external/REPOSITORY_NAME/PACKAGE for rules in other repositories.
This is a plan to fix #293. Ideally, we would simply make it impossible to create artifacts not under that location, but in practice, we cannot do that because some rules do want to do this, mostly those that are already problematic due to shared actions. So the battle plan is to eliminate as many calls to AnalysisEnvironment.getDerivedArtifact() as I possibly can and audit the rest.
--
MOS_MIGRATED_REVID=99351151
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 8f5351f..875e89f 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 @@
@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 @@
@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 @@
* 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 @@
: 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 @@
* 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 Artifact getRelatedArtifact(PathFragment pathFragment, String extension) {
PathFragment file = FileSystemUtils.replaceExtension(pathFragment, extension);
- return getAnalysisEnvironment().getDerivedArtifact(file, getConfiguration().getBinDirectory());
+ return getDerivedArtifact(file, getConfiguration().getBinDirectory());
}
/**
