You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/02/21 15:17:53 UTC

[GitHub] [maven] cstamas opened a new pull request, #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

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

   This PR deprecates the 'localRepository' mojo parameter expression, and Core will emit warning if used by any Mojo.
   
   ---
   
   https://issues.apache.org/jira/browse/MNG-7706


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1009:
URL: https://github.com/apache/maven/pull/1009#discussion_r1113615727


##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.plugin.internal;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.HashMap;
+
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.apache.maven.plugin.descriptor.Parameter;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
+import org.codehaus.plexus.configuration.PlexusConfiguration;
+
+/**
+ * Print warnings if deprecated core parameters are used in mojo.
+ *
+ * @since 3.9.1
+ */
+@Singleton
+@Named
+class DeprecatedCoreExpressionValidator extends AbstractMavenPluginParametersValidator {
+    private static final HashMap<String, String> DEPRECATED_CORE_PARAMETERS;
+
+    static {
+        HashMap<String, String> deprecatedCoreParameters = new HashMap<>();
+        deprecatedCoreParameters.put(
+                "localRepository", "Switch to '${repositorySystemSession}' and get LRM from there instead.");

Review Comment:
   user can also provide as parameter: 
   `session.localRepository`, `session.localRepository.basedir`
   
   
   
   
   



-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1492128884

   Moreover, please use mailing lists for these sort of questions, not old/closed pull requests https://maven.apache.org/mailing-lists.html


-- 
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] inad9300 commented on pull request #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "inad9300 (via GitHub)" <gi...@apache.org>.
inad9300 commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1491980697

   In practical terms, what does this mean for people with `<localRepository>` in their `settings.xml`? What are we supposed to do now?


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1492113730

   As description says "This PR deprecates the 'localRepository' **mojo parameter** expression". Has nothing to do with `settings.xml`. Don't mix the two.


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1009:
URL: https://github.com/apache/maven/pull/1009#discussion_r1113589571


##########
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluator.java:
##########
@@ -40,7 +40,7 @@
  * <tr><td><code>session</code></td>           <td></td>               <td>the actual {@link MavenSession}</td></tr>
  * <tr><td><code>session.*</code></td>         <td>(since Maven 3)</td><td></td></tr>
  * <tr><td><code>localRepository</code></td>   <td></td>
- *                                             <td>{@link MavenSession#getLocalRepository()}</td></tr>
+ *                                             <td>{@link MavenSession#getLocalRepository()} DEPRECATED</td></tr>

Review Comment:
   info about replacement can be useful here



-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1492359514

   Created https://issues.apache.org/jira/browse/MNG-7754


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1009:
URL: https://github.com/apache/maven/pull/1009#discussion_r1113941033


##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.plugin.internal;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.HashMap;
+
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.apache.maven.plugin.descriptor.Parameter;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
+import org.codehaus.plexus.configuration.PlexusConfiguration;
+
+/**
+ * Print warnings if deprecated core parameters are used in mojo.
+ *
+ * @since 3.9.1
+ */
+@Singleton
+@Named
+class DeprecatedCoreExpressionValidator extends AbstractMavenPluginParametersValidator {
+    private static final HashMap<String, String> DEPRECATED_CORE_PARAMETERS;
+
+    static {
+        HashMap<String, String> deprecatedCoreParameters = new HashMap<>();
+        deprecatedCoreParameters.put(
+                "localRepository", "Switch to '${repositorySystemSession}' and get LRM from there instead.");

Review Comment:
   Note: this is ONLY about `ArtifactRepository` type, hence, basedir (File) is okay



-- 
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] laeubi commented on a diff in pull request #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "laeubi (via GitHub)" <gi...@apache.org>.
laeubi commented on code in PR #1009:
URL: https://github.com/apache/maven/pull/1009#discussion_r1115253011


##########
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluator.java:
##########
@@ -40,7 +40,7 @@
  * <tr><td><code>session</code></td>           <td></td>               <td>the actual {@link MavenSession}</td></tr>
  * <tr><td><code>session.*</code></td>         <td>(since Maven 3)</td><td></td></tr>
  * <tr><td><code>localRepository</code></td>   <td></td>
- *                                             <td>{@link MavenSession#getLocalRepository()}</td></tr>
+ *                                             <td>{@link MavenSession#getLocalRepository()} DEPRECATED: Avoid use of {@link org.apache.maven.artifact.repository.ArtifactRepository} type. If you need access to local repository, switch to '${repositorySystemSession}' expression and get LRM from it instead.</td></tr>

Review Comment:
   I think some Plugin deleopers might have no clue what LRM is, probabbly also give a link to the wiki/jira/... that explains some details.



-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1009:
URL: https://github.com/apache/maven/pull/1009#discussion_r1115514268


##########
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluator.java:
##########
@@ -40,7 +40,7 @@
  * <tr><td><code>session</code></td>           <td></td>               <td>the actual {@link MavenSession}</td></tr>
  * <tr><td><code>session.*</code></td>         <td>(since Maven 3)</td><td></td></tr>
  * <tr><td><code>localRepository</code></td>   <td></td>
- *                                             <td>{@link MavenSession#getLocalRepository()}</td></tr>
+ *                                             <td>{@link MavenSession#getLocalRepository()} DEPRECATED: Avoid use of {@link org.apache.maven.artifact.repository.ArtifactRepository} type. If you need access to local repository, switch to '${repositorySystemSession}' expression and get LRM from it instead.</td></tr>

Review Comment:
   Updated javadoc, added link to the issue. Issue updated with "how to fix" https://issues.apache.org/jira/browse/MNG-7706



-- 
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 merged pull request #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #1009:
URL: https://github.com/apache/maven/pull/1009


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1009:
URL: https://github.com/apache/maven/pull/1009#discussion_r1113938714


##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.plugin.internal;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.HashMap;
+
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.apache.maven.plugin.descriptor.Parameter;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
+import org.codehaus.plexus.configuration.PlexusConfiguration;
+
+/**
+ * Print warnings if deprecated core parameters are used in mojo.
+ *
+ * @since 3.9.1
+ */
+@Singleton
+@Named
+class DeprecatedCoreExpressionValidator extends AbstractMavenPluginParametersValidator {
+    private static final HashMap<String, String> DEPRECATED_CORE_PARAMETERS;
+
+    static {
+        HashMap<String, String> deprecatedCoreParameters = new HashMap<>();
+        deprecatedCoreParameters.put(
+                "localRepository", "Switch to '${repositorySystemSession}' and get LRM from there instead.");

Review Comment:
   True! Fixing this



-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1438703907

   Ironically, ITs report this:
   ```
   Error:  Failures: 
   Warning: Tmng5576CdFriendlyVersions.testContinuousDeliveryFriendlyVersionsAreWarningFreeWithBuildConsumer:103->AbstractMavenIntegrationTestCase.assertFalse:703 [WARNING] Parameter 'localRepository' is deprecated core expression; Switch to '${repositorySystemSession}' and get LRM from there instead. ==> expected: <false> but was: <true>
   Warning: Tmng5576CdFriendlyVersions.testContinuousDeliveryFriendlyVersionsAreWarningFreeWithoutBuildConsumer:71->AbstractMavenIntegrationTestCase.assertFalse:703 [WARNING] Parameter 'localRepository' is deprecated core expression; Switch to '${repositorySystemSession}' and get LRM from there instead. ==> expected: <false> but was: <true>
   [INFO] 
   Error:  Tests run: 890, Failures: 2, Errors: 0, Skipped: 84
   ```


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1440791991

   Before this is merged, we need to do something with ITs:
   * either fix IT plugins, but am afraid that will start unrolling a snow ball of changes
   * fix the failing 2 IT to be more specific about warning it listens for...


-- 
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 #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1492325961

   This is a project maintained by volunteers, hence you can contribute as well! Feel free to produce as many quality patches and other improvements project-wise as you like! We will be glad to review them 😺 


-- 
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] inad9300 commented on pull request #1009: [MNG-7706] Deprecate 'localRepository' parameter expression

Posted by "inad9300 (via GitHub)" <gi...@apache.org>.
inad9300 commented on PR #1009:
URL: https://github.com/apache/maven/pull/1009#issuecomment-1492298862

   I upgrade to 3.9.x, I see a warning, and I have a single place where I use "localRepository". Maybe make messages more clear, make release notes more clear, and upgrade your communication channels to something more usable.


-- 
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