Ignore convenience symlinks in glob expansion

It's slightly unfortunate that this require moving symlink-related
options to an options scope that applies to query (because query _also_
needs to know to ignore the symlinks when expanding globs).

It's also slightly unfortunate that the behaviour of the symlink-related
flags is really about _changing_ the existence of symlinks - the
existence of the "ignore" value is somewhat problematic as it means that
evaluation of globs may vary depending on the outcomes of previous
builds' options, but this better (and more niche) than the current state
which is that a first build and subsequent build with the _same_ options
may give different results (because the first build may produce files in
bazel-bin which are then picked up as inputs by the second build).

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -403,6 +403,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/packages:exec_group",
         "//src/main/java/com/google/devtools/build/lib/packages:package_specification",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
+        "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils",
         "//src/main/java/com/google/devtools/build/lib/profiler/memory:current_rule_tracker",

src/main/java/com/google/devtools/build/lib/analysis/config/SymlinkDefinition.java

@@ -15,8 +15,8 @@
 package com.google.devtools.build.lib.analysis.config;
 
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.pkgcache.PackageOptions;
 import com.google.devtools.build.lib.vfs.Path;
 import java.util.Set;
 import java.util.function.Function;
@@ -39,7 +39,7 @@ public interface SymlinkDefinition {
    * no-op, and more than one candidate means a warning about ambiguous symlink destinations should
    * be emitted.
    *
-   * @param buildRequestOptions options that may control which symlinks get created and what they
+   * @param packageOptions options that may control which symlinks get created and what they
    *     point to.
    * @param targetConfigs the configurations for which symlinks should be created. If these have
    *     conflicting requirements, multiple candidates are returned.
@@ -51,7 +51,7 @@ public interface SymlinkDefinition {
    * @param execRoot the exec root.
    */
   ImmutableSet<Path> getLinkPaths(
-      BuildRequestOptions buildRequestOptions,
+      PackageOptions packageOptions,
       Set<BuildConfigurationValue> targetConfigs,
       Function<BuildOptions, BuildConfigurationValue> configGetter,
       RepositoryName repositoryName,

src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java

@@ -220,41 +220,6 @@ public class BuildRequestOptions extends OptionsBase {
               + "to end users out of console output.")
   public List<String> hideAspectResults;
 
-  @Option(
-      name = "symlink_prefix",
-      defaultValue = "null",
-      documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
-      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
-      help =
-          "The prefix that is prepended to any of the convenience symlinks that are created "
-              + "after a build. If omitted, the default value is the name of the build tool "
-              + "followed by a hyphen. If '/' is passed, then no symlinks are created and no "
-              + "warning is emitted. Warning: the special functionality for '/' will be deprecated "
-              + "soon; use --experimental_convenience_symlinks=ignore instead.")
-  @Nullable
-  public String symlinkPrefix;
-
-  @Option(
-      name = "experimental_convenience_symlinks",
-      converter = ConvenienceSymlinksConverter.class,
-      defaultValue = "normal",
-      documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
-      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
-      help =
-          "This flag controls how the convenience symlinks (the symlinks that appear in the "
-              + "workspace after the build) will be managed. Possible values:\n"
-              + "  normal (default): Each kind of convenience symlink will be created or deleted, "
-              + "as determined by the build.\n"
-              + "  clean: All symlinks will be unconditionally deleted.\n"
-              + "  ignore: Symlinks will be left alone.\n"
-              + "  log_only: Generate log messages as if 'normal' were passed, but don't actually "
-              + "perform any filesystem operations (useful for tools).\n"
-              + "Note that only symlinks whose names are generated by the current value of "
-              + "--symlink_prefix can be affected; if the prefix changes, any pre-existing "
-              + "symlinks will be left alone.")
-  @Nullable
-  public ConvenienceSymlinksMode experimentalConvenienceSymlinks;
-
   @Option(
       name = "experimental_convenience_symlinks_bep_event",
       defaultValue = "false",
@@ -330,10 +295,6 @@ public class BuildRequestOptions extends OptionsBase {
 
   public BuildRequestOptions() throws OptionsParsingException {}
 
-  public String getSymlinkPrefix(String productName) {
-    return symlinkPrefix == null ? productName + "-" : symlinkPrefix;
-  }
-
   @Option(
       name = "use_action_cache",
       defaultValue = "true",
@@ -353,17 +314,6 @@ public String getSymlinkPrefix(String productName) {
       help = "Whether to use action rewinding to recover from lost inputs.")
   public boolean rewindLostInputs;
 
-  @Option(
-      name = "incompatible_skip_genfiles_symlink",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
-      effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE},
-      help =
-          "If set to true, the genfiles symlink will not be created. For more information, see "
-              + "https://github.com/bazelbuild/bazel/issues/8651")
-  public boolean incompatibleSkipGenfilesSymlink;
-
   @Option(
       name = "target_pattern_file",
       defaultValue = "",
@@ -478,31 +428,4 @@ public ProgressReportIntervalConverter() {
       super(0, 3600);
     }
   }
-
-  /**
-   * The {@link BoolOrEnumConverter} for the {@link ConvenienceSymlinksMode} where NORMAL is true
-   * and IGNORE is false.
-   */
-  public static class ConvenienceSymlinksConverter
-      extends BoolOrEnumConverter<ConvenienceSymlinksMode> {
-    public ConvenienceSymlinksConverter() {
-      super(
-          ConvenienceSymlinksMode.class,
-          "convenience symlinks mode",
-          ConvenienceSymlinksMode.NORMAL,
-          ConvenienceSymlinksMode.IGNORE);
-    }
-  }
-
-  /** Determines how the convenience symlinks are presented to the user */
-  enum ConvenienceSymlinksMode {
-    /** Will manage symlinks based on the symlink prefix. */
-    NORMAL,
-    /** Will clean up any existing symlinks. */
-    CLEAN,
-    /** Will not create or clean up any symlinks. */
-    IGNORE,
-    /** Will not create or clean up any symlinks, but will record the symlinks. */
-    LOG_ONLY
-  }
 }

src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java

@@ -92,7 +92,7 @@ private boolean outputTargets(
     String productName = runtime.getProductName();
     PathPrettyPrinter prettyPrinter =
         new PathPrettyPrinter(
-            request.getBuildOptions().getSymlinkPrefix(productName),
+            request.getPackageOptions().getSymlinkPrefix(productName),
             result.getConvenienceSymlinks());
     OutErr outErr = request.getOutErr();
 

src/main/java/com/google/devtools/build/lib/buildtool/ConfigSymlink.java

@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.SymlinkDefinition;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.pkgcache.PackageOptions;
 import com.google.devtools.build.lib.vfs.Path;
 import java.util.Set;
 import java.util.function.Function;
@@ -48,7 +49,7 @@ public String getLinkName(String symlinkPrefix, String workspaceBaseName) {
 
   @Override
   public ImmutableSet<Path> getLinkPaths(
-      BuildRequestOptions buildRequestOptions,
+      PackageOptions packageOptions,
       Set<BuildConfigurationValue> targetConfigs,
       Function<BuildOptions, BuildConfigurationValue> configGetter,
       RepositoryName repositoryName,

src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java

@@ -55,7 +55,6 @@
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.test.TestActionContext;
-import com.google.devtools.build.lib.buildtool.BuildRequestOptions.ConvenienceSymlinksMode;
 import com.google.devtools.build.lib.buildtool.buildevent.ConvenienceSymlinksIdentifiedEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.ExecutionProgressReceiverAvailableEvent;
@@ -76,6 +75,9 @@
 import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
 import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.pkgcache.LoadingOptions;
+import com.google.devtools.build.lib.pkgcache.PackageOptions;
+import com.google.devtools.build.lib.pkgcache.PackageOptions.ConvenienceSymlinksMode;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
 import com.google.devtools.build.lib.profiler.ProfilePhase;
@@ -754,10 +756,10 @@ public ImmutableMap<PathFragment, PathFragment> handleConvenienceSymlinks(
         Profiler.instance().profile("ExecutionTool.handleConvenienceSymlinks")) {
       OutputDirectoryLinksUtils.SymlinkCreationResult convenienceSymlinks =
           OutputDirectoryLinksUtils.EMPTY_SYMLINK_CREATION_RESULT;
-      if (request.getBuildOptions().experimentalConvenienceSymlinks
+      if (request.getPackageOptions().experimentalConvenienceSymlinks
           != ConvenienceSymlinksMode.IGNORE) {
         convenienceSymlinks =
-            createConvenienceSymlinks(request.getBuildOptions(), targetsToBuild, configuration);
+            createConvenienceSymlinks(request.getPackageOptions(), targetsToBuild, configuration);
       }
       if (request.getBuildOptions().experimentalConvenienceSymlinksBepEvent) {
         env.getEventBus()
@@ -786,7 +788,7 @@ public ImmutableMap<PathFragment, PathFragment> handleConvenienceSymlinks(
    * is to prevent confusion by pointing to an outdated directory the current build never used.
    */
   private OutputDirectoryLinksUtils.SymlinkCreationResult createConvenienceSymlinks(
-      BuildRequestOptions buildRequestOptions,
+      PackageOptions packageOptions,
       ImmutableSet<ConfiguredTarget> targetsToBuild,
       BuildConfigurationValue configuration) {
     SkyframeExecutor executor = env.getSkyframeExecutor();
@@ -832,7 +834,7 @@ private OutputDirectoryLinksUtils.SymlinkCreationResult createConvenienceSymlink
         Profiler.instance().profile("OutputDirectoryLinksUtils.createOutputDirectoryLinks")) {
       return OutputDirectoryLinksUtils.createOutputDirectoryLinks(
           runtime.getRuleClassProvider().getSymlinkDefinitions(),
-          buildRequestOptions,
+          packageOptions,
           env.getWorkspaceName(),
           env.getWorkspace(),
           env.getDirectories(),

src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java

@@ -26,10 +26,11 @@
 import com.google.devtools.build.lib.analysis.config.SymlinkDefinition;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.ConvenienceSymlink;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.ConvenienceSymlink.Action;
-import com.google.devtools.build.lib.buildtool.BuildRequestOptions.ConvenienceSymlinksMode;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.pkgcache.PackageOptions;
+import com.google.devtools.build.lib.pkgcache.PackageOptions.ConvenienceSymlinksMode;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -55,7 +56,7 @@ private OutputDirectoryLinksUtils() {}
    *
    * <p>The order of the result indicates precedence for {@link PathPrettyPrinter}.
    */
-  private static ImmutableList<SymlinkDefinition> getAllLinkDefinitions(
+  public static ImmutableList<SymlinkDefinition> getAllLinkDefinitions(
       Iterable<SymlinkDefinition> symlinkDefinitions) {
     ImmutableList.Builder<SymlinkDefinition> builder = ImmutableList.builder();
     builder.addAll(STANDARD_LINK_DEFINITIONS);
@@ -93,7 +94,7 @@ private static ImmutableList<SymlinkDefinition> getAllLinkDefinitions(
    */
   static SymlinkCreationResult createOutputDirectoryLinks(
       Iterable<SymlinkDefinition> symlinkDefinitions,
-      BuildRequestOptions buildRequestOptions,
+      PackageOptions packageOptions,
       String workspaceName,
       Path workspace,
       BlazeDirectories directories,
@@ -104,8 +105,8 @@ static SymlinkCreationResult createOutputDirectoryLinks(
     Path execRoot = directories.getExecRoot(workspaceName);
     Path outputPath = directories.getOutputPath(workspaceName);
     Path outputBase = directories.getOutputBase();
-    String symlinkPrefix = buildRequestOptions.getSymlinkPrefix(productName);
-    ConvenienceSymlinksMode mode = buildRequestOptions.experimentalConvenienceSymlinks;
+    String symlinkPrefix = packageOptions.getSymlinkPrefix(productName);
+    ConvenienceSymlinksMode mode = packageOptions.experimentalConvenienceSymlinks;
     if (NO_CREATE_SYMLINKS_PREFIX.equals(symlinkPrefix)) {
       return EMPTY_SYMLINK_CREATION_RESULT;
     }
@@ -131,7 +132,7 @@ static SymlinkCreationResult createOutputDirectoryLinks(
       } else {
         Set<Path> candidatePaths =
             symlink.getLinkPaths(
-                buildRequestOptions,
+                packageOptions,
                 targetConfigs,
                 configGetter,
                 repositoryName,
@@ -319,17 +320,17 @@ private static void removeLink(
           new ConfigSymlink("genfiles", BuildConfigurationValue::getGenfilesDirectory) {
             @Override
             public ImmutableSet<Path> getLinkPaths(
-                BuildRequestOptions buildRequestOptions,
+                PackageOptions packageOptions,
                 Set<BuildConfigurationValue> targetConfigs,
                 Function<BuildOptions, BuildConfigurationValue> configGetter,
                 RepositoryName repositoryName,
                 Path outputPath,
                 Path execRoot) {
-              if (buildRequestOptions.incompatibleSkipGenfilesSymlink) {
+              if (packageOptions.incompatibleSkipGenfilesSymlink) {
                 return ImmutableSet.of();
               }
               return super.getLinkPaths(
-                  buildRequestOptions,
+                  packageOptions,
                   targetConfigs,
                   configGetter,
                   repositoryName,
@@ -346,7 +347,7 @@ public String getLinkName(String symlinkPrefix, String workspaceBaseName) {
 
             @Override
             public ImmutableSet<Path> getLinkPaths(
-                BuildRequestOptions buildRequestOptions,
+                PackageOptions packageOptions,
                 Set<BuildConfigurationValue> targetConfigs,
                 Function<BuildOptions, BuildConfigurationValue> configGetter,
                 RepositoryName repositoryName,
@@ -364,7 +365,7 @@ public String getLinkName(String symlinkPrefix, String workspaceBaseName) {
 
             @Override
             public ImmutableSet<Path> getLinkPaths(
-                BuildRequestOptions buildRequestOptions,
+                PackageOptions packageOptions,
                 Set<BuildConfigurationValue> targetConfigs,
                 Function<BuildOptions, BuildConfigurationValue> configGetter,
                 RepositoryName repositoryName,