Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

Commit 4b1712a

Browse files
Adolfo Santosfacebook-github-bot
Adolfo Santos
authored andcommitted
fix BUCK_CLASSPATH limit on linux
Summary: 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: 2a38ef676ca60ea124f08db0767290c7df307783
1 parent 2c6a6b9 commit 4b1712a

File tree

8 files changed

+249
-35
lines changed

8 files changed

+249
-35
lines changed

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

+1-27
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@
1616

1717
package com.facebook.buck.cli.bootstrapper;
1818

19-
import java.io.File;
20-
import java.net.MalformedURLException;
21-
import java.net.URL;
22-
import java.net.URLClassLoader;
23-
import java.nio.file.Paths;
2419
import java.util.Arrays;
2520

2621
/**
@@ -39,7 +34,7 @@
3934
*/
4035
public final class ClassLoaderBootstrapper {
4136

42-
private static final ClassLoader classLoader = createClassLoader();
37+
private static final ClassLoader classLoader = ClassLoaderFactory.withEnv().create();
4338

4439
private ClassLoaderBootstrapper() {}
4540

@@ -60,25 +55,4 @@ public static Class<?> loadClass(String name) {
6055
throw new NoClassDefFoundError(name);
6156
}
6257
}
63-
64-
@SuppressWarnings("PMD.BlacklistedSystemGetenv")
65-
private static ClassLoader createClassLoader() {
66-
// BUCK_CLASSPATH is not set by a user, no need to use EnvVariablesProvider.
67-
String classPath = System.getenv("BUCK_CLASSPATH");
68-
if (classPath == null) {
69-
throw new RuntimeException("BUCK_CLASSPATH not set");
70-
}
71-
72-
String[] strings = classPath.split(File.pathSeparator);
73-
URL[] urls = new URL[strings.length];
74-
for (int i = 0; i < urls.length; i++) {
75-
try {
76-
urls[i] = Paths.get(strings[i]).toUri().toURL();
77-
} catch (MalformedURLException e) {
78-
throw new RuntimeException(e);
79-
}
80-
}
81-
82-
return new URLClassLoader(urls);
83-
}
8458
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.facebook.buck.cli.bootstrapper;
18+
19+
import static java.util.Objects.requireNonNull;
20+
21+
import java.io.File;
22+
import java.net.MalformedURLException;
23+
import java.net.URL;
24+
import java.net.URLClassLoader;
25+
import java.nio.file.Paths;
26+
import java.util.function.Function;
27+
import java.util.stream.Stream;
28+
29+
/** Creates a ClassLoader from {@code System.getenv()}. classpath entries. */
30+
public class ClassLoaderFactory {
31+
32+
/** Expose a provider to facilitate mock tests. */
33+
@FunctionalInterface
34+
public interface ClassPathProvider extends Function<String, String> {}
35+
36+
static final String BUCK_CLASSPATH = "BUCK_CLASSPATH";
37+
static final String EXTRA_BUCK_CLASSPATH = "EXTRA_BUCK_CLASSPATH";
38+
39+
private final ClassPathProvider classPathProvider;
40+
41+
@SuppressWarnings("PMD.BlacklistedSystemGetenv")
42+
static ClassLoaderFactory withEnv() {
43+
return new ClassLoaderFactory(System.getenv()::get);
44+
}
45+
46+
ClassLoaderFactory(ClassPathProvider classPathProvider) {
47+
this.classPathProvider = requireNonNull(classPathProvider);
48+
}
49+
50+
/**
51+
* Create a new ClassLoader that concats {@value BUCK_CLASSPATH} and {@value EXTRA_BUCK_CLASSPATH}
52+
*
53+
* @return ClassLoader instance to env classpath.
54+
*/
55+
public ClassLoader create() {
56+
// BUCK_CLASSPATH is not set by a user, no need to use EnvVariablesProvider.
57+
String classPath = classPathProvider.apply(BUCK_CLASSPATH);
58+
String extraClassPath = classPathProvider.apply(EXTRA_BUCK_CLASSPATH);
59+
60+
if (classPath == null) {
61+
throw new RuntimeException(BUCK_CLASSPATH + " not set");
62+
}
63+
64+
URL[] urls =
65+
Stream.of(classPath, extraClassPath)
66+
.flatMap(this::splitPaths)
67+
.filter(this::nonBlank)
68+
.map(this::toUrl)
69+
.toArray(URL[]::new);
70+
71+
return new URLClassLoader(urls);
72+
}
73+
74+
private Stream<String> splitPaths(String paths) {
75+
if (paths == null) {
76+
return Stream.empty();
77+
}
78+
return Stream.of(paths.split(File.pathSeparator));
79+
}
80+
81+
private boolean nonBlank(String path) {
82+
return !path.isBlank();
83+
}
84+
85+
private URL toUrl(String path) {
86+
try {
87+
return Paths.get(path).toUri().toURL();
88+
} catch (MalformedURLException e) {
89+
throw new RuntimeException(e);
90+
}
91+
}
92+
}

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

+2-8
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
import com.facebook.buck.util.function.ThrowingSupplier;
6363
import com.facebook.buck.util.hashing.FileHashLoader;
6464
import com.facebook.buck.util.types.Pair;
65-
import com.google.common.base.Joiner;
6665
import com.google.common.base.Verify;
6766
import com.google.common.collect.ImmutableList;
6867
import com.google.common.collect.ImmutableMap;
@@ -74,7 +73,6 @@
7473
import com.google.common.hash.HashFunction;
7574
import com.google.common.io.ByteStreams;
7675
import java.io.ByteArrayInputStream;
77-
import java.io.File;
7876
import java.io.FileInputStream;
7977
import java.io.IOException;
8078
import java.io.InputStream;
@@ -694,8 +692,8 @@ private ImmutableSortedMap<String, String> getBuilderEnvironmentOverrides(
694692
String relativePluginRoot = relativizePathString(cellPrefixRoot, PLUGIN_ROOT);
695693
String relativePluginResources = relativizePathString(cellPrefixRoot, PLUGIN_RESOURCES);
696694
return ImmutableSortedMap.<String, String>naturalOrder()
697-
.put("CLASSPATH", classpathArg(bootstrapClasspath))
698-
.put("BUCK_CLASSPATH", classpathArg(classpath))
695+
.putAll(BuckClasspath.getClasspathEnv(classpath))
696+
.putAll(BuckClasspath.getBootstrapClasspathEnv(bootstrapClasspath))
699697
.put(
700698
"EXTRA_JAVA_ARGS",
701699
ManagementFactory.getRuntimeMXBean().getInputArguments().stream()
@@ -870,8 +868,4 @@ private ThrowingSupplier<ClassPath, IOException> prepareClassPath(
870868
},
871869
IOException.class);
872870
}
873-
874-
private static String classpathArg(Iterable<Path> classpath) {
875-
return Joiner.on(File.pathSeparator).join(classpath);
876-
}
877871
}

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

+3
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,12 @@ function getJavaPathForVersion() {
3333
}
3434

3535
export BUCK_CLASSPATH=$(resolveList $BUCK_CLASSPATH)
36+
BUCK_CLASSPATH_EXTRA=$(resolveList "$BUCK_CLASSPATH_EXTRA")
3637
export CLASSPATH=$(resolveList $CLASSPATH)
3738
export BUCK_ISOLATED_ROOT=$PWD
3839

40+
export BUCK_CLASSPATH_EXTRA
41+
3942
export BUCK_PLUGIN_RESOURCES=$(resolve $BUCK_PLUGIN_RESOURCES)
4043
export BUCK_PLUGIN_ROOT=$(resolve $BUCK_PLUGIN_ROOT)
4144
# necessary for "isolated buck" invocations to work correctly

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

+37
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
package com.facebook.buck.util.env;
1818

19+
import com.google.common.base.Joiner;
1920
import com.google.common.base.Preconditions;
2021
import com.google.common.collect.ImmutableList;
22+
import com.google.common.collect.ImmutableMap;
2123
import com.google.common.collect.Streams;
2224
import java.io.File;
2325
import java.io.IOException;
@@ -34,10 +36,17 @@
3436
/** Current process classpath. Set by buckd or by launcher script. */
3537
public class BuckClasspath {
3638

39+
/*
40+
* Env values limit when spawning external process.
41+
* https://github.com/torvalds/linux/blob/master/include/uapi/linux/limits.h#L8
42+
*/
43+
public static final int ENV_ARG_MAX = 131000;
44+
3745
public static final String BOOTSTRAP_MAIN_CLASS =
3846
"com.facebook.buck.cli.bootstrapper.ClassLoaderBootstrapper";
3947

4048
public static final String ENV_VAR_NAME = "BUCK_CLASSPATH";
49+
public static final String EXTRA_ENV_VAR_NAME = "EXTRA_BUCK_CLASSPATH";
4150
public static final String BOOTSTRAP_ENV_VAR_NAME = "CLASSPATH";
4251
public static final String TEST_ENV_VAR_NAME = "BUCK_TEST_CLASSPATH_FILE";
4352

@@ -151,4 +160,32 @@ private static ImmutableList<Path> readClasspaths(Path classpathsFile) throws IO
151160
.map(Path::toAbsolutePath)
152161
.collect(ImmutableList.toImmutableList());
153162
}
163+
164+
/**
165+
* Generate env map containing buck classpath. Will split the classpath into an EXTRA_* env value
166+
* if it exceeds the linux limit.
167+
*
168+
* @param classpath to be exported.
169+
* @return map containing env variables.
170+
*/
171+
public static ImmutableMap<String, String> getClasspathEnv(Iterable<Path> classpath) {
172+
String arg = asArgument(classpath);
173+
int limit = arg.length() > ENV_ARG_MAX ? arg.lastIndexOf(File.pathSeparator, ENV_ARG_MAX) : -1;
174+
if (limit > 0 && limit < arg.length()) {
175+
return ImmutableMap.of(
176+
ENV_VAR_NAME, arg.substring(0, limit),
177+
EXTRA_ENV_VAR_NAME, arg.substring(limit + 1));
178+
}
179+
return ImmutableMap.of(ENV_VAR_NAME, arg);
180+
}
181+
182+
/** Generate env map containing buck bootstrap classpath. */
183+
public static ImmutableMap<String, String> getBootstrapClasspathEnv(
184+
ImmutableList<Path> bootstrapClasspath) {
185+
return ImmutableMap.of(BOOTSTRAP_ENV_VAR_NAME, asArgument(bootstrapClasspath));
186+
}
187+
188+
public static String asArgument(Iterable<Path> classpath) {
189+
return Joiner.on(File.pathSeparator).join(classpath);
190+
}
154191
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.facebook.buck.cli.bootstrapper;
18+
19+
import static org.hamcrest.Matchers.arrayWithSize;
20+
import static org.hamcrest.Matchers.containsString;
21+
import static org.hamcrest.Matchers.hasItemInArray;
22+
import static org.hamcrest.Matchers.not;
23+
import static org.hamcrest.junit.MatcherAssert.assertThat;
24+
import static org.hamcrest.object.HasToString.hasToString;
25+
import static org.junit.Assert.assertThrows;
26+
27+
import java.net.URL;
28+
import java.net.URLClassLoader;
29+
import java.util.HashMap;
30+
import java.util.Map;
31+
import org.junit.Before;
32+
import org.junit.Test;
33+
34+
public class ClassLoaderFactoryTest {
35+
36+
static final String CLASS_PATH_JAR = "/classPath.jar";
37+
static final String EXTRA_CLASS_PATH_JAR = "/extraClassPath.jar";
38+
39+
private final Map<String, String> testEnvironment = new HashMap<>();
40+
private final ClassLoaderFactory classLoaderFactory =
41+
new ClassLoaderFactory(testEnvironment::get);
42+
43+
@Before
44+
public void setUp() {
45+
testEnvironment.clear();
46+
}
47+
48+
@Test
49+
public void testMissingBuckClassPlath() {
50+
String expectedMessage = ClassLoaderFactory.BUCK_CLASSPATH + " not set";
51+
assertThrows(expectedMessage, RuntimeException.class, classLoaderFactory::create);
52+
}
53+
54+
@Test
55+
public void testBuckClassPlath() {
56+
testEnvironment.put(ClassLoaderFactory.BUCK_CLASSPATH, CLASS_PATH_JAR);
57+
58+
URL[] urls = ((URLClassLoader) classLoaderFactory.create()).getURLs();
59+
60+
assertThat(urls, arrayWithSize(1));
61+
assertThat(urls, hasItemInArray(hasToString(containsString(CLASS_PATH_JAR))));
62+
assertThat(urls, not(hasItemInArray(hasToString(containsString(EXTRA_CLASS_PATH_JAR)))));
63+
}
64+
65+
@Test
66+
public void testBuckClassPlathWithExtraClassPath() {
67+
testEnvironment.put(ClassLoaderFactory.BUCK_CLASSPATH, CLASS_PATH_JAR);
68+
testEnvironment.put(ClassLoaderFactory.EXTRA_BUCK_CLASSPATH, EXTRA_CLASS_PATH_JAR);
69+
70+
URL[] urls = ((URLClassLoader) classLoaderFactory.create()).getURLs();
71+
72+
assertThat(urls, arrayWithSize(2));
73+
assertThat(urls, hasItemInArray(hasToString(containsString(CLASS_PATH_JAR))));
74+
assertThat(urls, hasItemInArray(hasToString(containsString(EXTRA_CLASS_PATH_JAR))));
75+
}
76+
}

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

+37
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,25 @@
1616

1717
package com.facebook.buck.util.env;
1818

19+
import static org.hamcrest.CoreMatchers.allOf;
20+
import static org.hamcrest.CoreMatchers.is;
21+
import static org.hamcrest.CoreMatchers.not;
22+
import static org.hamcrest.MatcherAssert.assertThat;
23+
import static org.hamcrest.collection.IsMapContaining.hasEntry;
24+
import static org.hamcrest.collection.IsMapContaining.hasKey;
25+
import static org.hamcrest.core.StringContains.containsString;
1926
import static org.junit.Assume.assumeTrue;
2027

2128
import com.facebook.buck.cli.bootstrapper.ClassLoaderBootstrapper;
2229
import com.facebook.buck.util.function.ThrowingFunction;
2330
import com.facebook.buck.util.stream.RichStream;
2431
import com.google.common.collect.ImmutableList;
32+
import com.google.common.collect.ImmutableMap;
2533
import java.net.URL;
2634
import java.net.URLClassLoader;
2735
import java.nio.file.Path;
36+
import java.nio.file.Paths;
37+
import java.util.Collections;
2838
import org.junit.Test;
2939

3040
public class BuckClasspathTest {
@@ -62,6 +72,33 @@ public void testBootstrapClasspathDoesNotHaveAccessToBuckFiles() throws Exceptio
6272
classLoader.loadClass(ImmutableList.class.getName());
6373
}
6474

75+
@Test
76+
public void testEnvHasBuckClasspath() {
77+
String name = "testEnvHasBuckClasspath.jar";
78+
ImmutableMap<String, String> env =
79+
BuckClasspath.getClasspathEnv(Collections.singleton(Paths.get(name)));
80+
81+
assertThat(
82+
env,
83+
allOf(
84+
hasEntry(is(BuckClasspath.ENV_VAR_NAME), containsString(name)),
85+
not(hasKey(is(BuckClasspath.EXTRA_ENV_VAR_NAME)))));
86+
}
87+
88+
@Test
89+
public void testEnvHasBuckClasspathAndExtraClasspath() {
90+
String name = "testEnvHasBuckClasspathAndExtraClasspath.jar";
91+
ImmutableMap<String, String> env =
92+
BuckClasspath.getClasspathEnv(
93+
Collections.nCopies(BuckClasspath.ENV_ARG_MAX / name.length(), Paths.get(name)));
94+
95+
assertThat(
96+
env,
97+
allOf(
98+
hasEntry(is(BuckClasspath.ENV_VAR_NAME), containsString(name)),
99+
hasEntry(is(BuckClasspath.EXTRA_ENV_VAR_NAME), containsString(name))));
100+
}
101+
65102
public URLClassLoader createClassLoader(ImmutableList<Path> classpath) {
66103
URL[] urls =
67104
RichStream.from(classpath)

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

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public class EnvironmentFilter {
3636
"Apple_PubSub_Socket_Render", // OS X pubsub control variable.
3737
"BUCK_BUILD_ID", // Build ID passed in from Python.
3838
"BUCK_CLASSPATH", // Main classpath; set in Python
39+
"BUCK_CLASSPATH_EXTRA", // Main classpath; set in Python
3940
"CHGHG", // Mercurial
4041
"CHGTIMEOUT", // Mercurial
4142
"CLASSPATH", // Bootstrap classpath; set in Python.

0 commit comments

Comments
 (0)