gnodet-bot commented on code in PR #10906:
URL: https://github.com/apache/maven/pull/10906#discussion_r4258917164
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -125,6 +125,13 @@ public final int invoke(InvokerRequest invokerRequest) {
oldProps.putAll(System.getProperties());
ClassLoader oldCL = Thread.currentThread().getContextClassLoader();
try (C context = createContext(invokerRequest)) {
+ // MSYS2 and Git Bash hand over Windows paths in their POSIX
style, e.g. /c/Users/alice
+ String userHome = System.getProperty("user.home");
+ String nativeUserHome = MsysPaths.toNativePath(userHome);
+ if (!Objects.equals(userHome, nativeUserHome)) {
+ System.setProperty("user.home", nativeUserHome);
+ context.logger.debug("Converted user.home '" + userHome + "'
to '" + nativeUserHome + "'");
+ }
Review Comment:
⚠️ **Incomplete normalization** — `MavenCli` also converts
`maven.repo.local`, `user.settings`, `alternateSettings`, and `user.toolchains`
when those are passed via `-D` (see its `initialize()` method). This block only
converts `user.home`; `localRepositoryPath()` below handles the local-repo case
separately, but the settings/toolchains paths are left untouched.
If a user runs `mvn -Duser.settings=/c/Users/alice/.m2/settings.xml`, the
conversion won't fire through `LookupInvoker` and the path will remain in POSIX
form.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/MsysPaths.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.cling.invoker;
+
+import java.util.Locale;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Converts the paths MSYS2 and Git Bash hand over in their POSIX style, like
{@code /c/Users/alice}, to native Windows
+ * paths, like {@code C:\Users\alice}.
+ */
+final class MsysPaths {
+
+ /** A drive letter, the only single letter directory right below the root
of such a path. */
+ private static final Pattern DRIVE_PATH =
Pattern.compile("^/([a-zA-Z])/(.*)$");
+
+ private MsysPaths() {}
+
+ /**
+ * Converts the path on Windows, and returns it as it is anywhere else.
+ *
+ * @param path the path, may be {@code null}
+ * @return the native path, or {@code null} if the path is {@code null}
+ */
+ static String toNativePath(String path) {
+ return toNativePath(path, isWindows());
+ }
+
+ static String toNativePath(String path, boolean windows) {
Review Comment:
💡 **Missing Javadoc** — the public-facing overload `toNativePath(String)`
has Javadoc, but this package-private `toNativePath(String, boolean)` overload
(used directly in tests) has none. A brief `@param windows` explanation would
help future readers understand why the boolean exists.
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -441,6 +461,25 @@ void initialize(CliRequest cliRequest) throws
ExitException {
//
String mavenHome = System.getProperty(Constants.MAVEN_HOME);
+ if (System.getProperty("os.name",
"").toLowerCase(Locale.ROOT).startsWith("windows")) {
+ final String userHome = System.getProperty("user.home");
+ if (userHome != null) {
+ System.setProperty("user.home", msysToWindowsPath(userHome));
+ }
+
+ for (final String key : new String[] {
+ Constants.MAVEN_REPO_LOCAL, // -Dmaven.repo.local
+ "user.settings", // -s / --settings
+ "alternateSettings",
+ "user.toolchains"
+ }) {
+ final String value = System.getProperty(key);
+ if (value != null) {
+ System.setProperty(key, msysToWindowsPath(value));
+ }
+ }
+ }
Review Comment:
💡 **Compat layer / backport** — `compat/maven-embedder` is the deprecated
Maven 3.x compatibility module. This is useful for Maven 4 users entering via
the legacy `MavenCli` embedder API, but Maven 3.9.x users (who still run native
`MavenCli`) won't get the fix. Worth considering a backport PR against
`maven-3.9.x`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
