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

Test to specify licenseSet includes from CLI #878

Merged

Conversation

mathieu
Copy link
Contributor

@mathieu mathieu commented Jan 8, 2025

Added an IT test to reproduce the solution provided in #536. In the tri-license-set-with-includes integration test we have 3 licenseSets and would only need to format the Unformatted2.java file via the command line.

On needs to update the license-maven-plugin/src/it/tri-license-set-with-includes/invoker.properties files to test others ways of setting this.

It fails for me with the following variations:

invoker.goals=license:format -Dlicense.licenseSets.licenseSet.includes=src/main/java/com/mycila/it/Unformatted2.java
invoker.goals=license:format -Dlicense.licenseSet.includes=src/main/java/com/mycila/it/Unformatted2.java
invoker.goals=license:format -Dlicense.includes=src/main/java/com/mycila/it/Unformatted2.java

@mathieucarbou
Copy link
Owner

mathieucarbou commented Jan 11, 2025

The third flavor is the one that existed before and was deprecated:

But to be considered, a nearly complete legacy config has to be provided:

if (legacyConfigHeader == null && (this.legacyConfigInlineHeader == null || this.legacyConfigInlineHeader.isEmpty())) {
return null;
}

licenseSets is an array here, and no property is attached so it cannot be accessed:

I think we should add one, yes, like you did in this PR, so that you an do for example:

mvn license:format -Dlicense.licenseSets[0].includes=src/main/java/com/mycila/it/Unformatted2.java -Dlicense.licenseSets[0].excludes=src/main/java/com/mycila/it/Unformatted1.java

I will try that... Anyway, this PR updating the properties make sense.

@mathieucarbou
Copy link
Owner

mathieucarbou commented Jan 11, 2025

@mathieu : the test initial states are wrong IMO.

The pom config should reflect the correct state of the project. So if we have this:

            <licenseSet>
              <header>mock-license-1.txt</header>
              <excludes>
                <exclude>invoker.properties</exclude>
                <exclude>pom.xml</exclude>
                <exclude>*.groovy</exclude>
                <exclude>**/*.bak</exclude>
                <exclude>*.log</exclude>
                <exclude>mock-license-*</exclude>
                <exclude>**/Unformatted2.java</exclude>
                <exclude>**/Unformatted3.java</exclude>
              </excludes>
            </licenseSet>
            <licenseSet>
              <header>mock-license-2.txt</header>
              <excludes>
                <exclude>invoker.properties</exclude>
                <exclude>pom.xml</exclude>
                <exclude>*.groovy</exclude>
                <exclude>**/*.bak</exclude>
                <exclude>*.log</exclude>
                <exclude>mock-license-*</exclude>
                <exclude>**/Unformatted1.java</exclude>
                <exclude>**/Unformatted3.java</exclude>
              </excludes>
            </licenseSet>
            <licenseSet>
              <header>mock-license-3.txt</header>
              <includes>
                <include>**/Unformatted3.java</include>
              </includes>
            </licenseSet>

And we commit the file Unformatted2.java, It means that in main, I should have all the files according to the filters formatted with the correct license. Which means Unformatted1 and 3 should be formatted correctly already.

To simulate a commit-hook acting on a new file, Unformatted1.java and Unformatted3.java should be formatted because Unformatted2.java is the one added if I understand your invoker command.

Also, the commit hook should consider the number of license check definitions in the POM.

If I correct the initial state and update the invoker command like that, it works:

invoker.goals=license:format -Dlicense.licenseSets[0].includes=src/main/java/com/mycila/it/Unformatted2.java -Dlicense.licenseSets[1].includes=src/main/java/com/mycila/it/Unformatted2.java -Dlicense.licenseSets[2].includes=src/main/java/com/mycila/it/Unformatted2.java

In this example, Unformatted2.java is the new file added in the commit and the commit hook will format it based on the second header style.

@mathieucarbou mathieucarbou marked this pull request as ready for review January 11, 2025 16:54
@mathieucarbou mathieucarbou merged commit 25624db into mathieucarbou:master Jan 11, 2025
12 checks passed
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