From 4373197059117c8a91788ab14f4e9a415c843c4a Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Mon, 18 Jul 2022 15:47:49 +0200 Subject: [PATCH 1/2] Add CommandLinePathFactory to CommandEnvironment Progress on #15856 --- .../build/lib/runtime/CommandEnvironment.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index a3830c527e4f16..977da98b0604e5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -107,6 +107,7 @@ public class CommandEnvironment { private final ImmutableList.Builder responseExtensions = ImmutableList.builder(); private final Consumer shutdownReasonConsumer; private final BuildResultListener buildResultListener; + private final CommandLinePathFactory commandLinePathFactory; private OutputService outputService; private String workspaceName; @@ -273,6 +274,23 @@ public void exit(AbruptExitException exception) { } this.buildResultListener = new BuildResultListener(); this.eventBus.register(this.buildResultListener); + + ImmutableMap.Builder wellKnownRoots = ImmutableMap.builder(); + // This is necessary because some tests don't have a workspace set. + putIfValueNotNull(wellKnownRoots, "workspace", directories.getWorkspace()); + + this.commandLinePathFactory = + new CommandLinePathFactory(runtime.getFileSystem(), wellKnownRoots.build()); + } + + private static void putIfValueNotNull( + ImmutableMap.Builder map, K key, @Nullable V value) { + Preconditions.checkNotNull(map); + Preconditions.checkNotNull(key); + + if (value != null) { + map.put(key, value); + } } private Path computeWorkingDirectory(CommonCommandOptions commandOptions) @@ -840,4 +858,8 @@ public void addResponseExtensions(Iterable extensions) { public BuildResultListener getBuildResultListener() { return buildResultListener; } + + public CommandLinePathFactory getCommandLinePathFactory() { + return commandLinePathFactory; + } } From a4aa22d123f40aa653d9db9a0806eede91bfbe5e Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Mon, 18 Jul 2022 19:56:23 +0200 Subject: [PATCH 2/2] use factory method --- .../build/lib/runtime/CommandEnvironment.java | 16 +-------------- .../lib/runtime/CommandLinePathFactory.java | 20 ++++++++++++++++++- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 977da98b0604e5..7bd7feaa65159e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -275,22 +275,8 @@ public void exit(AbruptExitException exception) { this.buildResultListener = new BuildResultListener(); this.eventBus.register(this.buildResultListener); - ImmutableMap.Builder wellKnownRoots = ImmutableMap.builder(); - // This is necessary because some tests don't have a workspace set. - putIfValueNotNull(wellKnownRoots, "workspace", directories.getWorkspace()); - this.commandLinePathFactory = - new CommandLinePathFactory(runtime.getFileSystem(), wellKnownRoots.build()); - } - - private static void putIfValueNotNull( - ImmutableMap.Builder map, K key, @Nullable V value) { - Preconditions.checkNotNull(map); - Preconditions.checkNotNull(key); - - if (value != null) { - map.put(key, value); - } + CommandLinePathFactory.create(runtime.getFileSystem(), directories); } private Path computeWorkingDirectory(CommonCommandOptions commandOptions) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java index e39bf074b2d91c..6f80777660fd2e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java @@ -13,10 +13,12 @@ // limitations under the License. package com.google.devtools.build.lib.runtime; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -41,10 +43,26 @@ public final class CommandLinePathFactory { private static final Splitter PATH_SPLITTER = Splitter.on(File.pathSeparator); + static CommandLinePathFactory create(FileSystem fileSystem, BlazeDirectories directories) { + Preconditions.checkNotNull(fileSystem); + Preconditions.checkNotNull(directories); + + ImmutableMap.Builder wellKnownRoots = ImmutableMap.builder(); + + // This is necessary because some tests don't have a workspace set. + Path workspace = directories.getWorkspace(); + if (workspace != null) { + wellKnownRoots.put("workspace", workspace); + } + + return new CommandLinePathFactory(fileSystem, wellKnownRoots.build()); + } + private final FileSystem fileSystem; private final ImmutableMap roots; - public CommandLinePathFactory(FileSystem fileSystem, ImmutableMap roots) { + @VisibleForTesting + CommandLinePathFactory(FileSystem fileSystem, ImmutableMap roots) { this.fileSystem = Preconditions.checkNotNull(fileSystem); this.roots = Preconditions.checkNotNull(roots); }