fix BUCK_CLASSPATH limit on linux (#2692)

Summary:
Pull Request resolved: #2692

When running `buck test ...` the RemoteExecution tests fails with **"Argument list too long"**.

Reason: The concatanation of paths used as BUCK_CLASSPATH is crossing the limit emposed by linux for the env variable values. (https://github.com/torvalds/linux/blob/master/include/uapi/linux/limits.h#L8)

How to reproduce the problem on a devserver:
- execute jshell
```
/usr/local/java-runtime/impl/11/bin/jshell
```
- paste the script that spawn a process with an env value
```
int test(String env, int size) throws Exception {
    var p = new ProcessBuilder("ls");
    p.environment().put(env, "b".repeat(size));
    return p.start().waitFor();
}

test("BUCK_CLASSPATH", 131_056)
// ok: 0

test("BUCK_CLASSPATH", 131_057)
// fail: 7 - Argument list too long
```

Reviewed By: bobyangyf

fbshipit-source-id: 82be3046d32894b9449bfec84c7cf1cc1e87f798

Author: Adolfo Santos

src/com/facebook/buck/cli/bootstrapper/ClassLoaderBootstrapper.java

@@ -16,11 +16,6 @@
 
 package com.facebook.buck.cli.bootstrapper;
 
-import java.io.File;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.nio.file.Paths;
 import java.util.Arrays;
 
 /**
@@ -39,7 +34,7 @@
  */
 public final class ClassLoaderBootstrapper {
 
-  private static final ClassLoader classLoader = createClassLoader();
+  private static final ClassLoader classLoader = ClassLoaderFactory.withEnv().create();
 
   private ClassLoaderBootstrapper() {}
 
@@ -60,25 +55,4 @@ public static Class<?> loadClass(String name) {
       throw new NoClassDefFoundError(name);
     }
   }
-
-  @SuppressWarnings("PMD.BlacklistedSystemGetenv")
-  private static ClassLoader createClassLoader() {
-    // BUCK_CLASSPATH is not set by a user, no need to use EnvVariablesProvider.
-    String classPath = System.getenv("BUCK_CLASSPATH");
-    if (classPath == null) {
-      throw new RuntimeException("BUCK_CLASSPATH not set");
-    }
-
-    String[] strings = classPath.split(File.pathSeparator);
-    URL[] urls = new URL[strings.length];
-    for (int i = 0; i < urls.length; i++) {
-      try {
-        urls[i] = Paths.get(strings[i]).toUri().toURL();
-      } catch (MalformedURLException e) {
-        throw new RuntimeException(e);
-      }
-    }
-
-    return new URLClassLoader(urls);
-  }
 }

src/com/facebook/buck/cli/bootstrapper/ClassLoaderFactory.java

@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.facebook.buck.cli.bootstrapper;
+
+import static java.util.Objects.requireNonNull;
+
+import java.io.File;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.Paths;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+/** Creates a ClassLoader from {@code System.getenv()}. classpath entries. */
+public class ClassLoaderFactory {
+
+  /** Expose a provider to facilitate mock tests. */
+  @FunctionalInterface
+  public interface ClassPathProvider extends Function<String, String> {}
+
+  static final String BUCK_CLASSPATH = "BUCK_CLASSPATH";
+  static final String EXTRA_BUCK_CLASSPATH = "EXTRA_BUCK_CLASSPATH";
+
+  private final ClassPathProvider classPathProvider;
+
+  @SuppressWarnings("PMD.BlacklistedSystemGetenv")
+  static ClassLoaderFactory withEnv() {
+    return new ClassLoaderFactory(System.getenv()::get);
+  }
+
+  ClassLoaderFactory(ClassPathProvider classPathProvider) {
+    this.classPathProvider = requireNonNull(classPathProvider);
+  }
+
+  /**
+   * Create a new ClassLoader that concats {@value BUCK_CLASSPATH} and {@value EXTRA_BUCK_CLASSPATH}
+   *
+   * @return ClassLoader instance to env classpath.
+   */
+  public ClassLoader create() {
+    // BUCK_CLASSPATH is not set by a user, no need to use EnvVariablesProvider.
+    String classPath = classPathProvider.apply(BUCK_CLASSPATH);
+    String extraClassPath = classPathProvider.apply(EXTRA_BUCK_CLASSPATH);
+
+    if (classPath == null) {
+      throw new RuntimeException(BUCK_CLASSPATH + " not set");
+    }
+
+    URL[] urls =
+        Stream.of(classPath, extraClassPath)
+            .flatMap(this::splitPaths)
+            .filter(this::nonBlank)
+            .map(this::toUrl)
+            .toArray(URL[]::new);
+
+    return new URLClassLoader(urls);
+  }
+
+  private Stream<String> splitPaths(String paths) {
+    if (paths == null) {
+      return Stream.empty();
+    }
+    return Stream.of(paths.split(File.pathSeparator));
+  }
+
+  private boolean nonBlank(String path) {
+    return !path.isBlank();
+  }
+
+  private URL toUrl(String path) {
+    try {
+      return Paths.get(path).toUri().toURL();
+    } catch (MalformedURLException e) {
+      throw new RuntimeException(e);
+    }
+  }
+}

src/com/facebook/buck/rules/modern/builders/ModernBuildRuleRemoteExecutionHelper.java

@@ -62,7 +62,6 @@
 import com.facebook.buck.util.function.ThrowingSupplier;
 import com.facebook.buck.util.hashing.FileHashLoader;
 import com.facebook.buck.util.types.Pair;
-import com.google.common.base.Joiner;
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -74,7 +73,6 @@
 import com.google.common.hash.HashFunction;
 import com.google.common.io.ByteStreams;
 import java.io.ByteArrayInputStream;
-import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -694,8 +692,8 @@ private ImmutableSortedMap<String, String> getBuilderEnvironmentOverrides(
     String relativePluginRoot = relativizePathString(cellPrefixRoot, PLUGIN_ROOT);
     String relativePluginResources = relativizePathString(cellPrefixRoot, PLUGIN_RESOURCES);
     return ImmutableSortedMap.<String, String>naturalOrder()
-        .put("CLASSPATH", classpathArg(bootstrapClasspath))
-        .put("BUCK_CLASSPATH", classpathArg(classpath))
+        .putAll(BuckClasspath.getClasspathEnv(classpath))
+        .putAll(BuckClasspath.getBootstrapClasspathEnv(bootstrapClasspath))
         .put(
             "EXTRA_JAVA_ARGS",
             ManagementFactory.getRuntimeMXBean().getInputArguments().stream()
@@ -870,8 +868,4 @@ private ThrowingSupplier<ClassPath, IOException> prepareClassPath(
         },
         IOException.class);
   }
-
-  private static String classpathArg(Iterable<Path> classpath) {
-    return Joiner.on(File.pathSeparator).join(classpath);
-  }
 }

src/com/facebook/buck/rules/modern/builders/trampoline.sh

@@ -33,9 +33,12 @@ function getJavaPathForVersion() {
 }
 
 export BUCK_CLASSPATH=$(resolveList $BUCK_CLASSPATH)
+BUCK_CLASSPATH_EXTRA=$(resolveList "$BUCK_CLASSPATH_EXTRA")
 export CLASSPATH=$(resolveList $CLASSPATH)
 export BUCK_ISOLATED_ROOT=$PWD
 
+export BUCK_CLASSPATH_EXTRA
+
 export BUCK_PLUGIN_RESOURCES=$(resolve $BUCK_PLUGIN_RESOURCES)
 export BUCK_PLUGIN_ROOT=$(resolve $BUCK_PLUGIN_ROOT)
 # necessary for "isolated buck" invocations to work correctly

src/com/facebook/buck/util/env/BuckClasspath.java

@@ -16,8 +16,10 @@
 
 package com.facebook.buck.util.env;
 
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Streams;
 import java.io.File;
 import java.io.IOException;
@@ -34,10 +36,17 @@
 /** Current process classpath. Set by buckd or by launcher script. */
 public class BuckClasspath {
 
+  /*
+   * Env values limit when spawning external process.
+   * https://github.com/torvalds/linux/blob/master/include/uapi/linux/limits.h#L8
+   */
+  public static final int ENV_ARG_MAX = 131000;
+
   public static final String BOOTSTRAP_MAIN_CLASS =
       "com.facebook.buck.cli.bootstrapper.ClassLoaderBootstrapper";
 
   public static final String ENV_VAR_NAME = "BUCK_CLASSPATH";
+  public static final String EXTRA_ENV_VAR_NAME = "EXTRA_BUCK_CLASSPATH";
   public static final String BOOTSTRAP_ENV_VAR_NAME = "CLASSPATH";
   public static final String TEST_ENV_VAR_NAME = "BUCK_TEST_CLASSPATH_FILE";
 
@@ -151,4 +160,32 @@ private static ImmutableList<Path> readClasspaths(Path classpathsFile) throws IO
         .map(Path::toAbsolutePath)
         .collect(ImmutableList.toImmutableList());
   }
+
+  /**
+   * Generate env map containing buck classpath. Will split the classpath into an EXTRA_* env value
+   * if it exceeds the linux limit.
+   *
+   * @param classpath to be exported.
+   * @return map containing env variables.
+   */
+  public static ImmutableMap<String, String> getClasspathEnv(Iterable<Path> classpath) {
+    String arg = asArgument(classpath);
+    int limit = arg.length() > ENV_ARG_MAX ? arg.lastIndexOf(File.pathSeparator, ENV_ARG_MAX) : -1;
+    if (limit > 0 && limit < arg.length()) {
+      return ImmutableMap.of(
+          ENV_VAR_NAME, arg.substring(0, limit),
+          EXTRA_ENV_VAR_NAME, arg.substring(limit + 1));
+    }
+    return ImmutableMap.of(ENV_VAR_NAME, arg);
+  }
+
+  /** Generate env map containing buck bootstrap classpath. */
+  public static ImmutableMap<String, String> getBootstrapClasspathEnv(
+      ImmutableList<Path> bootstrapClasspath) {
+    return ImmutableMap.of(BOOTSTRAP_ENV_VAR_NAME, asArgument(bootstrapClasspath));
+  }
+
+  public static String asArgument(Iterable<Path> classpath) {
+    return Joiner.on(File.pathSeparator).join(classpath);
+  }
 }

test/com/facebook/buck/cli/bootstrapper/ClassLoaderFactoryTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.facebook.buck.cli.bootstrapper;
+
+import static org.hamcrest.Matchers.arrayWithSize;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasItemInArray;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.junit.MatcherAssert.assertThat;
+import static org.hamcrest.object.HasToString.hasToString;
+import static org.junit.Assert.assertThrows;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ClassLoaderFactoryTest {
+
+  static final String CLASS_PATH_JAR = "/classPath.jar";
+  static final String EXTRA_CLASS_PATH_JAR = "/extraClassPath.jar";
+
+  private final Map<String, String> testEnvironment = new HashMap<>();
+  private final ClassLoaderFactory classLoaderFactory =
+      new ClassLoaderFactory(testEnvironment::get);
+
+  @Before
+  public void setUp() {
+    testEnvironment.clear();
+  }
+
+  @Test
+  public void testMissingBuckClassPlath() {
+    String expectedMessage = ClassLoaderFactory.BUCK_CLASSPATH + " not set";
+    assertThrows(expectedMessage, RuntimeException.class, classLoaderFactory::create);
+  }
+
+  @Test
+  public void testBuckClassPlath() {
+    testEnvironment.put(ClassLoaderFactory.BUCK_CLASSPATH, CLASS_PATH_JAR);
+
+    URL[] urls = ((URLClassLoader) classLoaderFactory.create()).getURLs();
+
+    assertThat(urls, arrayWithSize(1));
+    assertThat(urls, hasItemInArray(hasToString(containsString(CLASS_PATH_JAR))));
+    assertThat(urls, not(hasItemInArray(hasToString(containsString(EXTRA_CLASS_PATH_JAR)))));
+  }
+
+  @Test
+  public void testBuckClassPlathWithExtraClassPath() {
+    testEnvironment.put(ClassLoaderFactory.BUCK_CLASSPATH, CLASS_PATH_JAR);
+    testEnvironment.put(ClassLoaderFactory.EXTRA_BUCK_CLASSPATH, EXTRA_CLASS_PATH_JAR);
+
+    URL[] urls = ((URLClassLoader) classLoaderFactory.create()).getURLs();
+
+    assertThat(urls, arrayWithSize(2));
+    assertThat(urls, hasItemInArray(hasToString(containsString(CLASS_PATH_JAR))));
+    assertThat(urls, hasItemInArray(hasToString(containsString(EXTRA_CLASS_PATH_JAR))));
+  }
+}

test/com/facebook/buck/util/env/BuckClasspathTest.java

@@ -16,15 +16,25 @@
 
 package com.facebook.buck.util.env;
 
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.collection.IsMapContaining.hasEntry;
+import static org.hamcrest.collection.IsMapContaining.hasKey;
+import static org.hamcrest.core.StringContains.containsString;
 import static org.junit.Assume.assumeTrue;
 
 import com.facebook.buck.cli.bootstrapper.ClassLoaderBootstrapper;
 import com.facebook.buck.util.function.ThrowingFunction;
 import com.facebook.buck.util.stream.RichStream;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collections;
 import org.junit.Test;
 
 public class BuckClasspathTest {
@@ -62,6 +72,33 @@ public void testBootstrapClasspathDoesNotHaveAccessToBuckFiles() throws Exceptio
     classLoader.loadClass(ImmutableList.class.getName());
   }
 
+  @Test
+  public void testEnvHasBuckClasspath() {
+    String name = "testEnvHasBuckClasspath.jar";
+    ImmutableMap<String, String> env =
+        BuckClasspath.getClasspathEnv(Collections.singleton(Paths.get(name)));
+
+    assertThat(
+        env,
+        allOf(
+            hasEntry(is(BuckClasspath.ENV_VAR_NAME), containsString(name)),
+            not(hasKey(is(BuckClasspath.EXTRA_ENV_VAR_NAME)))));
+  }
+
+  @Test
+  public void testEnvHasBuckClasspathAndExtraClasspath() {
+    String name = "testEnvHasBuckClasspathAndExtraClasspath.jar";
+    ImmutableMap<String, String> env =
+        BuckClasspath.getClasspathEnv(
+            Collections.nCopies(BuckClasspath.ENV_ARG_MAX / name.length(), Paths.get(name)));
+
+    assertThat(
+        env,
+        allOf(
+            hasEntry(is(BuckClasspath.ENV_VAR_NAME), containsString(name)),
+            hasEntry(is(BuckClasspath.EXTRA_ENV_VAR_NAME), containsString(name))));
+  }
+
   public URLClassLoader createClassLoader(ImmutableList<Path> classpath) {
     URL[] urls =
         RichStream.from(classpath)

tools/ideabuck/src/com/facebook/buck/intellij/ideabuck/environment/EnvironmentFilter.java

@@ -36,6 +36,7 @@ public class EnvironmentFilter {
           "Apple_PubSub_Socket_Render", // OS X pubsub control variable.
           "BUCK_BUILD_ID", // Build ID passed in from Python.
           "BUCK_CLASSPATH", // Main classpath; set in Python
+          "BUCK_CLASSPATH_EXTRA", // Main classpath; set in Python
           "CHGHG", // Mercurial
           "CHGTIMEOUT", // Mercurial
           "CLASSPATH", // Bootstrap classpath; set in Python.