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

[GitHub] [maven-compiler-plugin] psiroky opened a new pull request, #173: [MCOMPILER-395] Support exclusions for 'annotationProcessorPaths'

psiroky opened a new pull request, #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173

   * this should be quite straightforward to review, since the plugin only needs to "push" the exclusions down to the maven-resolver and all the "magic" happens there
   
   
   Following this checklist to help us incorporate your 
   contribution quickly and easily:
   
    - [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MCOMPILER) filed 
          for the change (usually before you start working on it).  Trivial changes like typos do not 
          require a JIRA issue.  Your pull request should address just this issue, without 
          pulling in other changes.
    - [x] Each commit in the pull request should have a meaningful subject line and body.
    - [x] Format the pull request title like `[MCOMPILER-XXX] - Fixes bug in ApproximateQuantiles`,
          where you replace `MCOMPILER-XXX` with the appropriate JIRA issue. Best practice
          is to use the JIRA issue title in the pull request title and in the first line of the 
          commit message.
    - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will 
          be performed on your pull request automatically.
    - [x] You have run the integration tests successfully (`mvn -Prun-its clean verify`).
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under 
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [x] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   


-- 
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-compiler-plugin] gnodet merged pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173


-- 
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-compiler-plugin] gnodet commented on a diff in pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1101953132


##########
src/it/setup_annotation-verify-plugin/src/main/java/org.apache.maven.plugins.compiler.it/SourcePathReadGoal.java:
##########
@@ -38,15 +38,17 @@
 {
 
     @Parameter
-    protected String sourceClass;
+    private String sourceClass;
 
     @Parameter
-    protected String testSourceClass;
+    private String testSourceClass;
 
     @Parameter( defaultValue = "${project}" )
-    protected MavenProject project;
+    private MavenProject project;
+
+    @Parameter( defaultValue = "${project.build.sourceEncoding}" )
+    private String encoding;

Review Comment:
   Can this be moved in other PR ? 



-- 
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-compiler-plugin] psiroky commented on a diff in pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1102441906


##########
src/it/setup_annotation-verify-plugin/src/main/java/org.apache.maven.plugins.compiler.it/SourcePathReadGoal.java:
##########
@@ -38,15 +38,17 @@
 {
 
     @Parameter
-    protected String sourceClass;
+    private String sourceClass;
 
     @Parameter
-    protected String testSourceClass;
+    private String testSourceClass;
 
     @Parameter( defaultValue = "${project}" )
-    protected MavenProject project;
+    private MavenProject project;
+
+    @Parameter( defaultValue = "${project.build.sourceEncoding}" )
+    private String encoding;

Review Comment:
   Sure, let me remove that.



-- 
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-compiler-plugin] slawekjaranowski commented on a diff in pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

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


##########
src/main/java/org/apache/maven/plugin/compiler/DependencyExclusion.java:
##########
@@ -0,0 +1,114 @@
+package org.apache.maven.plugin.compiler;
+
+/*
+ * 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.
+ */
+
+import java.util.Objects;
+
+/**
+ * Simple representation of a Maven dependency exclusion.
+ */
+public class DependencyExclusion
+{
+    private String groupId;
+
+    private String artifactId;
+
+    private String classifier;
+
+    private String extension = "jar";
+
+    public String getGroupId()
+    {
+        return groupId;
+    }
+
+    public void setGroupId( String groupId )
+    {
+        this.groupId = groupId;
+    }
+
+    public String getArtifactId()
+    {
+        return artifactId;
+    }
+
+    public void setArtifactId( String artifactId )
+    {
+        this.artifactId = artifactId;
+    }
+
+    public String getClassifier()
+    {
+        return classifier;
+    }
+
+    public void setClassifier( String classifier )
+    {
+        this.classifier = classifier;
+    }
+
+    public String getExtension()
+    {
+        return extension;
+    }
+
+    public void setExtension( String extension )
+    {
+        this.extension = extension;
+    }
+
+    @Override
+    public boolean equals( Object obj )
+    {
+        if ( this == obj )
+        {
+            return true;
+        }
+        if ( obj == null || getClass() != obj.getClass() )
+        {
+            return false;
+        }
+        DependencyExclusion other = (DependencyExclusion) obj;
+        return Objects.equals( groupId, other.groupId )
+                && Objects.equals( artifactId, other.artifactId )
+                && Objects.equals( classifier, other.classifier )
+                && Objects.equals( extension, other.extension );
+    }
+
+    @Override
+    public int hashCode()
+    {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ( ( artifactId == null ) ? 0 : artifactId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( groupId == null ) ? 0 : groupId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( extension == null ) ? 0 : extension.hashCode() );

Review Comment:
   looks better



-- 
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-compiler-plugin] psiroky commented on pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#issuecomment-1425558840

   The failed test is `MCOMPILER-481-requires-static-included`, which seems to a [known issue](https://issues.apache.org/jira/browse/MCOMPILER-481?focusedCommentId=17679050&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17679050). The same test fails also on master with Maven 3.9.0. It is possibly related to surefire version 3.0.0.M8 (which is the default in Maven 3.9.0 I believe). Temporary workaround could be to pin down the surefire plugin version to some older one for that specific test case (`2.22.2` seems to be working).


-- 
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-compiler-plugin] slawekjaranowski commented on a diff in pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

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


##########
src/main/java/org/apache/maven/plugin/compiler/DependencyCoordinate.java:
##########
@@ -107,71 +123,17 @@ public boolean equals( Object obj )
         {
             return true;
         }
-        if ( obj == null )
-        {
-            return false;
-        }
-        if ( getClass() != obj.getClass() )
+        if ( obj == null || getClass() != obj.getClass() )
         {
             return false;
         }
         DependencyCoordinate other = (DependencyCoordinate) obj;
-        if ( artifactId == null )
-        {
-            if ( other.artifactId != null )
-            {
-                return false;
-            }
-        }
-        else if ( !artifactId.equals( other.artifactId ) )
-        {
-            return false;
-        }
-        if ( classifier == null )
-        {
-            if ( other.classifier != null )
-            {
-                return false;
-            }
-        }
-        else if ( !classifier.equals( other.classifier ) )
-        {
-            return false;
-        }
-        if ( groupId == null )
-        {
-            if ( other.groupId != null )
-            {
-                return false;
-            }
-        }
-        else if ( !groupId.equals( other.groupId ) )
-        {
-            return false;
-        }
-        if ( type == null )
-        {
-            if ( other.type != null )
-            {
-                return false;
-            }
-        }
-        else if ( !type.equals( other.type ) )
-        {
-            return false;
-        }
-        if ( version == null )
-        {
-            if ( other.version != null )
-            {
-                return false;
-            }
-        }
-        else if ( !version.equals( other.version ) )
-        {
-            return false;
-        }
-        return true;

Review Comment:
   😄 



-- 
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-compiler-plugin] psiroky commented on a diff in pull request #173: [MCOMPILER-395] Support exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1090427466


##########
src/main/java/org/apache/maven/plugin/compiler/DependencyCoordinate.java:
##########
@@ -107,71 +123,17 @@ public boolean equals( Object obj )
         {
             return true;
         }
-        if ( obj == null )
-        {
-            return false;
-        }
-        if ( getClass() != obj.getClass() )
+        if ( obj == null || getClass() != obj.getClass() )
         {
             return false;
         }
         DependencyCoordinate other = (DependencyCoordinate) obj;
-        if ( artifactId == null )
-        {
-            if ( other.artifactId != null )
-            {
-                return false;
-            }
-        }
-        else if ( !artifactId.equals( other.artifactId ) )
-        {
-            return false;
-        }
-        if ( classifier == null )
-        {
-            if ( other.classifier != null )
-            {
-                return false;
-            }
-        }
-        else if ( !classifier.equals( other.classifier ) )
-        {
-            return false;
-        }
-        if ( groupId == null )
-        {
-            if ( other.groupId != null )
-            {
-                return false;
-            }
-        }
-        else if ( !groupId.equals( other.groupId ) )
-        {
-            return false;
-        }
-        if ( type == null )
-        {
-            if ( other.type != null )
-            {
-                return false;
-            }
-        }
-        else if ( !type.equals( other.type ) )
-        {
-            return false;
-        }
-        if ( version == null )
-        {
-            if ( other.version != null )
-            {
-                return false;
-            }
-        }
-        else if ( !version.equals( other.version ) )
-        {
-            return false;
-        }
-        return true;

Review Comment:
   I took the liberty of cleaning-up this monster of `equals()` method. The only difference is that the order of fields that are being compared (e.g. previously it started with `artifactId` and now I moved that to `groupId`). I assume the idea behind this was some sort of optimization (e.g. `artifactId` potentially differs more often than `groupId`) ,but I am not really sure that's worth it. If anyone feels strongly about this, I can of course revert to the previous "state" - the change would be trivial.



-- 
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-compiler-plugin] psiroky commented on pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#issuecomment-1412835375

   @slawekjaranowski thansk, Javadoc updated (I hope this is what you meant by documentation). The rendered page now looks like this:
   ![compiler-plugin-path-exclusions](https://user-images.githubusercontent.com/670547/216179946-9a684435-b680-41ac-a61a-1ea92eef6429.png)
   


-- 
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-compiler-plugin] psiroky commented on a diff in pull request #173: [MCOMPILER-395] Support exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1090432857


##########
src/main/java/org/apache/maven/plugin/compiler/DependencyExclusion.java:
##########
@@ -0,0 +1,114 @@
+package org.apache.maven.plugin.compiler;
+
+/*
+ * 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.
+ */
+
+import java.util.Objects;
+
+/**
+ * Simple representation of a Maven dependency exclusion.
+ */
+public class DependencyExclusion
+{
+    private String groupId;
+
+    private String artifactId;
+
+    private String classifier;
+
+    private String extension = "jar";
+
+    public String getGroupId()
+    {
+        return groupId;
+    }
+
+    public void setGroupId( String groupId )
+    {
+        this.groupId = groupId;
+    }
+
+    public String getArtifactId()
+    {
+        return artifactId;
+    }
+
+    public void setArtifactId( String artifactId )
+    {
+        this.artifactId = artifactId;
+    }
+
+    public String getClassifier()
+    {
+        return classifier;
+    }
+
+    public void setClassifier( String classifier )
+    {
+        this.classifier = classifier;
+    }
+
+    public String getExtension()
+    {
+        return extension;
+    }
+
+    public void setExtension( String extension )
+    {
+        this.extension = extension;
+    }
+
+    @Override
+    public boolean equals( Object obj )
+    {
+        if ( this == obj )
+        {
+            return true;
+        }
+        if ( obj == null || getClass() != obj.getClass() )
+        {
+            return false;
+        }
+        DependencyExclusion other = (DependencyExclusion) obj;
+        return Objects.equals( groupId, other.groupId )
+                && Objects.equals( artifactId, other.artifactId )
+                && Objects.equals( classifier, other.classifier )
+                && Objects.equals( extension, other.extension );
+    }
+
+    @Override
+    public int hashCode()
+    {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ( ( artifactId == null ) ? 0 : artifactId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( groupId == null ) ? 0 : groupId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( extension == null ) ? 0 : extension.hashCode() );

Review Comment:
   I would be very much in favor of using the simplified version like this
   ```
   return Objects.hash( groupId, artifactId, classifier, extension );
   ```
   However, this will create a new array every time the method is called, which could potentially be a performance concern (since currently the hashCode() does not allocate anything). I don't think this method will be used frequently though, since the plugin only creates an instance and then transforms it into Aether Exclusion class.
   
   Would be great to get feedback from others on this.
   
   Edit: I decided to go with the simplified version. If anyone is strongly against that, I will revert those changes.



-- 
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-compiler-plugin] psiroky commented on a diff in pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1102441906


##########
src/it/setup_annotation-verify-plugin/src/main/java/org.apache.maven.plugins.compiler.it/SourcePathReadGoal.java:
##########
@@ -38,15 +38,17 @@
 {
 
     @Parameter
-    protected String sourceClass;
+    private String sourceClass;
 
     @Parameter
-    protected String testSourceClass;
+    private String testSourceClass;
 
     @Parameter( defaultValue = "${project}" )
-    protected MavenProject project;
+    private MavenProject project;
+
+    @Parameter( defaultValue = "${project.build.sourceEncoding}" )
+    private String encoding;

Review Comment:
   Sure, let me remove that.
   
   Edit: Changes in the `SourcePathReadGoal` class removed.



-- 
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-compiler-plugin] psiroky commented on a diff in pull request #173: [MCOMPILER-395] Support exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1090432857


##########
src/main/java/org/apache/maven/plugin/compiler/DependencyExclusion.java:
##########
@@ -0,0 +1,114 @@
+package org.apache.maven.plugin.compiler;
+
+/*
+ * 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.
+ */
+
+import java.util.Objects;
+
+/**
+ * Simple representation of a Maven dependency exclusion.
+ */
+public class DependencyExclusion
+{
+    private String groupId;
+
+    private String artifactId;
+
+    private String classifier;
+
+    private String extension = "jar";
+
+    public String getGroupId()
+    {
+        return groupId;
+    }
+
+    public void setGroupId( String groupId )
+    {
+        this.groupId = groupId;
+    }
+
+    public String getArtifactId()
+    {
+        return artifactId;
+    }
+
+    public void setArtifactId( String artifactId )
+    {
+        this.artifactId = artifactId;
+    }
+
+    public String getClassifier()
+    {
+        return classifier;
+    }
+
+    public void setClassifier( String classifier )
+    {
+        this.classifier = classifier;
+    }
+
+    public String getExtension()
+    {
+        return extension;
+    }
+
+    public void setExtension( String extension )
+    {
+        this.extension = extension;
+    }
+
+    @Override
+    public boolean equals( Object obj )
+    {
+        if ( this == obj )
+        {
+            return true;
+        }
+        if ( obj == null || getClass() != obj.getClass() )
+        {
+            return false;
+        }
+        DependencyExclusion other = (DependencyExclusion) obj;
+        return Objects.equals( groupId, other.groupId )
+                && Objects.equals( artifactId, other.artifactId )
+                && Objects.equals( classifier, other.classifier )
+                && Objects.equals( extension, other.extension );
+    }
+
+    @Override
+    public int hashCode()
+    {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ( ( artifactId == null ) ? 0 : artifactId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( groupId == null ) ? 0 : groupId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( extension == null ) ? 0 : extension.hashCode() );

Review Comment:
   I would be very much in favor of using the simplified version like this
   ```
   return Objects.hash( groupId, artifactId, classifier, extension );
   ```
   However, this will create a new array every time the method is called, which could potentially be a performance concern (since currently the hashCode() does not allocate anything). I don't think this method will be used frequently though, since the plugin only creates an instance and then transforms it into Aether Exclusion class.
   
   Would be great to get feedback from others on 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-compiler-plugin] psiroky commented on a diff in pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on code in PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#discussion_r1090432857


##########
src/main/java/org/apache/maven/plugin/compiler/DependencyExclusion.java:
##########
@@ -0,0 +1,114 @@
+package org.apache.maven.plugin.compiler;
+
+/*
+ * 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.
+ */
+
+import java.util.Objects;
+
+/**
+ * Simple representation of a Maven dependency exclusion.
+ */
+public class DependencyExclusion
+{
+    private String groupId;
+
+    private String artifactId;
+
+    private String classifier;
+
+    private String extension = "jar";
+
+    public String getGroupId()
+    {
+        return groupId;
+    }
+
+    public void setGroupId( String groupId )
+    {
+        this.groupId = groupId;
+    }
+
+    public String getArtifactId()
+    {
+        return artifactId;
+    }
+
+    public void setArtifactId( String artifactId )
+    {
+        this.artifactId = artifactId;
+    }
+
+    public String getClassifier()
+    {
+        return classifier;
+    }
+
+    public void setClassifier( String classifier )
+    {
+        this.classifier = classifier;
+    }
+
+    public String getExtension()
+    {
+        return extension;
+    }
+
+    public void setExtension( String extension )
+    {
+        this.extension = extension;
+    }
+
+    @Override
+    public boolean equals( Object obj )
+    {
+        if ( this == obj )
+        {
+            return true;
+        }
+        if ( obj == null || getClass() != obj.getClass() )
+        {
+            return false;
+        }
+        DependencyExclusion other = (DependencyExclusion) obj;
+        return Objects.equals( groupId, other.groupId )
+                && Objects.equals( artifactId, other.artifactId )
+                && Objects.equals( classifier, other.classifier )
+                && Objects.equals( extension, other.extension );
+    }
+
+    @Override
+    public int hashCode()
+    {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ( ( artifactId == null ) ? 0 : artifactId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( groupId == null ) ? 0 : groupId.hashCode() );
+        result = prime * result + ( ( classifier == null ) ? 0 : classifier.hashCode() );
+        result = prime * result + ( ( extension == null ) ? 0 : extension.hashCode() );

Review Comment:
   ~~I would be very much in favor of using the simplified version like this~~
   ```
   return Objects.hash( groupId, artifactId, classifier, extension );
   ```
   ~~However, this will create a new array every time the method is called, which could potentially be a performance concern (since currently the hashCode() does not allocate anything). I don't think this method will be used frequently though, since the plugin only creates an instance and then transforms it into Aether Exclusion class.~~
   
   Would be great to get feedback from others on this.
   
   Edit: I decided to go with the simplified version. If anyone is strongly against that, I will revert those changes.



-- 
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-compiler-plugin] psiroky commented on pull request #173: [MCOMPILER-395] Allow dependency exclusions for 'annotationProcessorPaths'

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on PR #173:
URL: https://github.com/apache/maven-compiler-plugin/pull/173#issuecomment-1412453523

   @slawekjaranowski @cstamas hello again! Would you guys have some time to review this PR?


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