This repository has been archived by the owner on Nov 10, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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: 7a95aac009e4c25c3ea536ec4a3d371525ee2654
- Loading branch information
1 parent
2c6a6b9
commit 102b8ba
Showing
8 changed files
with
249 additions
and
35 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
92 changes: 92 additions & 0 deletions
92
src/com/facebook/buck/cli/bootstrapper/ClassLoaderFactory.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
test/com/facebook/buck/cli/bootstrapper/ClassLoaderFactoryTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)))); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters