Merge branch 'master' into HEAD

bazelbuild · Sep 13, 2024 · ea3d750 · ea3d750
2 parents 66dd968 + 3b7e1e2
commit ea3d750
Show file tree

Hide file tree

Showing 37 changed files with 300 additions and 224 deletions.
diff --git a/.github/workflows/cherry-picker.yml b/.github/workflows/cherry-picker.yml
@@ -1,8 +1,9 @@
 name: cherry-picker
 
 on:
-  pull_request_target:
+  pull_request:
     types: [closed]
+    branches: [master]
   issues:
     types: [closed, milestoned]
 

diff --git a/.github/workflows/update-lockfiles.yml b/.github/workflows/update-lockfiles.yml
@@ -1,7 +1,7 @@
 name: update-lockfiles
 
 on:
-  pull_request_target:
+  pull_request:
     branches:
       - "release-**"
     types:

diff --git a/site/en/community/images/virtuslab.svg b/site/en/community/images/virtuslab.svg
diff --git a/site/en/docs/cc-toolchain-config-reference.md b/site/en/docs/cc-toolchain-config-reference.md
@@ -796,7 +796,7 @@ Note: The **Action** column indicates the relevant action type, if applicable.
    </td>
   </tr>
   <tr>
-   <td><strong><code>stripotps</code></strong>
+   <td><strong><code>stripopts</code></strong>
    </td>
    <td>strip</td>
    <td>Sequence of <code>stripopts</code>.

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Actions.java b/src/main/java/com/google/devtools/build/lib/actions/Actions.java
@@ -298,6 +298,16 @@ private static void assignOwnersAndThrowIfConflictMaybeToleratingSharedActions(
         return len1 - len2;
       };
 
+  public static boolean isRunfilesArtifactPair(Artifact runfilesTree, Artifact runfilesManifest) {
+    if (!runfilesTree.isMiddlemanArtifact()) {
+      return false;
+    }
+
+    return runfilesManifest
+        .getExecPathString()
+        .equals(runfilesTree.getExecPath().getRelative("MANIFEST").getPathString());
+  }
+
   /**
    * Finds Artifact prefix conflicts between generated artifacts. An artifact prefix conflict
    * happens if one action generates an artifact whose path is a strict prefix of another artifact's
@@ -341,7 +351,8 @@ private static void assignOwnersAndThrowIfConflictMaybeToleratingSharedActions(
         // Check length first so that we only detect strict prefix conflicts. Equal exec paths are
         // possible from shared actions.
         if (pathJ.getPathString().length() > pathI.getPathString().length()
-            && pathJ.startsWith(pathI)) {
+            && pathJ.startsWith(pathI)
+            && !isRunfilesArtifactPair(artifactI, artifactJ)) {
           ActionAnalysisMetadata actionI =
               Preconditions.checkNotNull(actionGraph.getGeneratingAction(artifactI), artifactI);
           ActionAnalysisMetadata actionJ =

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -635,12 +635,12 @@ public boolean hasKnownGeneratingAction() {
   }
 
   /**
-   * Returns true iff this is a middleman Artifact as determined by its root.
+   * Returns true iff this is a middleman Artifact.
    *
    * <p>If true, this artifact is necessarily a {@link DerivedArtifact}.
    */
-  public final boolean isMiddlemanArtifact() {
-    return root.isMiddlemanRoot();
+  public boolean isMiddlemanArtifact() {
+    return false;
   }
 
   /**
@@ -863,6 +863,8 @@ boolean differentOwnerOrRoot(ArtifactOwner owner, ArtifactRoot root) {
    */
   @VisibleForTesting
   public enum SpecialArtifactType {
+    RUNFILES,
+
     /** Google-specific legacy type. */
     FILESET,
 
@@ -902,6 +904,11 @@ public static SpecialArtifact create(
       this.type = type;
     }
 
+    @Override
+    public boolean isMiddlemanArtifact() {
+      return type == SpecialArtifactType.RUNFILES;
+    }
+
     @Override
     public boolean isFileset() {
       return type == SpecialArtifactType.FILESET;

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
@@ -250,6 +250,18 @@ public Artifact.DerivedArtifact getFilesetArtifact(
             /*contentBasedPath=*/ false);
   }
 
+  public Artifact.DerivedArtifact getRunfilesArtifact(
+      PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
+    validatePath(rootRelativePath, root);
+    return (Artifact.DerivedArtifact)
+        getArtifact(
+            root,
+            root.getExecPath().getRelative(rootRelativePath),
+            owner,
+            SpecialArtifactType.RUNFILES,
+            /* contentBasedPath= */ false);
+  }
+
   /**
    * Returns an artifact that represents a TreeArtifact; that is, a directory containing some tree
    * of ArtifactFiles unknown at analysis time.

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
@@ -158,9 +158,7 @@ public static ArtifactRoot asDerivedRoot(
         "execPath: %s contains parent directory reference (..)",
         execPath);
     Preconditions.checkArgument(
-        isOutputRootType(rootType) || isMiddlemanRootType(rootType),
-        "%s is not a derived root type",
-        rootType);
+        isOutputRootType(rootType), "%s is not a derived root type", rootType);
     Path root = execRoot.getRelative(execPath);
     return INTERNER.intern(new ArtifactRoot(Root.fromPath(root), execPath, rootType));
   }
@@ -183,7 +181,6 @@ public enum RootType {
     MainSource,
     ExternalSource,
     Output,
-    Middleman,
     // Sibling root types are in effect when --experimental_sibling_repository_layout is activated.
     // These will eventually replace the above Output and Middleman types when the flag becomes
     // the default option and then removed.
@@ -235,16 +232,6 @@ private static boolean isOutputRootType(RootType rootType) {
         || rootType == RootType.Output;
   }
 
-  private static boolean isMiddlemanRootType(RootType rootType) {
-    return rootType == RootType.SiblingMainMiddleman
-        || rootType == RootType.SiblingExternalMiddleman
-        || rootType == RootType.Middleman;
-  }
-
-  boolean isMiddlemanRoot() {
-    return isMiddlemanRootType(rootType);
-  }
-
   public boolean isExternal() {
     return rootType == RootType.ExternalSource
         || rootType == RootType.SiblingExternalOutput
@@ -256,7 +243,7 @@ public boolean isExternal() {
    * created without the --experimental_sibling_repository_layout flag set.
    */
   public boolean isLegacy() {
-    return rootType == RootType.Output || rootType == RootType.Middleman;
+    return rootType == RootType.Output;
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
@@ -15,7 +15,6 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
-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.concurrent.ThreadSafety.ThreadSafe;
@@ -52,17 +51,16 @@ public MiddlemanFactory(
    * @param runfilesTree the runfiles tree for which the middleman being created for
    * @param runfilesManifest the runfiles manifest for the runfiles tree
    * @param repoMappingManifest the repo mapping manifest for the runfiles tree
-   * @param middlemanDir the directory in which to place the middleman.
+   * @param rootRelativePath the root relative path of the runfiles tree
+   * @param root the root where the runfiles three is located
    */
   public Artifact createRunfilesMiddleman(
       ActionOwner owner,
-      @Nullable Artifact owningArtifact,
       RunfilesTree runfilesTree,
       @Nullable Artifact runfilesManifest,
       @Nullable Artifact repoMappingManifest,
-      ArtifactRoot middlemanDir) {
-    Preconditions.checkArgument(middlemanDir.isMiddlemanRoot());
-
+      PathFragment rootRelativePath,
+      ArtifactRoot root) {
     NestedSetBuilder<Artifact> depsBuilder = NestedSetBuilder.stableOrder();
     depsBuilder.addTransitive(runfilesTree.getArtifacts());
     if (runfilesManifest != null) {
@@ -73,17 +71,12 @@ public Artifact createRunfilesMiddleman(
     }
 
     NestedSet<Artifact> deps = depsBuilder.build();
-    String middlemanPath = owningArtifact == null
-       ? Label.print(owner.getLabel())
-       : owningArtifact.getRootRelativePath().getPathString();
-    String escapedFilename = Actions.escapedPath(middlemanPath);
-    PathFragment stampName = PathFragment.create("_middlemen/" + escapedFilename + "-runfiles");
-    Artifact.DerivedArtifact stampFile =
-        artifactFactory.getDerivedArtifact(stampName, middlemanDir, actionRegistry.getOwner());
+    Artifact.DerivedArtifact middlemanArtifact =
+        artifactFactory.getRunfilesArtifact(rootRelativePath, root, actionRegistry.getOwner());
     MiddlemanAction middlemanAction =
-        new MiddlemanAction(owner, runfilesTree, deps, ImmutableSet.of(stampFile));
+        new MiddlemanAction(owner, runfilesTree, deps, ImmutableSet.of(middlemanArtifact));
     actionRegistry.registerAction(middlemanAction);
-    return stampFile;
+    return middlemanArtifact;
   }
 
 }
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
@@ -327,11 +327,6 @@ public PathFragment getGenfilesFragment() {
     return getConfiguration().getGenfilesFragment(getLabel().getRepository());
   }
 
-  @Override
-  public ArtifactRoot getMiddlemanDirectory() {
-    return getConfiguration().getMiddlemanDirectory(getLabel().getRepository());
-  }
-
   public Rule getRule() {
     return rule;
   }

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
@@ -23,6 +23,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.CommandLine;
 import com.google.devtools.build.lib.actions.RunfilesTree;
 import com.google.devtools.build.lib.analysis.SourceManifestAction.ManifestType;
@@ -257,6 +258,12 @@ private static RunfilesSupport create(
       runfilesManifest = null;
     }
 
+    ArtifactRoot root = executable.getRoot();
+    PathFragment executableRootRelativePath = executable.getRootRelativePath();
+    PathFragment runfilesRootRelativePath =
+        executableRootRelativePath.replaceName(
+            executableRootRelativePath.getBaseName() + RUNFILES_DIR_EXT);
+
     RunfilesTreeImpl runfilesTree =
         new RunfilesTreeImpl(
             runfilesDirExecPath(executable),
@@ -268,7 +275,12 @@ private static RunfilesSupport create(
 
     Artifact runfilesMiddleman =
         createRunfilesMiddleman(
-            ruleContext, executable, runfilesTree, runfilesManifest, repoMappingManifest);
+            ruleContext,
+            runfilesTree,
+            runfilesManifest,
+            repoMappingManifest,
+            runfilesRootRelativePath,
+            root);
 
     return new RunfilesSupport(
         runfilesTree,
@@ -418,20 +430,21 @@ public Artifact getRunfilesMiddleman() {
 
   private static Artifact createRunfilesMiddleman(
       ActionConstructionContext context,
-      Artifact owningExecutable,
       RunfilesTree runfilesTree,
       @Nullable Artifact runfilesManifest,
-      Artifact repoMappingManifest) {
+      Artifact repoMappingManifest,
+      PathFragment rootRelativePath,
+      ArtifactRoot root) {
     return context
         .getAnalysisEnvironment()
         .getMiddlemanFactory()
         .createRunfilesMiddleman(
             context.getActionOwner(),
-            owningExecutable,
             runfilesTree,
             runfilesManifest,
             repoMappingManifest,
-            context.getMiddlemanDirectory());
+            rootRelativePath,
+            root);
   }
 
   /**

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
@@ -39,9 +39,6 @@ public interface ActionConstructionContext extends ActionRegistry {
   /** Returns the bin directory for constructed actions. */
   ArtifactRoot getBinDirectory();
 
-  /** Returns the internal directory (used for middlemen) for constructed actions. */
-  ArtifactRoot getMiddlemanDirectory();
-
   /**
    * Returns the action owner that should be used for the default execution group's execution
    * platform.

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java
@@ -455,16 +455,6 @@ public String getHostPathSeparator() {
     return outputDirectories.getHostPathSeparator();
   }
 
-  /**
-   * Returns the internal directory (used for middlemen) for this build configuration.
-   *
-   * @deprecated Use {@code RuleContext#getMiddlemanDirectory} instead whenever possible.
-   */
-  @Deprecated
-  public ArtifactRoot getMiddlemanDirectory(RepositoryName repositoryName) {
-    return outputDirectories.getMiddlemanDirectory(repositoryName);
-  }
-
   public boolean isStrictFilesets() {
     return options.strictFilesets;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -509,7 +509,7 @@ public ExecConfigurationDistinguisherSchemeConverter() {
 
   @Option(
       name = "legacy_external_runfiles",
-      defaultValue = "true",
+      defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.OUTPUT_SELECTION,
       effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
       help =

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/config/OutputDirectories.java
@@ -67,7 +67,6 @@ public class OutputDirectories {
   public enum OutputDirectory {
     BIN("bin"),
     GENFILES("genfiles"),
-    MIDDLEMAN("internal"),
     TESTLOGS("testlogs"),
     COVERAGE("coverage-metadata"),
     OUTPUT("");
@@ -88,11 +87,7 @@ public ArtifactRoot getRoot(
       Path execRoot = directories.getExecRoot(workspaceName);
       // e.g., [[execroot/my_workspace]/bazel-out/config/bin]
       return ArtifactRoot.asDerivedRoot(
-          execRoot,
-          this == MIDDLEMAN ? RootType.Middleman : RootType.Output,
-          directories.getRelativeOutputPath(),
-          outputDirName,
-          name);
+          execRoot, RootType.Output, directories.getRelativeOutputPath(), outputDirName, name);
     }
   }
 
@@ -104,7 +99,6 @@ public ArtifactRoot getRoot(
   private final ArtifactRoot genfilesDirectory;
   private final ArtifactRoot coverageDirectory;
   private final ArtifactRoot testlogsDirectory;
-  private final ArtifactRoot middlemanDirectory;
 
   private final boolean mergeGenfilesDirectory;
 
@@ -127,8 +121,6 @@ public ArtifactRoot getRoot(
     this.genfilesDirectory = OutputDirectory.GENFILES.getRoot(mnemonic, directories, workspaceName);
     this.coverageDirectory = OutputDirectory.COVERAGE.getRoot(mnemonic, directories, workspaceName);
     this.testlogsDirectory = OutputDirectory.TESTLOGS.getRoot(mnemonic, directories, workspaceName);
-    this.middlemanDirectory =
-        OutputDirectory.MIDDLEMAN.getRoot(mnemonic, directories, workspaceName);
 
     this.mergeGenfilesDirectory = options.mergeGenfilesDirectory;
     this.siblingRepositoryLayout = siblingRepositoryLayout;
@@ -209,13 +201,6 @@ String getHostPathSeparator() {
     return OS.getCurrent() == OS.WINDOWS ? ";" : ":";
   }
 
-  /** Returns the internal directory (used for middlemen) for this build configuration. */
-  ArtifactRoot getMiddlemanDirectory(RepositoryName repositoryName) {
-    return siblingRepositoryLayout
-        ? buildDerivedRoot("internal", repositoryName, true)
-        : middlemanDirectory;
-  }
-
   String getMnemonic() {
     return mnemonic;
   }