From cab651940bae81d50e912b268adf54a5d29f131e Mon Sep 17 00:00:00 2001 From: Hitesh Date: Sat, 20 Jun 2026 20:47:34 +0530 Subject: [PATCH 1/4] [MNG-8768] Add executable() function for conditional profile activation based on PATH Implements MNG-8768 (GitHub #10115): adds an executable(name_or_path) function to Maven's condition-based profile activation DSL. The function evaluates to true if the given executable name can be found in the system PATH, or if an absolute/relative path points directly to an executable file. Use-case (from the issue): auto-detect the presence of x86_64-linux-musl-gcc when building GraalVM native images with the native-maven-plugin, and conditionally add --static --libc=musl to the build options only when the compiler wrapper is available on the current machine. Example pom.xml usage: executable('x86_64-linux-musl-gcc') Implementation details: - ConditionFunctions.executable() delegates to new ExecutableFinder helper - ExecutableFinder splits the env.PATH system property and probes each directory; on Windows it also tries .exe / .cmd / .bat / .com extensions - Absolute/relative paths (containing a path separator) are checked directly without PATH traversal - PATH is sourced from ProfileActivationContext.getSystemProperty('env.PATH') (Maven's normalised env var key), with a System.getenv('PATH') fallback Tests: - 5 new end-to-end tests in ConditionProfileActivatorTest (MNG-8768) - 12 new unit tests in ExecutableFinderTest covering PATH search, Windows extension probing, absolute paths, non-executable files, and empty PATH --- .../model/profile/ConditionFunctions.java | 32 +++ .../impl/model/profile/ExecutableFinder.java | 179 +++++++++++++++ .../ConditionProfileActivatorTest.java | 76 +++++++ .../model/profile/ExecutableFinderTest.java | 211 ++++++++++++++++++ 4 files changed, 498 insertions(+) create mode 100644 impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java index 1a9087273962..8322bf3543b0 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java @@ -237,4 +237,36 @@ public Object inrange(List args) { String range = ConditionParser.toString(args.get(1)); return versionParser.parseVersionRange(range).contains(versionParser.parseVersion(version)); } + + /** + * Checks whether a given executable can be found in the system PATH, or – if an + * absolute / relative path is supplied – whether that path itself is an executable file. + * + *

Usage examples in a profile {@code }: + *

+     *   executable('musl-gcc')
+     *   executable('x86_64-linux-musl-gcc')
+     *   executable('/usr/bin/musl-gcc')
+     * 
+ * + *

When a plain name (without path separators) is given the function searches every + * directory listed in the {@code PATH} environment variable. On Windows, the platform + * executable extensions ({@code .exe}, {@code .cmd}, {@code .bat}, {@code .com}) are + * tried automatically when the name does not already carry an extension. + * + * @param args A list containing a single string argument: the executable name or path + * @return {@code true} if the executable is found and is a regular, executable file, + * {@code false} otherwise + * @throws IllegalArgumentException if the number of arguments is not exactly one + */ + public Object executable(List args) { + if (args.size() != 1) { + throw new IllegalArgumentException("executable function requires exactly one argument"); + } + String name = ConditionParser.toString(args.get(0)); + if (name == null || name.isBlank()) { + return false; + } + return ExecutableFinder.isExecutableInPath(name, context); + } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java new file mode 100644 index 000000000000..031e2337c330 --- /dev/null +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.maven.impl.model.profile; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +import org.apache.maven.api.services.model.ProfileActivationContext; + +/** + * Helper that implements the OS-aware PATH search used by the {@code executable()} condition function. + * + *

The search strategy is: + *

    + *
  1. If {@code name} contains a path separator (i.e. it already looks like a path), treat it as + * an absolute or relative file path and check it directly.
  2. + *
  3. Otherwise, retrieve the {@code PATH} value from the activation context's system properties + * (Maven normalises env vars to {@code env.PATH} / {@code env.Path} etc.) and split it by the + * platform path separator. Each directory is searched in order.
  4. + *
  5. On Windows, when the candidate does not already have one of the known executable extensions + * ({@code .exe}, {@code .cmd}, {@code .bat}, {@code .com}), those extensions are appended and + * tried as well.
  6. + *
+ * + * @since 4.x + */ +class ExecutableFinder { + + /** Windows-specific executable file extensions, in search order. */ + private static final String[] WINDOWS_EXTENSIONS = {".exe", ".cmd", ".bat", ".com"}; + + /** The system property key under which Maven exposes the {@code PATH} environment variable. */ + private static final String ENV_PATH_KEY = "env.PATH"; + + private ExecutableFinder() {} + + /** + * Returns {@code true} when {@code name} resolves to an executable file. + * + * @param name the executable name (e.g. {@code "musl-gcc"}) or an absolute/relative path + * @param context the current profile activation context + * @return {@code true} if the executable is found and is a regular, executable file + */ + static boolean isExecutableInPath(String name, ProfileActivationContext context) { + boolean isWindows = isWindows(); + + // If the name already contains a path separator treat it as a direct path. + if (name.contains("/") || name.contains(File.separator)) { + Path candidate = Paths.get(name); + return isExecutableFile(candidate, isWindows); + } + + // --- plain name: search PATH --- + String pathValue = getPathValue(context); + if (pathValue == null || pathValue.isBlank()) { + return false; + } + + String[] dirs = pathValue.split(File.pathSeparator, -1); + for (String dir : dirs) { + if (dir.isBlank()) { + continue; + } + Path base = Paths.get(dir).resolve(name); + if (isExecutableFile(base, isWindows)) { + return true; + } + // On Windows also try known executable extensions (unless already present). + if (isWindows && !hasWindowsExtension(name)) { + for (String ext : WINDOWS_EXTENSIONS) { + Path withExt = Paths.get(dir).resolve(name + ext); + if (isExecutableFile(withExt, isWindows)) { + return true; + } + } + } + } + return false; + } + + // ----------------------------------------------------------------------- + // Package-private helpers (visible to tests) + // ----------------------------------------------------------------------- + + /** + * Retrieves the PATH value from the activation context. + * + *

Maven places env vars in system properties as {@code env.}. On Windows, env var names + * are normalised to upper-case, so we first try {@code env.PATH} and fall back to a direct + * {@link System#getenv(String)} call to handle edge cases. + * + * @param context the profile activation context + * @return the raw PATH string, or {@code null} if not available + */ + static String getPathValue(ProfileActivationContext context) { + // Maven stores env vars as "env." (upper-cased on Windows) + String pathValue = context.getSystemProperty(ENV_PATH_KEY); + if (pathValue == null) { + // Fallback: try the OS environment directly (works in tests that do not populate + // env-based system properties). + pathValue = System.getenv("PATH"); + } + return pathValue; + } + + /** + * Returns the list of candidate names to probe for a given executable name on the current OS. + * On Windows the known executable extensions are appended when the name has none. + * + * @param name the bare executable name (no directory part) + * @param isWindows whether the current OS is Windows + * @return ordered list of candidate file names to probe in each PATH directory + */ + static List candidateNames(String name, boolean isWindows) { + List candidates = new ArrayList<>(); + candidates.add(name); + if (isWindows && !hasWindowsExtension(name)) { + for (String ext : WINDOWS_EXTENSIONS) { + candidates.add(name + ext); + } + } + return candidates; + } + + // ----------------------------------------------------------------------- + // Private utilities + // ----------------------------------------------------------------------- + + private static boolean isWindows() { + return System.getProperty("os.name", "").toLowerCase(Locale.ROOT).contains("windows"); + } + + /** + * Returns {@code true} if {@code path} is a regular file that the JVM considers executable. + * On Windows, any regular file is treated as potentially executable (the OS itself uses the + * extension to decide); the {@link Files#isExecutable} check is still applied so that + * read-only / locked files are excluded. + */ + private static boolean isExecutableFile(Path path, boolean isWindows) { + if (!Files.isRegularFile(path)) { + return false; + } + // On Windows Files.isExecutable() always returns true for regular files – that is fine + // because we are already filtering by extension in the caller. On Unix we rely on the + // execute bit. + return isWindows || Files.isExecutable(path); + } + + private static boolean hasWindowsExtension(String name) { + String lower = name.toLowerCase(Locale.ROOT); + for (String ext : WINDOWS_EXTENSIONS) { + if (lower.endsWith(ext)) { + return true; + } + } + return false; + } +} diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java index a47ffc32259f..5d7722b15736 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java @@ -490,4 +490,80 @@ protected ProfileActivationContext newFileContext(Path path) { protected ProfileActivationContext newFileContext() { return newFileContext(tempDir); } + + // ----------------------------------------------------------------------- + // executable() tests (MNG-8768) + // ----------------------------------------------------------------------- + + /** + * Puts a fake executable into a temporary directory and activates a profile only when + * that directory is on the PATH – confirming the PATH-search logic end-to-end. + */ + @Test + void testExecutablePresentInPath() throws Exception { + // Create a fake executable in the temp dir + Path fakeExec = tempDir.resolve("my-fake-tool"); + Files.createFile(fakeExec); + // Make it executable on POSIX systems; on Windows Files.isExecutable() returns true anyway + fakeExec.toFile().setExecutable(true); + + String pathValue = tempDir.toAbsolutePath().toString(); + Map sysProps = Map.of("env.PATH", pathValue); + + Profile profile = newProfile("executable('my-fake-tool')"); + assertActivation(true, profile, newContext(null, sysProps)); + } + + /** + * Verifies that the function returns false for a name that is definitely not on PATH. + */ + @Test + void testExecutableNotInPath() { + // Use an empty/nonexistent PATH so that nothing can be found + Map sysProps = Map.of("env.PATH", ""); + + Profile profile = newProfile("executable('this-tool-does-not-exist-anywhere-42')"); + assertActivation(false, profile, newContext(null, sysProps)); + } + + /** + * An absolute path to an existing executable file must resolve to true. + */ + @Test + void testExecutableWithAbsolutePath() throws Exception { + Path fakeExec = tempDir.resolve("abs-tool"); + Files.createFile(fakeExec); + fakeExec.toFile().setExecutable(true); + + String absPath = fakeExec.toAbsolutePath().toString().replace('\\', '/'); + Profile profile = newProfile("executable('" + absPath + "')"); + + // PATH content does not matter for absolute paths + assertActivation(true, profile, newContext(null, Map.of("env.PATH", ""))); + } + + /** + * An absolute path to a non-existent file must resolve to false. + */ + @Test + void testExecutableAbsolutePathMissing() { + Profile profile = newProfile("executable('/no/such/executable/path/42/bin/tool')"); + assertActivation(false, profile, newContext(null, Map.of("env.PATH", ""))); + } + + /** + * not(executable(...)) must invert the result correctly. + */ + @Test + void testExecutableNegated() throws Exception { + Path fakeExec = tempDir.resolve("neg-tool"); + Files.createFile(fakeExec); + fakeExec.toFile().setExecutable(true); + + String pathValue = tempDir.toAbsolutePath().toString(); + Map sysProps = Map.of("env.PATH", pathValue); + + assertActivation(false, newProfile("not(executable('neg-tool'))"), newContext(null, sysProps)); + assertActivation(true, newProfile("not(executable('no-such-neg-tool'))"), newContext(null, sysProps)); + } } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java new file mode 100644 index 000000000000..4baddbfcbfd2 --- /dev/null +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java @@ -0,0 +1,211 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.maven.impl.model.profile; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import org.apache.maven.api.services.model.ProfileActivationContext; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Unit tests for {@link ExecutableFinder}. + */ +class ExecutableFinderTest { + + @TempDir + Path tempDir; + + /** + * Minimal stub – only {@link ProfileActivationContext#getSystemProperty(String)} is used + * by {@link ExecutableFinder#getPathValue}. + */ + private static ProfileActivationContext contextWithPath(String pathValue) { + Map props = pathValue != null ? Map.of("env.PATH", pathValue) : Map.of(); + return new ProfileActivationContext() { + @Override + public boolean isProfileActive(String profileId) { + return false; + } + + @Override + public boolean isProfileInactive(String profileId) { + return false; + } + + @Override + public String getSystemProperty(String key) { + return props.get(key); + } + + @Override + public String getUserProperty(String key) { + return null; + } + + @Override + public String getModelProperty(String key) { + return null; + } + + @Override + public String getModelArtifactId() { + return null; + } + + @Override + public String getModelPackaging() { + return null; + } + + @Override + public String getModelRootDirectory() { + return null; + } + + @Override + public String getModelBaseDirectory() { + return null; + } + + @Override + public String interpolatePath(String path) { + return path; + } + + @Override + public boolean exists(String path, boolean glob) { + return false; + } + }; + } + + // ----------------------------------------------------------------------- + // candidateNames() + // ----------------------------------------------------------------------- + + @Test + void candidateNamesUnixNoExtension() { + List names = ExecutableFinder.candidateNames("musl-gcc", false); + assertEquals(List.of("musl-gcc"), names); + } + + @Test + void candidateNamesWindowsNoExtension() { + List names = ExecutableFinder.candidateNames("musl-gcc", true); + assertEquals(List.of("musl-gcc", "musl-gcc.exe", "musl-gcc.cmd", "musl-gcc.bat", "musl-gcc.com"), names); + } + + @Test + void candidateNamesWindowsAlreadyHasExtension() { + // When the name already has a Windows extension, no extras should be added. + List names = ExecutableFinder.candidateNames("myapp.exe", true); + assertEquals(List.of("myapp.exe"), names); + } + + // ----------------------------------------------------------------------- + // getPathValue() + // ----------------------------------------------------------------------- + + @Test + void getPathValueFromContext() { + String expected = "/usr/bin" + File.pathSeparator + "/usr/local/bin"; + ProfileActivationContext ctx = contextWithPath(expected); + assertEquals(expected, ExecutableFinder.getPathValue(ctx)); + } + + @Test + void getPathValueFallsBackToSystemEnv() { + // No env.PATH in context -> should fall back to System.getenv("PATH") + ProfileActivationContext ctx = contextWithPath(null); + String fromEnv = System.getenv("PATH"); + assertEquals(fromEnv, ExecutableFinder.getPathValue(ctx)); + } + + // ----------------------------------------------------------------------- + // isExecutableInPath() - plain name + // ----------------------------------------------------------------------- + + @Test + void findsExecutableByName() throws Exception { + Path exec = tempDir.resolve("my-tool"); + Files.createFile(exec); + exec.toFile().setExecutable(true); + + assertTrue(ExecutableFinder.isExecutableInPath("my-tool", contextWithPath(tempDir.toString()))); + } + + @Test + void returnsFalseWhenFileIsNotExecutable() throws Exception { + Path exec = tempDir.resolve("non-exec-tool"); + Files.createFile(exec); + exec.toFile().setExecutable(false); + + // Only meaningful on POSIX; on Windows the execute bit is not enforced by the JVM. + String os = System.getProperty("os.name", "").toLowerCase(); + if (!os.contains("windows")) { + assertFalse(ExecutableFinder.isExecutableInPath("non-exec-tool", contextWithPath(tempDir.toString()))); + } + } + + @Test + void returnsFalseWhenToolNotInPath() { + assertFalse(ExecutableFinder.isExecutableInPath( + "this-tool-definitely-does-not-exist-anywhere-12345", contextWithPath(tempDir.toString()))); + } + + @Test + void returnsFalseForEmptyPath() { + assertFalse(ExecutableFinder.isExecutableInPath("any-tool", contextWithPath(""))); + } + + // ----------------------------------------------------------------------- + // isExecutableInPath() - absolute / relative path + // ----------------------------------------------------------------------- + + @Test + void findsExecutableByAbsolutePath() throws Exception { + Path exec = tempDir.resolve("abs-exec"); + Files.createFile(exec); + exec.toFile().setExecutable(true); + + String absPath = exec.toAbsolutePath().toString(); + // Absolute paths contain path separators -> direct check + assertTrue(ExecutableFinder.isExecutableInPath(absPath, contextWithPath(""))); + } + + @Test + void returnsFalseForAbsolutePathThatDoesNotExist() { + assertFalse(ExecutableFinder.isExecutableInPath("/no/such/path/to/some/binary/42", contextWithPath(""))); + } + + @Test + void returnsFalseForDirectoryPath() throws Exception { + // Directories must not be accepted even if they exist. + assertFalse(ExecutableFinder.isExecutableInPath(tempDir.toAbsolutePath().toString(), contextWithPath(""))); + } +} From 6d48629151d47ec7a69b2bc2db0c584a10d092b7 Mon Sep 17 00:00:00 2001 From: Hitesh Date: Mon, 22 Jun 2026 21:11:28 +0530 Subject: [PATCH 2/4] Address review comments for executable() function - Add a warning in DefaultModelValidator if executable() condition is used, noting reproducibility issues - Add a Javadoc warning to ConditionFunctions.executable() - Remove System.getenv('PATH') fallback in ExecutableFinder - Fix OS checking to use ProfileActivationContext instead of System.getProperty - Convert Paths.get to Path.of - Remove dead code (candidateNames) - Update tests: remove fallback test, use @DisabledOnOs(OS.WINDOWS) for OS-specific test --- .../impl/model/DefaultModelValidator.java | 11 +++++ .../model/profile/ConditionFunctions.java | 4 ++ .../impl/model/profile/ExecutableFinder.java | 49 ++++--------------- .../model/profile/ExecutableFinderTest.java | 40 ++------------- your-database.db | 0 5 files changed, 29 insertions(+), 75 deletions(-) create mode 100644 your-database.db diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index 8cc8cb3459b3..1a470669d13e 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -713,6 +713,17 @@ private void validate30RawProfileActivation(ModelProblemCollector problems, Acti return; } + if (activation.getCondition() != null && activation.getCondition().contains("executable(")) { + addViolation( + problems, + Severity.WARNING, + Version.V40, + prefix + "activation.condition", + null, + "Profile activation relies on the 'executable' function, which makes the profile activation environment-dependent. This means the published POM will not be reproducible. Consider using this function only in local build profiles or stripping it before publication.", + activation.getLocation("condition")); + } + final Deque stk = new LinkedList<>(); final Supplier pathSupplier = () -> { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java index 8322bf3543b0..6dce84ee8853 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionFunctions.java @@ -242,6 +242,10 @@ public Object inrange(List args) { * Checks whether a given executable can be found in the system PATH, or – if an * absolute / relative path is supplied – whether that path itself is an executable file. * + *

Warning: relying on local system environment variables like PATH makes + * profile activation non-reproducible. This function should typically be used only in local + * build profiles and not in consumer POMs that are published to a remote repository.

+ * *

Usage examples in a profile {@code }: *

      *   executable('musl-gcc')
diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java
index 031e2337c330..35dbcbadfc66 100644
--- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java
+++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java
@@ -21,9 +21,6 @@
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Locale;
 
 import org.apache.maven.api.services.model.ProfileActivationContext;
@@ -63,11 +60,11 @@ private ExecutableFinder() {}
      * @return {@code true} if the executable is found and is a regular, executable file
      */
     static boolean isExecutableInPath(String name, ProfileActivationContext context) {
-        boolean isWindows = isWindows();
+        boolean isWindows = isWindows(context);
 
         // If the name already contains a path separator treat it as a direct path.
         if (name.contains("/") || name.contains(File.separator)) {
-            Path candidate = Paths.get(name);
+            Path candidate = Path.of(name);
             return isExecutableFile(candidate, isWindows);
         }
 
@@ -82,14 +79,14 @@ static boolean isExecutableInPath(String name, ProfileActivationContext context)
             if (dir.isBlank()) {
                 continue;
             }
-            Path base = Paths.get(dir).resolve(name);
+            Path base = Path.of(dir).resolve(name);
             if (isExecutableFile(base, isWindows)) {
                 return true;
             }
             // On Windows also try known executable extensions (unless already present).
             if (isWindows && !hasWindowsExtension(name)) {
                 for (String ext : WINDOWS_EXTENSIONS) {
-                    Path withExt = Paths.get(dir).resolve(name + ext);
+                    Path withExt = Path.of(dir).resolve(name + ext);
                     if (isExecutableFile(withExt, isWindows)) {
                         return true;
                     }
@@ -106,49 +103,23 @@ static boolean isExecutableInPath(String name, ProfileActivationContext context)
     /**
      * Retrieves the PATH value from the activation context.
      *
-     * 

Maven places env vars in system properties as {@code env.}. On Windows, env var names - * are normalised to upper-case, so we first try {@code env.PATH} and fall back to a direct - * {@link System#getenv(String)} call to handle edge cases. + *

Maven places env vars in system properties as {@code env.}. + * On Windows, env var names are normalised to upper-case (e.g. {@code env.PATH}). * * @param context the profile activation context * @return the raw PATH string, or {@code null} if not available */ static String getPathValue(ProfileActivationContext context) { - // Maven stores env vars as "env." (upper-cased on Windows) - String pathValue = context.getSystemProperty(ENV_PATH_KEY); - if (pathValue == null) { - // Fallback: try the OS environment directly (works in tests that do not populate - // env-based system properties). - pathValue = System.getenv("PATH"); - } - return pathValue; - } - - /** - * Returns the list of candidate names to probe for a given executable name on the current OS. - * On Windows the known executable extensions are appended when the name has none. - * - * @param name the bare executable name (no directory part) - * @param isWindows whether the current OS is Windows - * @return ordered list of candidate file names to probe in each PATH directory - */ - static List candidateNames(String name, boolean isWindows) { - List candidates = new ArrayList<>(); - candidates.add(name); - if (isWindows && !hasWindowsExtension(name)) { - for (String ext : WINDOWS_EXTENSIONS) { - candidates.add(name + ext); - } - } - return candidates; + return context.getSystemProperty(ENV_PATH_KEY); } // ----------------------------------------------------------------------- // Private utilities // ----------------------------------------------------------------------- - private static boolean isWindows() { - return System.getProperty("os.name", "").toLowerCase(Locale.ROOT).contains("windows"); + private static boolean isWindows(ProfileActivationContext context) { + String osName = context.getSystemProperty("os.name"); + return osName != null && osName.toLowerCase(Locale.ROOT).contains("windows"); } /** diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java index 4baddbfcbfd2..035434e083cf 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java @@ -21,11 +21,12 @@ import java.io.File; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; import java.util.Map; import org.apache.maven.api.services.model.ProfileActivationContext; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.io.TempDir; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -104,29 +105,6 @@ public boolean exists(String path, boolean glob) { }; } - // ----------------------------------------------------------------------- - // candidateNames() - // ----------------------------------------------------------------------- - - @Test - void candidateNamesUnixNoExtension() { - List names = ExecutableFinder.candidateNames("musl-gcc", false); - assertEquals(List.of("musl-gcc"), names); - } - - @Test - void candidateNamesWindowsNoExtension() { - List names = ExecutableFinder.candidateNames("musl-gcc", true); - assertEquals(List.of("musl-gcc", "musl-gcc.exe", "musl-gcc.cmd", "musl-gcc.bat", "musl-gcc.com"), names); - } - - @Test - void candidateNamesWindowsAlreadyHasExtension() { - // When the name already has a Windows extension, no extras should be added. - List names = ExecutableFinder.candidateNames("myapp.exe", true); - assertEquals(List.of("myapp.exe"), names); - } - // ----------------------------------------------------------------------- // getPathValue() // ----------------------------------------------------------------------- @@ -138,14 +116,6 @@ void getPathValueFromContext() { assertEquals(expected, ExecutableFinder.getPathValue(ctx)); } - @Test - void getPathValueFallsBackToSystemEnv() { - // No env.PATH in context -> should fall back to System.getenv("PATH") - ProfileActivationContext ctx = contextWithPath(null); - String fromEnv = System.getenv("PATH"); - assertEquals(fromEnv, ExecutableFinder.getPathValue(ctx)); - } - // ----------------------------------------------------------------------- // isExecutableInPath() - plain name // ----------------------------------------------------------------------- @@ -160,16 +130,14 @@ void findsExecutableByName() throws Exception { } @Test + @DisabledOnOs(OS.WINDOWS) void returnsFalseWhenFileIsNotExecutable() throws Exception { Path exec = tempDir.resolve("non-exec-tool"); Files.createFile(exec); exec.toFile().setExecutable(false); // Only meaningful on POSIX; on Windows the execute bit is not enforced by the JVM. - String os = System.getProperty("os.name", "").toLowerCase(); - if (!os.contains("windows")) { - assertFalse(ExecutableFinder.isExecutableInPath("non-exec-tool", contextWithPath(tempDir.toString()))); - } + assertFalse(ExecutableFinder.isExecutableInPath("non-exec-tool", contextWithPath(tempDir.toString()))); } @Test diff --git a/your-database.db b/your-database.db new file mode 100644 index 000000000000..e69de29bb2d1 From bfeb6f7938342f3247572694e12fc2d880e70eab Mon Sep 17 00:00:00 2001 From: Hitesh Date: Tue, 23 Jun 2026 11:43:26 +0530 Subject: [PATCH 3/4] Address second review: remove accidental file, fix Windows coverage, fix direct-path comment - Remove accidental your-database.db from repo root - Remove .replace('\\', '/') from ConditionProfileActivatorTest.testExecutableWithAbsolutePath() - Fix isExecutableFile() comment: no longer claims caller always filters by extension - Direct paths on Windows now also probe .exe/.cmd/.bat/.com extensions - Add contextWithPathAndOs() test helper that sets os.name in context - Add 3 new Windows-simulated tests: - findsExecutableWithWindowsExtensionInPath: PATH search finds my-tool.exe for 'my-tool' - findsExecutableWithWindowsExtensionByDirectPath: direct path probes .exe - doesNotAppendExtensionOnNonWindows: confirms no extension probing on Linux --- .../impl/model/profile/ExecutableFinder.java | 27 +++-- .../ConditionProfileActivatorTest.java | 2 +- .../model/profile/ExecutableFinderTest.java | 105 ++++++++++++++++++ your-database.db | 0 4 files changed, 126 insertions(+), 8 deletions(-) delete mode 100644 your-database.db diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java index 35dbcbadfc66..d004c86e25ae 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ExecutableFinder.java @@ -65,7 +65,18 @@ static boolean isExecutableInPath(String name, ProfileActivationContext context) // If the name already contains a path separator treat it as a direct path. if (name.contains("/") || name.contains(File.separator)) { Path candidate = Path.of(name); - return isExecutableFile(candidate, isWindows); + if (isExecutableFile(candidate, isWindows)) { + return true; + } + // On Windows also try known executable extensions for direct paths. + if (isWindows && !hasWindowsExtension(name)) { + for (String ext : WINDOWS_EXTENSIONS) { + if (isExecutableFile(Path.of(name + ext), isWindows)) { + return true; + } + } + } + return false; } // --- plain name: search PATH --- @@ -124,17 +135,19 @@ private static boolean isWindows(ProfileActivationContext context) { /** * Returns {@code true} if {@code path} is a regular file that the JVM considers executable. - * On Windows, any regular file is treated as potentially executable (the OS itself uses the - * extension to decide); the {@link Files#isExecutable} check is still applied so that - * read-only / locked files are excluded. + * + *

On Windows, {@link Files#isExecutable(Path)} always returns {@code true} for regular + * files, so this method simply checks that the file exists and is regular. The caller is + * responsible for probing Windows executable extensions ({@code .exe}, {@code .cmd}, etc.) + * when the user-supplied name does not already carry one. On Unix/POSIX systems the + * execute permission bit is checked via {@link Files#isExecutable(Path)}.

*/ private static boolean isExecutableFile(Path path, boolean isWindows) { if (!Files.isRegularFile(path)) { return false; } - // On Windows Files.isExecutable() always returns true for regular files – that is fine - // because we are already filtering by extension in the caller. On Unix we rely on the - // execute bit. + // On Windows Files.isExecutable() always returns true for regular files. + // On Unix we rely on the execute permission bit. return isWindows || Files.isExecutable(path); } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java index 5d7722b15736..017244f5979a 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionProfileActivatorTest.java @@ -535,7 +535,7 @@ void testExecutableWithAbsolutePath() throws Exception { Files.createFile(fakeExec); fakeExec.toFile().setExecutable(true); - String absPath = fakeExec.toAbsolutePath().toString().replace('\\', '/'); + String absPath = fakeExec.toAbsolutePath().toString(); Profile profile = newProfile("executable('" + absPath + "')"); // PATH content does not matter for absolute paths diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java index 035434e083cf..1e7847418e1a 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java @@ -21,6 +21,7 @@ import java.io.File; import java.nio.file.Files; import java.nio.file.Path; +import java.util.HashMap; import java.util.Map; import org.apache.maven.api.services.model.ProfileActivationContext; @@ -105,6 +106,75 @@ public boolean exists(String path, boolean glob) { }; } + /** + * Extended stub that also allows setting {@code os.name} for simulating Windows behaviour. + */ + private static ProfileActivationContext contextWithPathAndOs(String pathValue, String osName) { + Map props = new HashMap<>(); + if (pathValue != null) { + props.put("env.PATH", pathValue); + } + if (osName != null) { + props.put("os.name", osName); + } + return new ProfileActivationContext() { + @Override + public boolean isProfileActive(String profileId) { + return false; + } + + @Override + public boolean isProfileInactive(String profileId) { + return false; + } + + @Override + public String getSystemProperty(String key) { + return props.get(key); + } + + @Override + public String getUserProperty(String key) { + return null; + } + + @Override + public String getModelProperty(String key) { + return null; + } + + @Override + public String getModelArtifactId() { + return null; + } + + @Override + public String getModelPackaging() { + return null; + } + + @Override + public String getModelRootDirectory() { + return null; + } + + @Override + public String getModelBaseDirectory() { + return null; + } + + @Override + public String interpolatePath(String path) { + return path; + } + + @Override + public boolean exists(String path, boolean glob) { + return false; + } + }; + } + // ----------------------------------------------------------------------- // getPathValue() // ----------------------------------------------------------------------- @@ -176,4 +246,39 @@ void returnsFalseForDirectoryPath() throws Exception { // Directories must not be accepted even if they exist. assertFalse(ExecutableFinder.isExecutableInPath(tempDir.toAbsolutePath().toString(), contextWithPath(""))); } + + // ----------------------------------------------------------------------- + // Windows extension probing (simulated via os.name in context) + // ----------------------------------------------------------------------- + + @Test + void findsExecutableWithWindowsExtensionInPath() throws Exception { + // Simulate Windows: create my-tool.exe and search for "my-tool" + Path exec = tempDir.resolve("my-tool.exe"); + Files.createFile(exec); + + ProfileActivationContext ctx = contextWithPathAndOs(tempDir.toString(), "Windows 10"); + assertTrue(ExecutableFinder.isExecutableInPath("my-tool", ctx)); + } + + @Test + void findsExecutableWithWindowsExtensionByDirectPath() throws Exception { + // Simulate Windows: create tool.exe and search for the direct path without extension + Path exec = tempDir.resolve("tool.exe"); + Files.createFile(exec); + + String directPath = tempDir.resolve("tool").toAbsolutePath().toString(); + ProfileActivationContext ctx = contextWithPathAndOs("", "Windows 10"); + assertTrue(ExecutableFinder.isExecutableInPath(directPath, ctx)); + } + + @Test + void doesNotAppendExtensionOnNonWindows() throws Exception { + // On non-Windows, searching for "my-tool" must NOT find "my-tool.exe" + Path exec = tempDir.resolve("my-tool.exe"); + Files.createFile(exec); + + ProfileActivationContext ctx = contextWithPathAndOs(tempDir.toString(), "Linux"); + assertFalse(ExecutableFinder.isExecutableInPath("my-tool", ctx)); + } } diff --git a/your-database.db b/your-database.db deleted file mode 100644 index e69de29bb2d1..000000000000 From a61fd52943052f524d32daf981842d685cd8c995 Mon Sep 17 00:00:00 2001 From: Hitesh Date: Tue, 23 Jun 2026 12:57:21 +0530 Subject: [PATCH 4/4] Address minor nit: deduplicate contextWithPath test helpers --- .../model/profile/ExecutableFinderTest.java | 58 +------------------ 1 file changed, 1 insertion(+), 57 deletions(-) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java index 1e7847418e1a..3ada1a3fb5e8 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ExecutableFinderTest.java @@ -47,63 +47,7 @@ class ExecutableFinderTest { * by {@link ExecutableFinder#getPathValue}. */ private static ProfileActivationContext contextWithPath(String pathValue) { - Map props = pathValue != null ? Map.of("env.PATH", pathValue) : Map.of(); - return new ProfileActivationContext() { - @Override - public boolean isProfileActive(String profileId) { - return false; - } - - @Override - public boolean isProfileInactive(String profileId) { - return false; - } - - @Override - public String getSystemProperty(String key) { - return props.get(key); - } - - @Override - public String getUserProperty(String key) { - return null; - } - - @Override - public String getModelProperty(String key) { - return null; - } - - @Override - public String getModelArtifactId() { - return null; - } - - @Override - public String getModelPackaging() { - return null; - } - - @Override - public String getModelRootDirectory() { - return null; - } - - @Override - public String getModelBaseDirectory() { - return null; - } - - @Override - public String interpolatePath(String path) { - return path; - } - - @Override - public boolean exists(String path, boolean glob) { - return false; - } - }; + return contextWithPathAndOs(pathValue, null); } /**