[GitHub] [maven] adamretter opened a new pull request #357: Add wildcard option for direct goal execution id from command line

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] adamretter opened a new pull request #357: Add wildcard option for direct goal execution id from command line

GitBox

adamretter opened a new pull request #357:
URL: https://github.com/apache/maven/pull/357


   When specifying an execution-id for a goal on the command line (e.g. `mvn plugin:goal@my-execution-id`) also allow wildcard syntax to select all execution ids of the goal (e.g. `mvn plugin:goal@*`).
   
   This builds on the work done previously in https://issues.apache.org/jira/browse/MNG-5768
   
   I don't have a JIRA ticket id for this yet. This is my first exposure to working on the Apache Maven code-base. Before I invest further time, I would like to know if such a feature is desirable for the community?
   
   
   
   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] I already have an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) in place :-)
   


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] rfscholte commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox

rfscholte commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-637123202


   Maven is lifecycle driven. Being able to execute a goal from commandline is a feature, but I don't see the need to execute all available exeucutionIds. Most likely this will have in unexpected due to inheritence.
   More useful would be to describe the usecase, because there might be a better solution.


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] adamretter commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

adamretter commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-637454971


   @rfscholte Okay, so this is my use-case...
   
   I am using the [license-maven-plugin](http://mycila.mathieu.photography/license-maven-plugin/) which allows me to check for and/or format license headers on source-code files. However, I think my use case is equally applicable for any other plugin.
   
   Our projects's source code uses a couple of different licenses. The license plugin enables you to configure it for a single license. Therefore, we need one execution of the plugin's `check` goal per-license that we want to enforce, and we want that bound to the `verify` lifecycle phase.
   
   Therefore we have a configuration which looks something like this:
   ```xml
   <plugin>
       <groupId>com.mycila</groupId>
       <artifactId>license-maven-plugin</artifactId>
       <version>3.0</version>
   
       <configuration>
           <failIfMissing>false</failIfMissing>
           <strictCheck>true</strictCheck>
           <excludes>
               <exclude>LGPL-21-license.template.txt</exclude>
               <exclude>DBXML-license.template.txt</exclude>
           </excludes>
           <encoding>${project.build.sourceEncoding}</encoding>
       </configuration>
   
       <executions>
   
           <!-- Check that the LGPL 2.1 license is present and correct -->
           <execution>
               <id>check-lgpl-headers</id>
               <phase>verify</phase>
               <goals>
                   <goal>check</goal>
               </goals>
               <configuration>
                <header>${project.parent.relativePath}/LGPL-21-license.template.txt</header>
                <excludes>
                <exclude>src/main/java/org/exist/storage/btree/**</exclude>
                </excludes>
               </configuration>
           </execution>
   
           <!-- Check that the DBXML license is present and correct (only on BTree files) -->
           <execution>
               <id>check-dbxml-headers</id>
               <phase>verify</phase>
               <goals>
                   <goal>check</goal>
               </goals>
               <configuration>
                <header>${project.parent.relativePath}/DBXML-license.template.txt</header>
                <includes>
                <include>src/main/java/org/exist/storage/btree/**</exclude>
                </includes>
               </configuration>
           </execution>
   
       </executions>
   </plugin>
   ```
   
   During development and testing we often want to run `mvn license:check` or even `mvn license:format` to ensure that our source code has the correct licenses. Unfortunately that only run's for the *executionId* `default-cli` which equates to  `check-lgpl-headers`, which means that not all source files are correctly checked for the appropriate license headers.
   
   Now I could indeed run `mvn license:check@check-lgpl-headers && mvn license:check@check-dbxml-headers`. But that's not very nice... and also there are actually more than two executions involved, I only showed the two to keep my example short ;-)
   
   Of course, we could just be run `mvn verify`, but we have many other plugins also bound to that lifecycle, and some of them are very slow and intensive. Also that would only execute the `check` goals, it doesn't help us with the `format` goals.
   
   With my PR, running `mvn license:check@*` does correctly run each. It in fact creates one `MojoExecution` per executionId, and for each it merges the parent config with the just the config for that specific execution, so I don't think we have problems with inheritance.
   
   Hope that makes sense? Please let me know if I need to do a better job at explaining it...


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] rfscholte commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

rfscholte commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-637770365


   I don't know the plugin, but I would start there. Ask if they can improve it to handle different licenses.
   Next I would look into profiles, i.e. `mvn validate -Plicenses`, and overwrite the `check` goal to the `validate` phase. You might also do that for the `format` goal, bind it to the `process-sources`.
   
   I think the usage of wildcards is way to dangerous, it would open possibilities we will regret in the future. Let's keep it explicit.


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] adamretter commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

adamretter commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-637778452


   > I don't know the plugin, but I would start there. Ask if they can improve it to handle different licenses.
   
   Sure I can do that, or even send a PR to enable that... yet it seems to me that such a use-case could be appropriate for any plugins not just the license-maven-plugin plugin. For example, within our project:
   
   1. we make similar use of the `maven-dependency-plugin`, binding multiple executions of its `unpack` goal to the `package` phase, so that we can expand various dependencies.
   
   2. likewise with the `copy-rename-maven-plugin` we need multiple executions if we want to copy more that a single source file or folder. Again its goals are bound to the `package` phase.
   
   3. likewise with the `copy-maven-plugin` where we need to copy files to different output directories.
   
   I am interested by your suggestion of using profiles... I am not quite sure how that would work in a concrete manner though? So would I have 2 profiles (one for each license) each that have a separate configured instance of the license-maven-plugin? But then how would I ensure those two profiles are always enabled by default?


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] rfscholte commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

rfscholte commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-637789710


   ```
     <profiles>
       <profile>
         <id>license</id>
         <build>
           <plugins>
             <plugin>
               <groupId>com.mycila</groupId>
               <artifactId>license-maven-plugin</artifactId>
               <executions>
                 <execution>
                   <id>check-lgpl-headers</id>
                   <phase>validate</phase>
                 </execution>
                 <execution>
                   <id>check-dbxml-headers</id>
                   <phase>validate</phase>
                 </execution>
               </executions>
             </plugin>
           </plugins>
         </build>
       </profile>
     </profiles>
   ```
   With this, `mvn verify` will work as usual, doing the check at the end, `mvn validate -Plicense` will just call the `check` goals.
   
   All other goals are a sign to me there's an issue with your project structure, as a stack of plugins that are trying to fix something. Requiring a wildcard only confirms that to me.


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] adamretter commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

adamretter commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-638105933


   
   > With this, mvn verify will work as usual, doing the check at the end, mvn validate -Plicense will just call the check goals.
   
   @rfscholte Thanks Robert, that's an interesting approach to run the plugins using a profile rather than the wildcard option I added. But... isn't there a short-coming here? It looks like this assumes that I don't have anything else bound to the `verify` phase?


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] rfscholte closed pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

rfscholte closed pull request #357:
URL: https://github.com/apache/maven/pull/357


   


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

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] rfscholte commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

rfscholte commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-638413122


   It looks like you're still trying to convince me that wildcards are necessary. Your issue was the license check on the verify phase, I gave you one solution how to move them to the validate phase.
   If your verification are that important and they can be done before compilation, consider to bind those goals to the validate phase.
   Maven already provides a lot of options to control this. And even it it doesn't you can write a maven extension to control it.
   
   As said, this looks too specific for your usecase and in the end I expect more trouble than benefit, hence, I'll close 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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven] adamretter commented on pull request #357: Add wildcard option for direct goal execution id from command line

GitBox
In reply to this post by GitBox

adamretter commented on pull request #357:
URL: https://github.com/apache/maven/pull/357#issuecomment-639850919


   > It looks like you're still trying to convince me that wildcards are necessary.
   
   Not at all. I just wanted to check that the profile option you suggested worked as I assumed and that it would not *quite* solve my use-case.
   
   In the mean time I have sent a PR to improve the license plugin so that I don't need multiple executions - https://github.com/mycila/license-maven-plugin/pull/170
   
   p.s. Thanks for all the hard work on Maven - It's an awesome tool :-)


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

For queries about this service, please contact Infrastructure at:
[hidden email]