[MNG-8302] Warn when appropriate (#1810)

First, `rootDirectory` is nullable, CLIng code was not assuming this, fixed.
Second, emit the "no root found" warning ONLY when appropriate (when we have a POM in picture).

---

https://issues.apache.org/jira/browse/MNG-8302

api/maven-api-cli/src/main/java/org/apache/maven/api/cli/InvokerRequest.java

@@ -125,13 +125,14 @@ public interface InvokerRequest<O extends Options> {
     Path topDirectory();
 
     /**
-     * Returns the root directory of the Maven invocation.
+     * Returns the root directory of the Maven invocation, if found. This is determined by the presence of a
-     * This is determined by the presence of a .mvn directory or a POM with the root="true" property.
+     * {@code .mvn} directory or a POM with the root="true" property but is not always applicable (ie invocation
+     * from outside a checkout).
      *
-     * @return the root directory path
+     * @return the root directory path, if present
      */
     @Nonnull
-    Path rootDirectory();
+    Optional<Path> rootDirectory();
 
     /**
      * Returns the input stream for the Maven execution, if running in embedded mode.

maven-api-impl/src/main/java/org/apache/maven/api/services/model/RootLocator.java

@@ -22,6 +22,7 @@ import java.nio.file.Path;
 
 import org.apache.maven.api.Service;
 import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
 
 /**
  * Interface used to locate the root directory for a given project.
@@ -40,6 +41,9 @@ public interface RootLocator extends Service {
             + "Create a .mvn directory in the root directory or add the root=\"true\""
             + " attribute on the root project's model to identify it.";
 
+    @Nullable
+    Path findRoot(@Nonnull Path basedir);
+
     @Nonnull
     Path findMandatoryRoot(@Nonnull Path basedir);
 

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/rootlocator/DefaultRootLocator.java

@@ -46,13 +46,19 @@ public class DefaultRootLocator implements RootLocator {
                 .toList();
     }
 
-    @Nonnull
+    @Override
-    public Path findMandatoryRoot(@Nonnull Path basedir) {
+    public Path findRoot(Path basedir) {
         requireNonNull(basedir, "basedir is null");
         Path rootDirectory = basedir;
         while (rootDirectory != null && !isRootDirectory(rootDirectory)) {
             rootDirectory = rootDirectory.getParent();
         }
+        return rootDirectory;
+    }
+
+    @Nonnull
+    public Path findMandatoryRoot(@Nonnull Path basedir) {
+        Path rootDirectory = findRoot(basedir);
         Optional<Path> rdf = getRootDirectoryFallback();
         if (rootDirectory == null) {
             rootDirectory = rdf.orElseThrow(() -> new IllegalStateException(getNoRootMessage()));

maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseInvokerRequest.java

@@ -114,8 +114,8 @@ public abstract class BaseInvokerRequest<T extends Options> implements InvokerRe
     }
 
     @Override
-    public Path rootDirectory() {
+    public Optional<Path> rootDirectory() {
-        return rootDirectory;
+        return Optional.ofNullable(rootDirectory);
     }
 
     @Override

maven-cli/src/main/java/org/apache/maven/cling/invoker/BaseParser.java

@@ -32,17 +32,16 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.ServiceLoader;
 import java.util.function.Function;
 
 import org.apache.maven.api.Constants;
+import org.apache.maven.api.annotations.Nullable;
 import org.apache.maven.api.cli.InvokerRequest;
 import org.apache.maven.api.cli.Options;
 import org.apache.maven.api.cli.Parser;
 import org.apache.maven.api.cli.ParserException;
 import org.apache.maven.api.cli.ParserRequest;
 import org.apache.maven.api.cli.extensions.CoreExtension;
-import org.apache.maven.api.services.model.RootLocator;
 import org.apache.maven.cli.CLIReportingUtils;
 import org.apache.maven.cli.internal.extension.io.CoreExtensionsStaxReader;
 import org.apache.maven.cli.props.MavenPropertiesLoader;
@@ -76,14 +75,19 @@ public abstract class BaseParser<O extends Options, R extends InvokerRequest<O>>
         public Map<String, String> systemProperties;
         public Map<String, String> userProperties;
         public Path topDirectory;
+
+        @Nullable
         public Path rootDirectory;
+
         public List<CoreExtension> extensions;
         public Options options;
 
         public Map<String, String> extraInterpolationSource() {
             Map<String, String> extra = new HashMap<>();
             extra.put("session.topDirectory", topDirectory.toString());
-            extra.put("session.rootDirectory", rootDirectory.toString());
+            if (rootDirectory != null) {
+                extra.put("session.rootDirectory", rootDirectory.toString());
+            }
             return extra;
         }
     }
@@ -101,7 +105,7 @@ public abstract class BaseParser<O extends Options, R extends InvokerRequest<O>>
 
         // top/root
         context.topDirectory = requireNonNull(getTopDirectory(context));
-        context.rootDirectory = requireNonNull(getRootDirectory(context));
+        context.rootDirectory = getRootDirectory(context);
 
         // options
         List<O> parsedOptions = parseCliOptions(context);
@@ -194,11 +198,9 @@ public abstract class BaseParser<O extends Options, R extends InvokerRequest<O>>
         return getCanonicalPath(topDirectory);
     }
 
+    @Nullable
     protected Path getRootDirectory(LocalContext context) throws ParserException {
-        return getCanonicalPath(ServiceLoader.load(RootLocator.class)
+        return Utils.findRoot(context.topDirectory);
-                .iterator()
-                .next()
-                .findMandatoryRoot(requireNonNull(context.topDirectory)));
     }
 
     protected Map<String, String> populateSystemProperties(LocalContext context) throws ParserException {

maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java

@@ -643,11 +643,13 @@ public abstract class LookupInvoker<
         request.setBaseDirectory(context.invokerRequest.topDirectory().toFile());
         request.setSystemProperties(toProperties(context.invokerRequest.systemProperties()));
         request.setUserProperties(toProperties(context.invokerRequest.userProperties()));
-        request.setMultiModuleProjectDirectory(
-                context.invokerRequest.rootDirectory().toFile());
 
-        request.setRootDirectory(context.invokerRequest.rootDirectory());
         request.setTopDirectory(context.invokerRequest.topDirectory());
+        if (context.invokerRequest.rootDirectory().isPresent()) {
+            request.setMultiModuleProjectDirectory(
+                    context.invokerRequest.rootDirectory().get().toFile());
+            request.setRootDirectory(context.invokerRequest.rootDirectory().get());
+        }
 
         request.addPluginGroup("org.apache.maven.plugins");
         request.addPluginGroup("org.codehaus.mojo");

maven-cli/src/main/java/org/apache/maven/cling/invoker/Utils.java

@@ -24,9 +24,14 @@ import java.nio.file.Path;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Properties;
+import java.util.ServiceLoader;
 import java.util.function.Function;
 
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+import org.apache.maven.api.services.model.RootLocator;
 import org.apache.maven.cli.logging.Slf4jConfiguration;
 import org.apache.maven.execution.MavenExecutionRequest;
 import org.codehaus.plexus.interpolation.AbstractValueSource;
@@ -42,6 +47,7 @@ import static java.util.Objects.requireNonNull;
 public final class Utils {
     private Utils() {}
 
+    @Nullable
     public static File toFile(Path path) {
         if (path != null) {
             return path.toFile();
@@ -49,6 +55,7 @@ public final class Utils {
         return null;
     }
 
+    @Nonnull
     public static String stripLeadingAndTrailingQuotes(String str) {
         requireNonNull(str, "str");
         final int length = str.length();
@@ -61,6 +68,7 @@ public final class Utils {
         return str;
     }
 
+    @Nonnull
     public static Path getCanonicalPath(Path path) {
         requireNonNull(path, "path");
         try {
@@ -70,6 +78,7 @@ public final class Utils {
         }
     }
 
+    @Nonnull
     public static Map<String, String> toMap(Properties properties) {
         requireNonNull(properties, "properties");
         HashMap<String, String> map = new HashMap<>();
@@ -79,6 +88,7 @@ public final class Utils {
         return map;
     }
 
+    @Nonnull
     public static Properties toProperties(Map<String, String> properties) {
         requireNonNull(properties, "properties");
         Properties map = new Properties();
@@ -88,6 +98,7 @@ public final class Utils {
         return map;
     }
 
+    @Nonnull
     public static BasicInterpolator createInterpolator(Collection<Map<String, String>> properties) {
         StringSearchInterpolator interpolator = new StringSearchInterpolator();
         interpolator.addValueSource(new AbstractValueSource(false) {
@@ -105,6 +116,7 @@ public final class Utils {
         return interpolator;
     }
 
+    @Nonnull
     public static Function<String, String> prefix(String prefix, Function<String, String> cb) {
         return s -> {
             String v = null;
@@ -115,6 +127,8 @@ public final class Utils {
         };
     }
 
+    @SafeVarargs
+    @Nonnull
     public static Function<String, String> or(Function<String, String>... callbacks) {
         return s -> {
             for (Function<String, String> cb : callbacks) {
@@ -144,4 +158,23 @@ public final class Utils {
             case ERROR -> Logger.LEVEL_ERROR;
         };
     }
+
+    @Nullable
+    public static Path findRoot(Path topDirectory) {
+        requireNonNull(topDirectory, "topDirectory");
+        Path rootDirectory =
+                ServiceLoader.load(RootLocator.class).iterator().next().findRoot(topDirectory);
+        if (rootDirectory != null) {
+            return getCanonicalPath(rootDirectory);
+        }
+        return null;
+    }
+
+    @Nonnull
+    public static Path findMandatoryRoot(Path topDirectory) {
+        requireNonNull(topDirectory, "topDirectory");
+        return getCanonicalPath(Optional.ofNullable(
+                        ServiceLoader.load(RootLocator.class).iterator().next().findMandatoryRoot(topDirectory))
+                .orElseThrow());
+    }
 }

maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/BaseMavenParser.java

@@ -42,8 +42,8 @@ public abstract class BaseMavenParser<O extends MavenOptions, R extends MavenInv
         // CLI args
         result.add(parseMavenCliOptions(context.parserRequest.args()));
         // maven.config; if exists
-        Path mavenConfig = context.rootDirectory.resolve(".mvn/maven.config");
+        Path mavenConfig = context.rootDirectory != null ? context.rootDirectory.resolve(".mvn/maven.config") : null;
-        if (Files.isRegularFile(mavenConfig)) {
+        if (mavenConfig != null && Files.isRegularFile(mavenConfig)) {
             result.add(parseMavenConfigOptions(mavenConfig));
         }
         return result;

maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/DefaultMavenInvoker.java

@@ -43,6 +43,7 @@ import org.apache.maven.cli.CLIReportingUtils;
 import org.apache.maven.cli.event.ExecutionEventLogger;
 import org.apache.maven.cling.invoker.LookupInvoker;
 import org.apache.maven.cling.invoker.ProtoLookup;
+import org.apache.maven.cling.invoker.Utils;
 import org.apache.maven.eventspy.internal.EventSpyDispatcher;
 import org.apache.maven.exception.DefaultExceptionHandler;
 import org.apache.maven.exception.ExceptionHandler;
@@ -259,6 +260,12 @@ public abstract class DefaultMavenInvoker<
     @Override
     protected void populateRequest(C context, MavenExecutionRequest request) throws Exception {
         super.populateRequest(context, request);
+        if (context.invokerRequest.rootDirectory().isEmpty()) {
+            // maven requires this to be set; so default it (and see below at POM)
+            request.setMultiModuleProjectDirectory(
+                    context.invokerRequest.topDirectory().toFile());
+            request.setRootDirectory(context.invokerRequest.topDirectory());
+        }
 
         MavenOptions options = context.invokerRequest.options();
         request.setNoSnapshotUpdates(options.suppressSnapshotUpdates().orElse(false));
@@ -270,15 +277,24 @@ public abstract class DefaultMavenInvoker<
         request.setGlobalChecksumPolicy(determineGlobalChecksumPolicy(context));
 
         Path pom = determinePom(context);
-        request.setPom(pom != null ? pom.toFile() : null);
+        if (pom != null) {
+            request.setPom(pom.toFile());
+            if (pom.getParent() != null) {
+                request.setBaseDirectory(pom.getParent().toFile());
+            }
+
+            // project present, but we could not determine rootDirectory: extra work needed
+            if (context.invokerRequest.rootDirectory().isEmpty()) {
+                Path rootDirectory = Utils.findMandatoryRoot(context.invokerRequest.topDirectory());
+                request.setMultiModuleProjectDirectory(rootDirectory.toFile());
+                request.setRootDirectory(rootDirectory);
+            }
+        }
+
         request.setTransferListener(
                 determineTransferListener(context, options.noTransferProgress().orElse(false)));
         request.setExecutionListener(determineExecutionListener(context));
 
-        if ((request.getPom() != null) && (request.getPom().getParentFile() != null)) {
-            request.setBaseDirectory(request.getPom().getParentFile());
-        }
-
         request.setResumeFrom(options.resumeFrom().orElse(null));
         request.setResume(options.resume().orElse(false));
         request.setMakeBehavior(determineMakeBehavior(context));