[MCOMPILER-408] CompileMojo resets project artifacts when used in verify phase

[jira-importer]

Benjamin Marwell opened MCOMPILER-408 and commented

Problem Description / Setup

I tried to execute the google error-prone plugin in the compile phase, but was unsatisfied with the outcome. However, it seemed to be a perfect fit for the verify phase. Other plugins will also use the verify phase, e.g. checkstyle. I will also compile it to a different folder (which is target/classes-errorprone).

However, when using the CompileMojo, it will always call [projectArtifact.setFile(dir);|https://github.com/apache/maven-compiler-plugin/blob/master/src/main/java/org/apache/maven/plugin/compiler/CompilerMojo.java#L189]. There is no possibility to override this.

I do think this is not always the wanted behaviour.

Now, when executing 

mvn clean verify -Perrorprone 

the ear project will not find the war anymore, because the war had:

• Compile phase creating target/classes
• package phase, which set projectArtifact.setFile() to the .war file.
• verify phase, which set projectArtifact.setFile() back to target/classes.

Reproducible

Always, see above.

Workarounds

Do not execute verify+errorprone together with your main target phase and all reactor projects (e.g. including ear). But this will need two compile steps (one for packaging, anothero ne for  error-prone).

Expected behaviour

I can execute the maven-compiler-plugin with a compiler plugin like checkstyle in the verify phase without un-attaching any previous artifacts. At least as long as I compile into another directory.

Actual behaviour

I cannot execute the compiler in the verify phase again using the maven-compiler-plugin, because it will reset my project artifacts.

 

errorprone configuration

<profile>
  <id>errorprone</id>
  <activation>
    <activeByDefault>false</activeByDefault>
  </activation>
  <build>
    <plugins>
      <plugin>
        <artifactId>maven-compiler-plugin</artifactId>
        <executions>
          <execution>
            <id>errorprone</id>
            <phase>verify</phase>
            <goals><goal>compile</goal></goals>
            <configuration>
              <showWarnings>true</showWarnings>
              <compilerArgs>
                <arg>-XDcompilePolicy=simple</arg>
                <arg>
                  -Xplugin:ErrorProne
                  ${plugin.errorprone.excluded.paths}
                  -XepDisableWarningsInGeneratedCode
                  ${plugin.errorprone.additional.checks}
                </arg>
                <arg>-d</arg>
                <arg>${project.build.directory}/classes-ep</arg>
              </compilerArgs>
              <annotationProcessorPaths>
                <path>
                  <groupId>com.google.errorprone</groupId>
                  <artifactId>error_prone_core</artifactId>
                  <version>${dependency.errorprone.core.version}</version>
                </path>
              </annotationProcessorPaths>
              <useIncrementalCompilation>true</useIncrementalCompilation>
            </configuration>
          </execution>
        </executions>
      </plugin>
    </plugins>
  </build>
</profile> 

Note: I use incrementalCompilation=true which behaves like non-incremental compilation due to a bug which is open since ever.

Proposed sulutions

Either create a new goal (compile-only) or provide a switch to not invoke [projectArtifact.setFile(outputdir)|https://github.com/apache/maven-compiler-plugin/blob/master/src/main/java/org/apache/maven/plugin/compiler/CompilerMojo.java#L189].

 

---

Affects: 3.8.0, 3.8.1

[jira-importer]

Robert Scholte commented

To me this looks like fixing the wrong problem. What do you mean with ...but was unsatisfied with the outcome.? Compilation should be done during the compile phase. If error-prone can only do its job during compilation, then that's where things should be fixed.

[jira-importer]

Benjamin Marwell commented

Robert, I totally agree that compilation which leads to packaging should go into the compile phase. I just wouldn't want to have non-optimized code which was compiled with such a plugin in my final jar.

I see no reason why a second compilation step would be harmful when done with a compiler plugin in another phase of the build.

The only other option I see would be to execute mvn a second time or beforehand.

Or maybe I could make it a new Maven plugin which mimics the compiler plugin, but leaves out setting the output. Is that reasonable?

Perhaps Google's choice to implement errorprone with a compiler plugin was a bad idea, perhaps a bytecode scanner should be used for this job? As you have more experience with maven, I would like to hear your opinion on this.

Btw, sorry for phrases like "unsatisfied with the outcome". I meant something more technical like "I think this should be working, because of...".

[jira-importer]

Karl Heinz Marbaise commented

bmarwell Can you give more details why it does not work (cause the documentation of errorprone look like I expect that it should work)...and I don't see the real reason why your configuration is binding the compile plugin to the verify phase ? It would be very helpful to have full example project....

[jira-importer]

Benjamin Marwell commented

I can think of numerous reasons.

First of all, error prone verifies your code. It does so by compiling with a plugin, but this doesn't mean it should be in the same phase as the main compilation phase. On the contrary, I want to avoid this code to end up in a jar file I might ship. The only other way I could think of would be to run error prone first and then do mvn clean verify. That doesn't seem right.

Also, I'm just binding one execution of the compile plugin to the verify phase.

The idea behind all this is to be able to have just one mvn run and failing the build if a specific test fails. All similar plugins do not do this in the compile phase (eg checkstyle).

For error prone I am changing the output directory, because its generated classes should not be picked up by the packaging phase which runs after compile. Thus I was surprised to see this error in the first place.

So why not run this in the compile phase instead of the default execution? Because:

1. Some annotation processors will not work nicely with error prone
2. Some checks only work by disabling some optimizations, and I do not want to ship such jars
3. I want to avoid calling Maven twice (with profile and then clean install afterwards).
4. It is not possible the other way round because I do not want to install/deploy if a test fails.

Does this make sense? I think it would just integrate nicely in the verify phase.