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

6 files changed, 46 insertions, 78 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index e7c2fa501a..87d2253a14 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -56,7 +56,6 @@ import com.google.devtools.build.lib.rules.java.JavaCommon;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
-import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
@@ -791,8 +790,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
   static DexingOutput dex(RuleContext ruleContext, MultidexMode multidexMode, List<String> dexopts,
       Artifact deployJar,  Artifact proguardedJar, AndroidCommon common,
       JavaTargetAttributes attributes) {
-    Artifact classesDex = AndroidBinary.getDxArtifact(ruleContext,
-        getMultidexMode(ruleContext).getOutputDexFilename());
+    String classesDexFileName = getMultidexMode(ruleContext).getOutputDexFilename();
+    Artifact classesDex = AndroidBinary.getDxArtifact(ruleContext, classesDexFileName);
     if (!AndroidBinary.supportsMultidexMode(ruleContext, multidexMode)) {
       ruleContext.ruleError("Multidex mode \"" + multidexMode.getAttributeValue()
           + "\" not supported by this version of the Android SDK");
@@ -904,10 +903,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         return new DexingOutput(classesDex, javaResourceJar, shardDexes);
       } else {
         // Create an artifact for the intermediate zip output that includes non-.dex files.
-        PathFragment dexPath = classesDex.getRootRelativePath();
-        Artifact classesDexIntermediate = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-            dexPath.getParentDirectory().getRelative("intermediate_" + dexPath.getBaseName()),
-            ruleContext.getBinOrGenfilesDirectory());
+        Artifact classesDexIntermediate = AndroidBinary.getDxArtifact(
+            ruleContext,
+            "intermediate_" + classesDexFileName);
 
         // Have the dexer generate the intermediate file and the "cleaner" action consume this to
         // generate the final archive with only .dex files.
@@ -1255,8 +1253,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
    * Returns an intermediate artifact used to support dex generation.
    */
   public static Artifact getDxArtifact(RuleContext ruleContext, String baseName) {
-    return ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-        ruleContext.getUniqueDirectory("_dx").getRelative(baseName),
+    return ruleContext.getUniqueDirectoryArtifact("_dx", baseName,
         ruleContext.getBinOrGenfilesDirectory());
   }
 
@@ -1266,11 +1263,10 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
   public static Artifact getProguardConfigArtifact(RuleContext ruleContext, String prefix) {
     // TODO(bazel-team): Remove the redundant inclusion of the rule name, as getUniqueDirectory
     // includes the rulename as well.
-    return Preconditions.checkNotNull(
-        ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-            ruleContext.getUniqueDirectory("proguard").getRelative(
-                Joiner.on("_").join(prefix, ruleContext.getLabel().getName(), "proguard.cfg")),
-            ruleContext.getBinOrGenfilesDirectory()));
+    return Preconditions.checkNotNull(ruleContext.getUniqueDirectoryArtifact(
+        "proguard",
+        Joiner.on("_").join(prefix, ruleContext.getLabel().getName(), "proguard.cfg"),
+        ruleContext.getBinOrGenfilesDirectory()));
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index a34cb55824..e5bc26c15f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -300,12 +300,9 @@ public class AndroidCommon {
     // Since the Java sources are generated by combining all resources with the
     // ones included in the binary, the path of the artifact has to be unique
     // per binary and per library (not only per library).
-    PathFragment resourceJarsPathFragment = ruleContext.getUniqueDirectory("resource_jars");
-    PathFragment artifactPathFragment = resourceJarsPathFragment.getRelative(
-        container.getLabel().getPackageFragment().getRelative(artifactName));
-
-    Artifact artifact = ruleContext.getAnalysisEnvironment()
-        .getDerivedArtifact(artifactPathFragment, ruleContext.getBinOrGenfilesDirectory());
+    Artifact artifact = ruleContext.getUniqueDirectoryArtifact("resource_jars",
+        container.getLabel().getPackageIdentifier().getPathFragment().getRelative(artifactName),
+        ruleContext.getBinOrGenfilesDirectory());
     return artifact;
   }
 
@@ -647,16 +644,15 @@ public class AndroidCommon {
   private ImmutableMap<Artifact, Artifact> generateTranslatedIdlArtifacts(
       RuleContext ruleContext, Collection<Artifact> idls) {
     ImmutableMap.Builder<Artifact, Artifact> outputJavaSources = ImmutableMap.builder();
-    PathFragment rulePackage = ruleContext.getRule().getLabel().getPackageFragment();
     String ruleName = ruleContext.getRule().getName();
     // for each aidl file use aggregated preprocessed files to generate Java code
     for (Artifact idl : idls) {
       // Reconstruct the package tree under <rule>_aidl to avoid a name conflict
       // if the same AIDL files are used in multiple targets.
       PathFragment javaOutputPath = FileSystemUtils.replaceExtension(
-          rulePackage.getRelative(ruleName + "_aidl").getRelative(idl.getRootRelativePath()),
+          new PathFragment(ruleName + "_aidl").getRelative(idl.getRootRelativePath()),
           ".java");
-      Artifact output = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
+      Artifact output = ruleContext.getPackageRelativeArtifact(
           javaOutputPath, ruleContext.getConfiguration().getGenfilesDirectory());
       outputJavaSources.put(idl, output);
     }
@@ -688,14 +684,13 @@ public class AndroidCommon {
 
     // preprocess each aidl file
     preprocessedArgs.add("-p" + sdk.getFrameworkAidl().getExecPathString());
-    PathFragment rulePackage = ruleContext.getRule().getLabel().getPackageFragment();
     String ruleName = ruleContext.getRule().getName();
     for (Artifact idl : idls) {
       // Reconstruct the package tree under <rule>_aidl to avoid a name conflict
       // if the source AIDL files are also generated.
-      PathFragment preprocessedPath = rulePackage.getRelative(ruleName + "_aidl")
+      PathFragment preprocessedPath = new PathFragment(ruleName + "_aidl")
           .getRelative(idl.getRootRelativePath());
-      Artifact preprocessed = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
+      Artifact preprocessed = ruleContext.getPackageRelativeArtifact(
           preprocessedPath, ruleContext.getConfiguration().getGenfilesDirectory());
       preprocessedIdls.add(preprocessed);
       preprocessedArgs.add("-p" + preprocessed.getExecPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
index fa40809201..010fa4830d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
@@ -378,9 +378,10 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
       for (Artifact specToValidate : getProguardConfigs(ruleContext)) {
         //If we're validating j/a/b/testapp/proguard.cfg, the output will be:
         //j/a/b/testapp/proguard.cfg_valid
-        Artifact output = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-            specToValidate.getRootRelativePath()
-                .replaceName(specToValidate.getFilename() + "_valid"),
+        Artifact output = ruleContext.getUniqueDirectoryArtifact(
+            "validated_proguard",
+            specToValidate.getRootRelativePath().replaceName(
+                specToValidate.getFilename() + "_valid"),
             ruleContext.getBinOrGenfilesDirectory());
         ruleContext.registerAction(new SpawnAction.Builder()
             .addInput(specToValidate)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index a379cc0227..e1f84e7feb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -35,6 +35,7 @@ import com.google.devtools.build.lib.rules.android.AndroidResourcesProvider.Reso
 import com.google.devtools.build.lib.rules.android.LocalResourceContainer.Builder.InvalidAssetPath;
 import com.google.devtools.build.lib.rules.android.LocalResourceContainer.Builder.InvalidResourcePath;
 import com.google.devtools.build.lib.rules.java.JavaUtil;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.List;
 
@@ -145,9 +146,8 @@ public final class ApplicationManifest {
    * @return the generated ApplicationManifest
    */
   public static ApplicationManifest generatedManifest(RuleContext ruleContext) {
-    Artifact generatedManifest = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-        ruleContext.getUniqueDirectory(ruleContext.getRule().getName() + "_generated")
-            .getChild("AndroidManifest.xml"),
+    Artifact generatedManifest = ruleContext.getUniqueDirectoryArtifact(
+        ruleContext.getRule().getName() + "_generated", new PathFragment("AndroidManifest.xml"),
         ruleContext.getBinOrGenfilesDirectory());
 
     String manifestPackage;
@@ -179,9 +179,8 @@ public final class ApplicationManifest {
       Iterable<ResourceContainer> resourceContainers) {
     if (!Iterables.isEmpty(getMergeeManifests(resourceContainers))) {
       Iterable<Artifact> exportedManifests = getMergeeManifests(resourceContainers);
-      Artifact outputManifest = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-          ruleContext.getUniqueDirectory(
-              ruleContext.getRule().getName() + "_merged").getChild("AndroidManifest.xml"),
+      Artifact outputManifest = ruleContext.getUniqueDirectoryArtifact(
+          ruleContext.getRule().getName() + "_merged", "AndroidManifest.xml",
           ruleContext.getBinOrGenfilesDirectory());
       AndroidManifestMergeHelper.createMergeManifestAction(ruleContext, getManifest(),
           exportedManifests, ImmutableList.of("all"), outputManifest);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
index d8377d218e..de9c089be4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
@@ -57,6 +57,10 @@ import javax.annotation.Nullable;
  */
 public final class JackCompilationHelper {
 
+  private static final String PARTIAL_JACK_DIRECTORY = "_jill";
+
+  private static final String JACK_DIRECTORY = "_jack";
+
   /** Filetype for the intermediate library created by Jack. */
   public static final FileType JACK_LIBRARY_TYPE = FileType.of(".jack");
 
@@ -366,13 +370,10 @@ public final class JackCompilationHelper {
    * @see #postprocessPartialJackAndAddResources(Artifact,Artifact)
    */
   private Artifact convertJarToPartialJack(Artifact jar) {
-    PathFragment outputPath =
-        FileSystemUtils.replaceExtension(
-            getPartialJackRoot().getRelative(jar.getRootRelativePath()), ".jack");
-    Artifact result =
-        ruleContext
-            .getAnalysisEnvironment()
-            .getDerivedArtifact(outputPath, ruleContext.getBinOrGenfilesDirectory());
+    Artifact result = ruleContext.getUniqueDirectoryArtifact(
+        PARTIAL_JACK_DIRECTORY,
+        FileSystemUtils.replaceExtension(jar.getRootRelativePath(), ".jack"),
+        ruleContext.getBinOrGenfilesDirectory());
     ruleContext.registerAction(
         new SpawnAction.Builder()
             .setExecutable(jillBinary)
@@ -391,13 +392,11 @@ public final class JackCompilationHelper {
    * non-resource files and returning a zip file containing only resources.
    */
   private Artifact extractResourcesFromJar(Artifact jar) {
-    PathFragment outputPath =
-        FileSystemUtils.replaceExtension(
-            getPartialJackRoot().getRelative(jar.getRootRelativePath()), "-resources.zip");
-    Artifact result =
-        ruleContext
-            .getAnalysisEnvironment()
-            .getDerivedArtifact(outputPath, ruleContext.getBinOrGenfilesDirectory());
+    Artifact result =  ruleContext.getUniqueDirectoryArtifact(
+        PARTIAL_JACK_DIRECTORY,
+        FileSystemUtils.replaceExtension(jar.getRootRelativePath(), "-resources.zip"),
+        ruleContext.getBinOrGenfilesDirectory());
+
     ruleContext.registerAction(
         new SpawnAction.Builder()
             .setExecutable(resourceExtractorBinary)
@@ -416,13 +415,11 @@ public final class JackCompilationHelper {
    */
   private Artifact postprocessPartialJackAndAddResources(
       Artifact partialJackLibrary, Artifact resources) {
-    PathFragment outputPath =
-        getFinalizedJackRoot()
-            .getRelative(partialJackLibrary.getRootRelativePath().relativeTo(getPartialJackRoot()));
-    Artifact result =
-        ruleContext
-            .getAnalysisEnvironment()
-            .getDerivedArtifact(outputPath, ruleContext.getBinOrGenfilesDirectory());
+    Artifact result = ruleContext.getUniqueDirectoryArtifact(
+        JACK_DIRECTORY,
+        partialJackLibrary.getRootRelativePath().relativeTo(
+            ruleContext.getUniqueDirectory(PARTIAL_JACK_DIRECTORY)),
+        ruleContext.getBinOrGenfilesDirectory());
     CustomCommandLine.Builder builder =
         CustomCommandLine.builder()
             // Have jack double-check its behavior and crash rather than producing invalid output
@@ -446,26 +443,6 @@ public final class JackCompilationHelper {
   }
 
   /**
-   * Creates an intermediate directory to store partially-converted Jack libraries.
-   *
-   * @see #convertJarToPartialJack(Artifact)
-   */
-  private PathFragment getPartialJackRoot() {
-    PathFragment rulePath = ruleContext.getLabel().toPathFragment();
-    return rulePath.replaceName(rulePath.getBaseName() + "_jill");
-  }
-
-  /**
-   * Creates an intermediate directory to store fully-converted Jack libraries.
-   *
-   * @see #postprocessPartialJackAndAddResources(Artifact,Artifact)
-   */
-  private PathFragment getFinalizedJackRoot() {
-    PathFragment rulePath = ruleContext.getLabel().toPathFragment();
-    return rulePath.replaceName(rulePath.getBaseName() + "_jack");
-  }
-
-  /**
    * Creates an action to build an empty jack library given by outputArtifact.
    */
   private void buildEmptyJackAction() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
index 965106e7aa..7c48f1bdc8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
@@ -80,8 +80,8 @@ public final class NativeLibs {
       // The native deps name file must be the only file in its directory because ApkBuilder does
       // not have an option to add a particular file to the .apk, only one to add every file in a
       // particular directory.
-      Artifact nativeDepsName = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-          ruleContext.getUniqueDirectory("nativedeps_filename").getRelative(nativeDepsFileName),
+      Artifact nativeDepsName = ruleContext.getUniqueDirectoryArtifact(
+          "nativedeps_filename", nativeDepsFileName,
           ruleContext.getBinOrGenfilesDirectory());
       ruleContext.registerAction(new FileWriteAction(ruleContext.getActionOwner(), nativeDepsName,
           anyNativeLibrary.getExecPath().getBaseName(), false));