Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wildcard option for direct goal execution id from command line #357

Closed

Conversation

adamretter
Copy link

@adamretter adamretter commented Jun 1, 2020

When specifying an execution-id for a goal on the command line (e.g. mvn plugin:goal@my-execution-id) this PR now also allows 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
you have to acknowledge this by using the following check-box.

…tion-id) also allow wildcard syntax to select all execution ids (e.g. plugin:goal@*)
@rfscholte
Copy link
Contributor

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.

@adamretter
Copy link
Author

@rfscholte Okay, so this is my use-case...

I am using the 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:

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

@rfscholte
Copy link
Contributor

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.

@adamretter
Copy link
Author

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?

@rfscholte
Copy link
Contributor

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

@adamretter
Copy link
Author

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?

@rfscholte
Copy link
Contributor

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.

@adamretter
Copy link
Author

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 - mathieucarbou/license-maven-plugin#170

p.s. Thanks for all the hard work on Maven - It's an awesome tool :-)

gnodet added a commit to gnodet/maven that referenced this pull request Nov 20, 2024
* Disable MNG-6330 because it now validly finds the parent given the path is not specified
* Add missing roots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants