Back out "Reduce Rust compile rule path length"

Summary:
Original commit changeset: 8ef9cf0fdeef

Reverts D31888529.

It breaks some tests (T107229114) and makes compilation error messages less helpful: https://fb.workplace.com/groups/rust.language/posts/7389150347800183/

Rust 1.58 has support for long paths on Windows which should solve original issue.
rust-lang/rust#89174

It's currently in beta, stable release will happen on Jan 13: https://forge.rust-lang.org/
But I think we can use beta version for some time on Windows.

Reviewed By: jsgf

fbshipit-source-id: 9af1a5631ab56b8bb285da7cc3044eb8ba1bde85

src/com/facebook/buck/features/rust/RustCompileRule.java

@@ -54,7 +54,6 @@
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Maps;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Optional;
 
 /** Generate a rustc command line with all appropriate dependencies in place. */
@@ -91,12 +90,11 @@ protected RustCompileRule(
       ImmutableList<Arg> depArgs,
       ImmutableList<Arg> linkerArgs,
       ImmutableSortedMap<String, Arg> environment,
-      ImmutableSortedMap<SourcePath, String> mappedSources,
+      ImmutableSortedMap<SourcePath, Optional<String>> mappedSources,
       String rootModule,
       RemapSrcPaths remapSrcPaths,
       Optional<String> xcrunSdkPath,
       boolean withDownwardApi) {
-
     super(
         buildTarget,
         projectFilesystem,
@@ -130,7 +128,7 @@ public static RustCompileRule from(
       ImmutableList<Arg> depArgs,
       ImmutableList<Arg> linkerArgs,
       ImmutableSortedMap<String, Arg> environment,
-      ImmutableSortedMap<SourcePath, String> mappedSources,
+      ImmutableSortedMap<SourcePath, Optional<String>> mappedSources,
       String rootModule,
       RemapSrcPaths remapSrcPaths,
       Optional<String> xcrunSdkPath,
@@ -186,7 +184,7 @@ static class Impl implements Buildable {
     @AddToRuleKey private final String rootModule;
     @AddToRuleKey private final OutputPath output;
 
-    @AddToRuleKey private final ImmutableSortedMap<SourcePath, String> mappedSources;
+    @AddToRuleKey private final ImmutableSortedMap<SourcePath, Optional<String>> mappedSources;
 
     @AddToRuleKey private final RustBuckConfig.RemapSrcPaths remapSrcPaths;
 
@@ -208,7 +206,7 @@ public Impl(
         ImmutableSortedMap<String, Arg> environment,
         String rootModule,
         String outputName,
-        ImmutableSortedMap<SourcePath, String> mappedSources,
+        ImmutableSortedMap<SourcePath, Optional<String>> mappedSources,
         RemapSrcPaths remapSrcPaths,
         Optional<String> xcrunpath,
         boolean withDownwardApi) {
@@ -255,7 +253,20 @@ public ImmutableList<Step> getBuildSteps(
               mappedSources.entrySet().stream()
                   .collect(
                       ImmutableMap.toImmutableMap(
-                          ent -> Paths.get(ent.getValue()),
+                          ent -> {
+                            Path path;
+                            if (ent.getValue().isPresent()) {
+                              path =
+                                  buildTarget
+                                      .getCellRelativeBasePath()
+                                      .getPath()
+                                      .toPath(filesystem.getFileSystem())
+                                      .resolve(ent.getValue().get());
+                            } else {
+                              path = resolver.getCellUnsafeRelPath(ent.getKey()).getPath();
+                            }
+                            return path;
+                          },
                           ent -> resolver.getAbsolutePath(ent.getKey()).getPath()))));
       steps.addAll(
           CxxPrepareForLinkStep.create(
@@ -296,7 +307,6 @@ protected ImmutableList<String> getShellCommandInternal(
               ImmutableList<String> linkerCmd = linker.getCommandPrefix(resolver);
               ImmutableList.Builder<String> cmd = ImmutableList.builder();
               Path src = scratchDir.resolve(rootModule);
-
               cmd.addAll(compiler.getCommandPrefix(resolver));
               if (executionContext.getAnsi().isAnsiTerminal()) {
                 cmd.add("--color=always");

src/com/facebook/buck/features/rust/RustCompileUtils.java

@@ -82,7 +82,6 @@
 import com.google.common.hash.Hashing;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -112,7 +111,7 @@ public static RustCompileRule requireBuild(
       CrateType crateType,
       Optional<String> edition,
       LinkableDepType depType,
-      ImmutableSortedMap<SourcePath, String> mappedSources,
+      ImmutableSortedMap<SourcePath, Optional<String>> mappedSources,
       String rootModule,
       boolean forceRlib,
       boolean preferStatic,
@@ -246,7 +245,7 @@ private static RustCompileRule createBuild(
       Optional<String> edition,
       LinkableDepType depType,
       boolean rpath,
-      ImmutableSortedMap<SourcePath, String> mappedSources,
+      ImmutableSortedMap<SourcePath, Optional<String>> mappedSources,
       String rootModule,
       boolean forceRlib,
       boolean preferStatic,
@@ -642,7 +641,7 @@ public static BinaryWrapperRule createBinaryBuildRule(
 
     CxxPlatform cxxPlatform = rustPlatform.getCxxPlatform();
 
-    Pair<String, ImmutableSortedMap<SourcePath, String>> rootModuleAndSources =
+    Pair<String, ImmutableSortedMap<SourcePath, Optional<String>>> rootModuleAndSources =
         getRootModuleAndSources(
             projectFilesystem,
             buildTarget,
@@ -830,15 +829,16 @@ public static Optional<String> getCrateRoot(
       SourcePathResolverAdapter resolver,
       String crate,
       ImmutableSet<String> defaults,
-      Stream<String> sources) {
+      Stream<Path> sources) {
     String crateName = String.format("%s.rs", crate);
     ImmutableList<String> res =
         sources
             .filter(
                 src -> {
-                  String name = Paths.get(src).getFileName().toString();
+                  String name = src.getFileName().toString();
                   return defaults.contains(name) || name.equals(crateName);
                 })
+            .map(src -> src.toString())
             .collect(ImmutableList.toImmutableList());
 
     if (res.size() == 1) {
@@ -854,7 +854,7 @@ public static Optional<String> getCrateRoot(
    * module is what's passed to rustc as the input source file, and may not exist until the
    * symlinking phase happens.
    */
-  static Pair<String, ImmutableSortedMap<SourcePath, String>> getRootModuleAndSources(
+  static Pair<String, ImmutableSortedMap<SourcePath, Optional<String>>> getRootModuleAndSources(
       ProjectFilesystem projectFilesystem,
       BuildTarget target,
       ActionGraphBuilder graphBuilder,
@@ -865,64 +865,55 @@ static Pair<String, ImmutableSortedMap<SourcePath, String>> getRootModuleAndSour
       ImmutableSortedSet<SourcePath> srcs,
       ImmutableSortedMap<SourcePath, String> mappedSrcs) {
 
-    ImmutableSortedMap.Builder<SourcePath, String> fixedBuilder = ImmutableSortedMap.naturalOrder();
-
-    Path buildTargetPath =
-        target.getCellRelativeBasePath().getPath().toPath(projectFilesystem.getFileSystem());
-
-    SourcePathResolverAdapter resolver = graphBuilder.getSourcePathResolver();
+    ImmutableSortedMap.Builder<SourcePath, Optional<String>> fixedBuilder =
+        ImmutableSortedMap.naturalOrder();
 
     srcs.stream()
         .map(
             src ->
                 CxxGenruleDescription.fixupSourcePath(graphBuilder, cxxPlatform.getFlavor(), src))
-        .forEach(
-            src -> {
-              SourcePath srcPath =
-                  CxxGenruleDescription.fixupSourcePath(graphBuilder, cxxPlatform.getFlavor(), src);
-              Path candidatePath = resolver.getCellUnsafeRelPath(src).getPath();
-
-              // These are files with no user-defined virtual path, they are either
-              // generated (ie with a genrule) or references files in the source
-              // tree
-
-              // If the path references files in the source tree
-              if (candidatePath.startsWith(buildTargetPath)) {
-                // We relativize them to reduce path amplification - mostly for
-                // windows's sake
-                fixedBuilder.put(srcPath, buildTargetPath.relativize(candidatePath).toString());
-              } else {
-
-                // These are generated files, since these are generated paths with hashes, they are
-                // rewritable.
-                fixedBuilder.put(
-                    srcPath, resolver.getAbsolutePath(srcPath).getPath().getFileName().toString());
-              }
-            });
+        .forEach(src -> fixedBuilder.put(src, Optional.empty()));
     mappedSrcs
         .entrySet()
         .forEach(
-            ent -> {
-              SourcePath sourcePath =
-                  CxxGenruleDescription.fixupSourcePath(
-                      graphBuilder, cxxPlatform.getFlavor(), ent.getKey());
-              Path virtualPath = Paths.get(ent.getValue()).normalize();
-              // These are rs files with user-defined virtual path, usually these are
-              // the from mapped_srcs
-              // Ban remappings that contain paths that go outside the base path
-              if (virtualPath.startsWith("..")) {
-                throw new HumanReadableException(
-                    "Mapped sources must be forward paths ie. no ../ ");
-              }
-              fixedBuilder.put(sourcePath, virtualPath.toString());
-            });
-
-    ImmutableSortedMap<SourcePath, String> fixed = fixedBuilder.build();
+            ent ->
+                fixedBuilder.put(
+                    CxxGenruleDescription.fixupSourcePath(
+                        graphBuilder, cxxPlatform.getFlavor(), ent.getKey()),
+                    Optional.of(ent.getValue())));
+
+    ImmutableSortedMap<SourcePath, Optional<String>> fixed = fixedBuilder.build();
+
+    SourcePathResolverAdapter resolver = graphBuilder.getSourcePathResolver();
+    Stream<Path> filenames =
+        Stream.concat(
+            srcs.stream()
+                .map(
+                    src ->
+                        CxxGenruleDescription.fixupSourcePath(
+                            graphBuilder, cxxPlatform.getFlavor(), src))
+                .map(sp -> resolver.getCellUnsafeRelPath(sp).getPath()),
+            mappedSrcs.values().stream()
+                .map(
+                    path ->
+                        target
+                            .getCellRelativeBasePath()
+                            .getPath()
+                            .toPath(projectFilesystem.getFileSystem())
+                            .resolve(path)));
 
     Optional<String> rootModule =
         crateRoot
+            .map(
+                name ->
+                    target
+                        .getCellRelativeBasePath()
+                        .getPath()
+                        .toPath(projectFilesystem.getFileSystem())
+                        .resolve(name)
+                        .toString())
             .map(Optional::of)
-            .orElseGet(() -> getCrateRoot(resolver, crate, defaultRoots, fixed.values().stream()));
+            .orElseGet(() -> getCrateRoot(resolver, crate, defaultRoots, filenames));
 
     return new Pair<>(
         rootModule.orElseThrow(

src/com/facebook/buck/features/rust/RustLibraryDescription.java

@@ -115,7 +115,7 @@ private RustCompileRule requireLibraryBuild(
       RustLibraryDescriptionArg args,
       Iterable<BuildRule> deps,
       ImmutableMap<String, BuildTarget> depsAliases) {
-    Pair<String, ImmutableSortedMap<SourcePath, String>> rootModuleAndSources =
+    Pair<String, ImmutableSortedMap<SourcePath, Optional<String>>> rootModuleAndSources =
         RustCompileUtils.getRootModuleAndSources(
             projectFilesystem,
             buildTarget,

test/com/facebook/buck/features/rust/RustCompileTest.java

@@ -289,7 +289,7 @@ private FakeRustCompileRule(
           srcs.stream()
               .collect(
                   ImmutableSortedMap.toImmutableSortedMap(
-                      Comparator.naturalOrder(), src -> src, src -> "random")),
+                      Comparator.naturalOrder(), src -> src, src -> Optional.empty())),
           rootModule,
           RustBuckConfig.RemapSrcPaths.NO,
           Optional.empty(),
@@ -308,12 +308,7 @@ static FakeRustCompileRule from(String target, ImmutableSortedSet<SourcePath> sr
               ImmutableSet.of("main.rs", "lib.rs"),
               srcs.stream()
                   .map(
-                      sp ->
-                          ruleFinder
-                              .getSourcePathResolver()
-                              .getCellUnsafeRelPath(sp)
-                              .getPath()
-                              .toString()));
+                      sp -> ruleFinder.getSourcePathResolver().getCellUnsafeRelPath(sp).getPath()));
 
       if (!root.isPresent()) {
         throw new HumanReadableException("No crate root source identified");