Make --symlink_prefix= work as expected again (i.e. create links called "bin"/"genfiles" instead of the default ones)

--
MOS_MIGRATED_REVID=108699595

5 files changed, 12 insertions, 32 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index 7298301494..c0f82e5ffe 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -33,13 +33,11 @@ import com.google.devtools.build.lib.runtime.BlazeCommandEventHandler;
 import com.google.devtools.build.lib.util.OptionsUtils;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.Converters.RangeConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsClassProvider;
-import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.OptionsProvider;
 
 import java.util.ArrayList;
@@ -57,24 +55,6 @@ import java.util.regex.Pattern;
  */
 public class BuildRequest implements OptionsClassProvider {
   /**
-   * A converter for symlink prefixes that defaults to {@code Constants.PRODUCT_NAME} and a
-   * minus sign if the option is not given.
-   *
-   * <p>Required because you cannot specify a non-constant value in annotation attributes.
-   */
-  public static class SymlinkPrefixConverter implements Converter<String> {
-    @Override
-    public String convert(String input) throws OptionsParsingException {
-      return input.isEmpty() ? Constants.PRODUCT_NAME + "-" : input;
-    }
-
-    @Override
-    public String getTypeDescription() {
-      return "a string";
-    }
-  }
-
-  /**
    * Options interface--can be used to parse command-line arguments.
    *
    * <p>See also ExecutionOptions; from the user's point of view, there's no
@@ -248,12 +228,11 @@ public class BuildRequest implements OptionsClassProvider {
     public boolean announce;
 
     @Option(name = "symlink_prefix",
-        defaultValue = "",
-        converter = SymlinkPrefixConverter.class,
+        defaultValue = "null",
         category = "misc",
         help = "The prefix that is prepended to any of the convenience symlinks that are created "
             + "after a build. If '/' is passed, then no symlinks are created and no warning is "
-            + "emitted. If omitted or is empty, the default value is the name of the build tool."
+            + "emitted. If omitted, the default value is the name of the build tool."
         )
     public String symlinkPrefix;
 
@@ -289,6 +268,10 @@ public class BuildRequest implements OptionsClassProvider {
       help = "List of top-level aspects"
     )
     public List<String> aspects;
+
+    public String getSymlinkPrefix() {
+      return symlinkPrefix == null ? Constants.PRODUCT_NAME + "-" : symlinkPrefix;
+    }
   }
 
   /**
@@ -552,10 +535,6 @@ public class BuildRequest implements OptionsClassProvider {
     return ImmutableSortedSet.copyOf(current);
   }
 
-  public String getSymlinkPrefix() {
-    return getBuildOptions().symlinkPrefix;
-  }
-
   public ImmutableSortedSet<String> getMultiCpus() {
     return ImmutableSortedSet.copyOf(getBuildOptions().multiCpus);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index aaeb7cddac..f99010f582 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -140,7 +140,7 @@ class BuildResultPrinter {
                 + OutputDirectoryLinksUtils.getPrettyPath(temp.getPath(),
                 runtime.getWorkspaceName(),
                 runtime.getWorkspace(),
-                request.getSymlinkPrefix()));
+                request.getBuildOptions().getSymlinkPrefix()));
           }
         }
       }
@@ -156,7 +156,8 @@ class BuildResultPrinter {
 
   private String formatArtifactForShowResults(Artifact artifact, BuildRequest request) {
     return "  " + OutputDirectoryLinksUtils.getPrettyPath(artifact.getPath(),
-        runtime.getWorkspaceName(), runtime.getWorkspace(), request.getSymlinkPrefix());
+        runtime.getWorkspaceName(), runtime.getWorkspace(),
+        request.getBuildOptions().getSymlinkPrefix());
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 659c90619f..b50317b1a5 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -353,7 +353,7 @@ public class ExecutionTool {
         OutputDirectoryLinksUtils.createOutputDirectoryLinks(
             runtime.getWorkspaceName(), getWorkspace(), getExecRoot(),
             runtime.getOutputPath(), getReporter(), targetConfigurations.get(0),
-            request.getSymlinkPrefix());
+            request.getBuildOptions().getSymlinkPrefix());
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
index 63b7174cf9..8268d38c33 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
@@ -106,7 +106,7 @@ public final class CleanCommand implements BlazeCommand {
     env.getReporter().handle(Event.info(null/*location*/, cleanBanner));
     try {
       String symlinkPrefix =
-          options.getOptions(BuildRequest.BuildRequestOptions.class).symlinkPrefix;
+          options.getOptions(BuildRequest.BuildRequestOptions.class).getSymlinkPrefix();
       actuallyClean(env, runtime.getOutputBase(), cleanOptions, symlinkPrefix);
       return ExitCode.SUCCESS;
     } catch (IOException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index f2aaf449bf..2328d494b5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -243,7 +243,7 @@ public class RunCommand implements BlazeCommand  {
     PathFragment prettyExecutablePath =
         OutputDirectoryLinksUtils.getPrettyPath(executablePath,
             runtime.getWorkspaceName(), runtime.getWorkspace(),
-            options.getOptions(BuildRequestOptions.class).symlinkPrefix);
+            options.getOptions(BuildRequestOptions.class).getSymlinkPrefix());
     List<String> cmdLine = new ArrayList<>();
     if (runOptions.scriptPath == null) {
       cmdLine.add(runtime.getDirectories().getExecRoot()