You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/05/11 14:30:33 UTC

[GitHub] [maven] cstamas opened a new pull request, #735: Trasnport: use config properties instead user properties.

cstamas opened a new pull request, #735:
URL: https://github.com/apache/maven/pull/735

   This setting should be possible to be set via MAVEN_OPTS (system wide) or in .mvn/maven.config etc, Currently it is possible only via `-Dxxx` AFTER mvn....


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870431077


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   IMHO this is correct change: we do want to allow setting transport in various ways, allowing the change to be "persistent". What we have on master works ONLY if you use `-Dmaven.resolver.transport=xx` on CLI and nothing else.



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870393670


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   yup, this is "config" properties (merged plus added things). I think I understand from where "request.systemProperties" are sourced, but from where are "request.userProperties" sourced?



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] slawekjaranowski commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870423542


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   `request.*Properties` is populated in 
   https://github.com/apache/maven/blob/master/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870388634


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   So this is a merged, final version of the properties and the actual source is irrelevant?



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas closed pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
cstamas closed pull request #735: Trasnport: use config properties instead user properties.
URL: https://github.com/apache/maven/pull/735


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870431077


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   IMHO this is correct change: we do want to allow setting transport in various ways, allowing the change to be "persistent". What we have on master works ONLY if you use `-Dmaven.resolver.transport=xx` on CLI and nothing else. This also allows CLI override (ie. you have it set in MAVEN_OPTS but you use `-Dxxx` on command line)



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870405282


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   Look into Maven CLI, that is the source for it. Commons CLI provides it and it is passed down the line.



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #735:
URL: https://github.com/apache/maven/pull/735#issuecomment-1125708120

   Superseded by https://github.com/apache/maven/pull/739


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] slawekjaranowski commented on a diff in pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #735:
URL: https://github.com/apache/maven/pull/735#discussion_r870423542


##########
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java:
##########
@@ -261,8 +261,7 @@ else if ( request.isUpdateSnapshots() )
         }
         session.setAuthenticationSelector( authSelector );
 
-        String transport = request.getUserProperties()
-                .getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );

Review Comment:
   `request.*Properties` are populated in 
   https://github.com/apache/maven/blob/master/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #735:
URL: https://github.com/apache/maven/pull/735#issuecomment-1124295612

   Do we want to address build consumer feature as well? As it suffers from very same issue


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #735: Trasnport: use config properties instead user properties.

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #735:
URL: https://github.com/apache/maven/pull/735#issuecomment-1124350714

   No, at least not for now. It is a user feature  therefore a user property 


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org