Skip to content

Commit b175ea0

Browse files
committed
[GR-52401] Fix #356 and #386: executable path resolution when GraalPy bin is last in $PATH.
PullRequest: graalpython/3243
2 parents 3c35312 + aefa7f6 commit b175ea0

File tree

6 files changed

+159
-24
lines changed

6 files changed

+159
-24
lines changed

graalpython/com.oracle.graal.python.shell/src/com/oracle/graal/python/shell/ConsoleHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -58,7 +58,7 @@
5858
* The interface to a source of input/output for the context, which may have different
5959
* implementations for different contexts.
6060
*/
61-
public abstract class ConsoleHandler {
61+
abstract class ConsoleHandler {
6262

6363
/**
6464
* Read a line of input, newline is <b>NOT</b> included in result.

graalpython/com.oracle.graal.python.shell/src/com/oracle/graal/python/shell/DefaultConsoleHandler.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -45,11 +45,11 @@
4545
import java.io.InputStream;
4646
import java.io.InputStreamReader;
4747

48-
public class DefaultConsoleHandler extends ConsoleHandler {
48+
class DefaultConsoleHandler extends ConsoleHandler {
4949

5050
private final BufferedReader in;
5151

52-
public DefaultConsoleHandler(InputStream in) {
52+
DefaultConsoleHandler(InputStream in) {
5353
this.in = new BufferedReader(new InputStreamReader(in));
5454
}
5555

graalpython/com.oracle.graal.python.shell/src/com/oracle/graal/python/shell/GraalPythonMain.java

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,13 @@
6666

6767
public class GraalPythonMain extends AbstractLanguageLauncher {
6868

69-
public static final String SHORT_HELP = "usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...\n" +
69+
private static final String SHORT_HELP = "usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...\n" +
7070
"Try `python -h' for more information.";
7171

72-
public static final String STRING_LIST_DELIMITER = "🏆";
72+
private static final String STRING_LIST_DELIMITER = "🏆";
7373

7474
// Duplicate of SysModuleBuiltins.INT_MAX_STR_DIGITS_THRESHOLD
75-
public static final int INT_MAX_STR_DIGITS_THRESHOLD = 640;
75+
private static final int INT_MAX_STR_DIGITS_THRESHOLD = 640;
7676

7777
/**
7878
* The first method called with the arguments by the thin launcher is
@@ -480,7 +480,7 @@ private static void print(String string) {
480480
System.err.println(string);
481481
}
482482

483-
protected String getLauncherExecName() {
483+
private String getLauncherExecName() {
484484
if (execName != null) {
485485
return execName;
486486
}
@@ -489,7 +489,7 @@ protected String getLauncherExecName() {
489489
if (execName == null) {
490490
return null;
491491
}
492-
execName = calculateProgramFullPath(execName);
492+
execName = calculateProgramFullPath(execName, Files::isExecutable, null);
493493
log("resolved executable name: ", execName);
494494
return execName;
495495
}
@@ -510,9 +510,12 @@ protected String getLauncherExecName() {
510510
* </dl>
511511
*
512512
* @param program The program name as passed in the process' argument vector (position 0).
513+
* @param isExecutable Check whether given {@link Path} exists and is executable (for testing).
514+
* @param envPath If non-null: value to be used as $PATH (for testing), otherwise
515+
* {@link #getEnv(String)} is used to retrieve $PATH.
513516
* @return The absolute path to the program or {@code null}.
514517
*/
515-
private String calculateProgramFullPath(String program) {
518+
private String calculateProgramFullPath(String program, Function<Path, Boolean> isExecutable, String envPath) {
516519
Path programPath = Paths.get(program);
517520

518521
// If this is an absolute path, we are already fine.
@@ -527,21 +530,22 @@ private String calculateProgramFullPath(String program) {
527530
*/
528531
if (programPath.getNameCount() < 2) {
529532
// Resolve the program name with respect to the PATH variable.
530-
String path = getEnv("PATH");
533+
String path = envPath != null ? envPath : getEnv("PATH");
531534
if (path != null) {
532535
log("resolving the executable name in $PATH = ", path);
533-
int last = 0;
534-
for (int i = path.indexOf(File.pathSeparatorChar); i != -1; i = path.indexOf(File.pathSeparatorChar, last)) {
535-
Path resolvedProgramName = Paths.get(path.substring(last, i)).resolve(programPath);
536-
log("checking if exists and is executable: ", resolvedProgramName);
537-
if (Files.isExecutable(resolvedProgramName)) {
536+
int i, previous = 0;
537+
do {
538+
i = path.indexOf(File.pathSeparatorChar, previous);
539+
int end = i == -1 ? path.length() : i;
540+
Path resolvedProgramName = Paths.get(path.substring(previous, end)).resolve(programPath);
541+
if (isExecutable.apply(resolvedProgramName)) {
538542
return resolvedProgramName.toString();
539543
}
540544

541545
// next start is the char after the separator because we have "path0:path1" and
542546
// 'i' points to ':'
543-
last = i + 1;
544-
}
547+
previous = i + 1;
548+
} while (i != -1);
545549
} else {
546550
log("executable name looks like it is from $PATH, but $PATH is not available.");
547551
}
@@ -1103,7 +1107,7 @@ private static ConsoleHandler createConsoleHandler(InputStream inStream, OutputS
11031107
* In case 2, we must implicitly execute a {@code quit("default, 0L, TRUE} command before
11041108
* exiting. So,in either case, we never return.
11051109
*/
1106-
public int readEvalPrint(Context context, ConsoleHandler consoleHandler) {
1110+
private int readEvalPrint(Context context, ConsoleHandler consoleHandler) {
11071111
int lastStatus = 0;
11081112
try {
11091113
setupREPL(context, consoleHandler);

graalpython/com.oracle.graal.python.shell/src/com/oracle/graal/python/shell/JLineConsoleHandler.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -56,9 +56,9 @@
5656
import java.util.function.IntConsumer;
5757
import java.util.function.IntFunction;
5858
import java.util.function.IntSupplier;
59+
5960
import org.graalvm.shadowed.org.jline.keymap.KeyMap;
6061
import org.graalvm.shadowed.org.jline.reader.Binding;
61-
6262
import org.graalvm.shadowed.org.jline.reader.Candidate;
6363
import org.graalvm.shadowed.org.jline.reader.Completer;
6464
import org.graalvm.shadowed.org.jline.reader.EndOfFileException;
@@ -72,15 +72,15 @@
7272
import org.graalvm.shadowed.org.jline.terminal.Terminal;
7373
import org.graalvm.shadowed.org.jline.terminal.TerminalBuilder;
7474

75-
public class JLineConsoleHandler extends ConsoleHandler {
75+
class JLineConsoleHandler extends ConsoleHandler {
7676
private final boolean noPrompt;
7777
private final Terminal terminal;
7878
private LineReader reader;
7979
private History history;
8080
private String prompt;
8181
private LinkedList<String> lineBuffer = new LinkedList<>();
8282

83-
public JLineConsoleHandler(InputStream inStream, OutputStream outStream, boolean noPrompt) {
83+
JLineConsoleHandler(InputStream inStream, OutputStream outStream, boolean noPrompt) {
8484
this.noPrompt = noPrompt;
8585
try {
8686
this.terminal = TerminalBuilder.builder().jna(false).streams(inStream, outStream).system(true).signalHandler(Terminal.SignalHandler.SIG_IGN).build();
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.test.shell;
42+
43+
import java.lang.reflect.Method;
44+
import java.nio.file.Path;
45+
import java.nio.file.Paths;
46+
import java.util.function.Function;
47+
48+
import org.junit.Assert;
49+
import org.junit.Before;
50+
import org.junit.Test;
51+
52+
import com.oracle.graal.python.shell.GraalPythonMain;
53+
54+
public class TestPathResolution {
55+
private Method calculateProgramFullPathMethod;
56+
57+
@Before
58+
public void setup() throws NoSuchMethodException {
59+
// Use reflection to avoid exposing the method in public API
60+
calculateProgramFullPathMethod = GraalPythonMain.class.getDeclaredMethod("calculateProgramFullPath", String.class, Function.class, String.class);
61+
calculateProgramFullPathMethod.setAccessible(true);
62+
}
63+
64+
public String calculateProgramFullPath(String executable, Function<Path, Boolean> isExecutable, String path) {
65+
try {
66+
return (String) calculateProgramFullPathMethod.invoke(new GraalPythonMain(), executable, isExecutable, path);
67+
} catch (Throwable e) {
68+
throw new RuntimeException(e);
69+
}
70+
}
71+
72+
@Test
73+
public void testProgramNameResolutionFullPath() {
74+
Assert.assertEquals("/absolute/path/graalpy",
75+
calculateProgramFullPath("/absolute/path/graalpy", path -> true, null));
76+
77+
Assert.assertEquals(Paths.get("./blah/graalpy").toAbsolutePath().normalize().toString(),
78+
calculateProgramFullPath("./blah/graalpy", path -> true, null));
79+
}
80+
81+
@Test
82+
public void testProgramNameResolutionEndOfPath() {
83+
Assert.assertEquals("/last/in/path/graalpy",
84+
calculateProgramFullPath("graalpy", path -> path.startsWith("/last"), "/first/in/path:/last/in/path/"));
85+
86+
// trailing slash
87+
Assert.assertEquals("/last/in/path/graalpy",
88+
calculateProgramFullPath("graalpy", path -> path.startsWith("/last"), "/first/in/path:/last/in/path/"));
89+
90+
// trailing colon
91+
Assert.assertEquals("/last/in/path/graalpy",
92+
calculateProgramFullPath("graalpy", path -> path.startsWith("/last"), "/first/in/path:/last/in/path:"));
93+
94+
// leading colon
95+
Assert.assertEquals("/last/in/path/graalpy",
96+
calculateProgramFullPath("graalpy", path -> path.startsWith("/last"), ":/first/in/path:/last/in/path"));
97+
}
98+
99+
@Test
100+
public void testProgramNameResolutionStartOfPath() {
101+
Assert.assertEquals("/first/in/path/graalpy",
102+
calculateProgramFullPath("graalpy", path -> path.startsWith("/first"), "/first/in/path:/last/in/path/"));
103+
104+
// trailing slash
105+
Assert.assertEquals("/first/in/path/graalpy",
106+
calculateProgramFullPath("graalpy", path -> path.startsWith("/first"), "/first/in/path/:/last/in/path/"));
107+
108+
// leading colon
109+
Assert.assertEquals("/first/in/path/graalpy",
110+
calculateProgramFullPath("graalpy", path -> path.startsWith("/first"), ":/first/in/path:/last/in/path"));
111+
}
112+
113+
@Test
114+
public void testProgramNameResolutionInPath() {
115+
Assert.assertEquals("/second/in/path/graalpy",
116+
calculateProgramFullPath("graalpy", path -> path.startsWith("/second"), "/first/in/path:/second/in/path/:/last/in/path"));
117+
118+
// with spaces
119+
Assert.assertEquals("/next/in/path/graalpy",
120+
calculateProgramFullPath("graalpy", path -> path.startsWith("/next"), "/first/in/path:/path with spaces:/next/in/path/:/last/in/path"));
121+
122+
// with spaces as the result
123+
Assert.assertEquals("/path with spaces/graalpy",
124+
calculateProgramFullPath("graalpy", path -> path.startsWith("/path with spaces"), "/first/in/path:/path with spaces:/next/in/path/:/last/in/path"));
125+
126+
// single path element
127+
Assert.assertEquals("/single/in/path/graalpy",
128+
calculateProgramFullPath("graalpy", path -> path.startsWith("/single"), "/single/in/path"));
129+
}
130+
}

mx.graalpython/mx_graalpython.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3453,6 +3453,7 @@ def apply(self, config):
34533453
(vmArgs, mainClass, mainClassArgs) = config
34543454
mainClassArgs.extend(['-JUnitOpenPackages', 'org.graalvm.truffle/com.oracle.truffle.api.impl=ALL-UNNAMED']) # for TruffleRunner/TCK
34553455
mainClassArgs.extend(['-JUnitOpenPackages', 'org.graalvm.py/*=ALL-UNNAMED']) # for Python internals
3456+
mainClassArgs.extend(['-JUnitOpenPackages', 'org.graalvm.py.launcher/*=ALL-UNNAMED']) # for Python launcher internals
34563457
if not PythonMxUnittestConfig.useResources:
34573458
vmArgs.append('-Dorg.graalvm.language.python.home=' + mx.dependency("GRAALPYTHON_GRAALVM_SUPPORT").get_output())
34583459
return (vmArgs, mainClass, mainClassArgs)

0 commit comments

Comments
 (0)