[ 
https://issues.apache.org/jira/browse/MNG-7914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17865165#comment-17865165
 ] 

ASF GitHub Bot commented on MNG-7914:
-------------------------------------

michael-o commented on code in PR #1595:
URL: https://github.com/apache/maven/pull/1595#discussion_r1674377952


##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -510,7 +503,8 @@ void logging(CliRequest cliRequest) {
         cliRequest.showErrors = cliRequest.verbose || 
commandLine.hasOption(CLIManager.ERRORS);
 
         // LOG COLOR
-        String styleColor = 
cliRequest.getUserProperties().getProperty(STYLE_COLOR_PROPERTY, "auto");
+        String styleColor = 
cliRequest.getUserProperties().getProperty("style.color", "auto");
+        styleColor = 
cliRequest.getUserProperties().getProperty(Constants.MAVEN_STYLE_COLOR_PROPERTY,
 styleColor);

Review Comment:
   Is that on purpose?



##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -1204,40 +1203,57 @@ void toolchains(CliRequest cliRequest) throws Exception 
{
                         "The specified user toolchains file does not exist: " 
+ userToolchainsFile);
             }
         } else {
-            userToolchainsFile = DEFAULT_USER_TOOLCHAINS_FILE;
+            String userToolchainsFileStr = 
cliRequest.getUserProperties().getProperty(Constants.MAVEN_USER_TOOLCHAINS);
+            if (userToolchainsFileStr != null) {
+                userToolchainsFile = new File(userToolchainsFileStr);
+            }
         }
 
-        File globalToolchainsFile;
+        File installationToolchainsFile = null;
 
-        if 
(cliRequest.commandLine.hasOption(CLIManager.ALTERNATE_GLOBAL_TOOLCHAINS)) {
-            globalToolchainsFile =
+        if 
(cliRequest.commandLine.hasOption(CLIManager.ALTERNATE_INSTALLATION_TOOLCHAINS))
 {
+            installationToolchainsFile =
+                    new 
File(cliRequest.commandLine.getOptionValue(CLIManager.ALTERNATE_INSTALLATION_TOOLCHAINS));
+            installationToolchainsFile = 
resolveFile(installationToolchainsFile, cliRequest.workingDirectory);
+
+            if (!installationToolchainsFile.isFile()) {
+                throw new FileNotFoundException(
+                        "The specified installation toolchains file does not 
exist: " + installationToolchainsFile);
+            }
+        } else if 
(cliRequest.commandLine.hasOption(CLIManager.ALTERNATE_GLOBAL_TOOLCHAINS)) {
+            installationToolchainsFile =
                     new 
File(cliRequest.commandLine.getOptionValue(CLIManager.ALTERNATE_GLOBAL_TOOLCHAINS));
-            globalToolchainsFile = resolveFile(globalToolchainsFile, 
cliRequest.workingDirectory);
+            installationToolchainsFile = 
resolveFile(installationToolchainsFile, cliRequest.workingDirectory);
 
-            if (!globalToolchainsFile.isFile()) {
+            if (!installationToolchainsFile.isFile()) {
                 throw new FileNotFoundException(
-                        "The specified global toolchains file does not exist: 
" + globalToolchainsFile);
+                        "The specified installation toolchains file does not 
exist: " + installationToolchainsFile);
             }
         } else {
-            globalToolchainsFile = DEFAULT_GLOBAL_TOOLCHAINS_FILE;
+            String installationToolchainsFileStr =
+                    
cliRequest.getUserProperties().getProperty(Constants.MAVEN_INSTALLATION_TOOLCHAINS);
+            if (installationToolchainsFileStr != null) {
+                installationToolchainsFile = new 
File(installationToolchainsFileStr);
+                installationToolchainsFile = 
resolveFile(installationToolchainsFile, cliRequest.workingDirectory);
+            }
         }
 
-        cliRequest.request.setGlobalToolchainsFile(globalToolchainsFile);
+        
cliRequest.request.setInstallationToolchainsFile(installationToolchainsFile);
         cliRequest.request.setUserToolchainsFile(userToolchainsFile);
 
         DefaultToolchainsBuildingRequest toolchainsRequest = new 
DefaultToolchainsBuildingRequest();
-        if (globalToolchainsFile.isFile()) {
-            toolchainsRequest.setGlobalToolchainsSource(new 
FileSource(globalToolchainsFile));
+        if (installationToolchainsFile != null && 
installationToolchainsFile.isFile()) {
+            toolchainsRequest.setGlobalToolchainsSource(new 
FileSource(installationToolchainsFile));
         }
-        if (userToolchainsFile.isFile()) {
+        if (userToolchainsFile != null && userToolchainsFile.isFile()) {
             toolchainsRequest.setUserToolchainsSource(new 
FileSource(userToolchainsFile));
         }
 
         eventSpyDispatcher.onEvent(toolchainsRequest);
 
         slf4jLogger.debug(
-                "Reading global toolchains from '{}'",
-                getLocation(toolchainsRequest.getGlobalToolchainsSource(), 
globalToolchainsFile));
+                "Reading installation toolchains from '{}'",
+                getLocation(toolchainsRequest.getGlobalToolchainsSource(), 
installationToolchainsFile));

Review Comment:
   Do you intend to deprecate old getters/setters as well (later)?



##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -874,9 +873,9 @@ private static <T> List<T> reverse(List<T> list) {
     }
 
     private List<File> parseExtClasspath(CliRequest cliRequest) {
-        String extClassPath = 
cliRequest.userProperties.getProperty(EXT_CLASS_PATH);
+        String extClassPath = 
cliRequest.userProperties.getProperty(Constants.MAVEN_EXT_CLASS_PATH);
         if (extClassPath == null) {
-            extClassPath = 
cliRequest.systemProperties.getProperty(EXT_CLASS_PATH);
+            extClassPath = 
cliRequest.systemProperties.getProperty(Constants.MAVEN_EXT_CLASS_PATH);

Review Comment:
   Can we add a note whether this should still be system property? If we are 
already in Java then we obviously don't need a system property at all, only 
user one.
   
   See: 
https://github.com/apache/maven/pull/1595/files#diff-b8b814f5b6b3855ae79dc45a0266b94871193dcef42803c316e07409e94af35cR1404





> Provide a single entry point for configuration
> ----------------------------------------------
>
>                 Key: MNG-7914
>                 URL: https://issues.apache.org/jira/browse/MNG-7914
>             Project: Maven
>          Issue Type: New Feature
>            Reporter: Guillaume Nodet
>            Priority: Major
>             Fix For: 4.0.x-candidate
>
>
> Looking at MNG-7772, this should not require any code change, but it's all 
> about configuration.
> I propose to load / interpolate the following files:
>  * {{${maven.home}/conf/maven.user.properties}}
>  * {{${maven.home}/conf/maven.system.properties}}
> Those files would be used to load additional user properties and system 
> properties for Maven.  In addition to the simple interpolation mechanism, we 
> should provide two enhancements using special keys {{{}$\{includes{}}}} and 
> {{{}$\{optionals{}}}} which would be used to load additional referenced 
> configuration files such as:
> {{    ${optionals} = ${user.home}/.m2/maven.user.properties, 
> ${session.rootDirectory}/.mvn/maven.user.properties}}
> Being loaded early when Maven is loaded, those files could reference 
> directories to load extensions from:
>   {{{}maven.core.extensions.directories = 
> ${session.rootDirectory}/.mvn/extensions.xml,{}}}{{{}${user.home}/.m2/extensions.xml,${maven.home}/extensions.xml{}}}
>  
> In various places, the maven code could be simplified and offer more 
> configuration points at the same time.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to