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

2 files changed, 16 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 55d65ef316..326575c9d3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParameterFile;
-import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.OutputGroupProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -32,7 +31,6 @@ import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
-import com.google.devtools.build.lib.analysis.Util;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -191,8 +189,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     // linkopt "-shared", which causes the result of linking to be a shared
     // library. In this case, the name of the executable target should end
     // in ".so".
-    PathFragment executableName = Util.getWorkspaceRelativePath(
-        ruleContext.getTarget(), "", OsUtils.executableExtension());
+    PathFragment executableName = ruleContext.getPackageDirectory().getRelative(
+        ruleContext.getTarget().getName() +  OsUtils.executableExtension());
     CppLinkAction.Builder linkActionBuilder = determineLinkerArguments(
         ruleContext, common, cppConfiguration, ccCompilationOutputs,
         cppCompilationContext.getCompilationPrerequisites(), fake, executableName);
@@ -543,8 +541,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
 
       for (SpawnAction.Builder packager : packagers) {
         Artifact intermediateOutput =
-            getIntermediateDwpFile(
-                context.getAnalysisEnvironment(), dwpOutput, intermediateDwpCount++);
+            getIntermediateDwpFile(context, dwpOutput, intermediateDwpCount++);
         context.registerAction(packager
             .addArgument("-o")
             .addOutputArgument(intermediateOutput)
@@ -574,14 +571,13 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   /**
    * Creates an intermediate dwp file keyed off the name and path of the final output.
    */
-  private static Artifact getIntermediateDwpFile(AnalysisEnvironment env, Artifact dwpOutput,
+  private static Artifact getIntermediateDwpFile(RuleContext ruleContext, Artifact dwpOutput,
       int orderNumber) {
     PathFragment outputPath = dwpOutput.getRootRelativePath();
     PathFragment intermediatePath =
         FileSystemUtils.appendWithoutExtension(outputPath, "-" + orderNumber);
-    return env.getDerivedArtifact(
-        outputPath.getParentDirectory().getRelative(
-            INTERMEDIATE_DWP_DIR + "/" + intermediatePath.getPathString()),
+    return ruleContext.getPackageRelativeArtifact(
+        new PathFragment(INTERMEDIATE_DWP_DIR + "/" + intermediatePath.getPathString()),
         dwpOutput.getRoot());
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 6833e291d7..751be081f5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -28,7 +28,6 @@ import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
-import com.google.devtools.build.lib.analysis.Util;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
@@ -41,7 +40,6 @@ import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.syntax.Label.SyntaxException;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.IncludeScanningUtil;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
 
@@ -303,11 +301,12 @@ public class CppHelper {
    */
   public static PathFragment getLinkedFilename(RuleContext ruleContext,
       LinkTargetType linkType) {
-    PathFragment relativePath = Util.getWorkspaceRelativePath(ruleContext.getTarget());
-    PathFragment linkedFileName = (linkType == LinkTargetType.EXECUTABLE) ?
-        relativePath :
-        relativePath.replaceName("lib" + relativePath.getBaseName() + linkType.getExtension());
-    return linkedFileName;
+    PathFragment result =
+        ruleContext.getPackageDirectory().getRelative(ruleContext.getLabel().getName());
+    if (linkType != LinkTargetType.EXECUTABLE) {
+      result = result.replaceName("lib" + result.getBaseName() + linkType.getExtension());
+    }
+    return result;
   }
 
   /**
@@ -404,11 +403,10 @@ public class CppHelper {
   public static CppModuleMap addCppModuleMapToContext(RuleContext ruleContext,
       CppCompilationContext.Builder contextBuilder) {
     // Create the module map artifact as a genfile.
-    PathFragment mapPath = FileSystemUtils.appendExtension(ruleContext.getLabel().toPathFragment(),
-        Iterables.getOnlyElement(CppFileTypes.CPP_MODULE_MAP.getExtensions()));
-    Artifact mapFile = ruleContext.getAnalysisEnvironment().getDerivedArtifact(mapPath,
-        ruleContext.getConfiguration().getGenfilesDirectory());
-    CppModuleMap moduleMap =
+    Artifact mapFile = ruleContext.getPackageRelativeArtifact(
+        ruleContext.getLabel().getName()
+            + Iterables.getOnlyElement(CppFileTypes.CPP_MODULE_MAP.getExtensions()),
+        ruleContext.getConfiguration().getGenfilesDirectory());    CppModuleMap moduleMap =
         new CppModuleMap(mapFile, ruleContext.getLabel().toString());
     contextBuilder.setCppModuleMap(moduleMap);
     return moduleMap;