Refactoring and support of <release> and <module> elements
#320
Conversation
desruisseaux
force-pushed
the
master
branch
2 times, most recently
from
5af3279 to
4082f1c
Compare
…as testing another plugin than this project. It was testing `pw.krejci:multi-release-jar-maven-plugin`, which fails with `NoSuchMethodError` with Maven 4-RC3. That external project may need to be updated for Maven 4.
Update for changes in the `Project` interface. The fix leverages the new `SourceRoot` element.
…of the `pom.xml` file. With this commit, the include/exclude filters declared in `<source>` are combined with the include/exclude filters declared in the puglin configuration. The module name and release information are also stored, but only partially supported yet.
…d body in a separated class. The intend is to reduce the complexity by splitting one big method into smaller methods. It may also help external applications (e.g. IDE) to use `ToolExecutor` in their environment.
…ment. Initial support of modular project (JPMS) by using the `<module>` element. Currently support only one or the other, not yet both in same time. Contains fixes for incremental build: * The `package-info.java` source files may produce no output. * Provide an option to rebuild everything as soon as one file changed. The `additions` aspect has been renamed `rebuild-on-add` for clarity and for consistency with `rebuild-on-change` added in the last above point.
…c link. This hack is needed only when we want to simulate the Maven 3 behaviour for compatibility reasons. This hack is fragile to the case where a module name is a single name (without dot) and that name is the same as the package name. This ambiguity breaks two tests. For allowing the tests to pass, this commit renames "foo" module as "foo.bar".
- Multi-release project defined with the new `<source>` element. - Modular project defined with the new `<source>` element.
…t the stack trace. apache#323
|
Hello @gnodet. I'm not sure how to proceed with this pull request, because it is massive and would require a lot of work for reviewers. This PR is hard to split in smaller pull requests focussing on different features. The best I could do is to try to organize the commits. |
…r and more readable output. Give a hint when a change of directory is needed for using the relative paths.
|
Note that this pull request does not contain the |
|
@desruisseaux is it worth adding an integration test with an annotation processor to ensure that if any file changed the module is totally cleaned as before and rebuilt? (ie there is no "diff" logic in the compilation pipeline). Similarly the template plugin (https://www.mojohaus.org/templating-maven-plugin/) can need something similar so it impacts the templated files if any other changed (case a date is in the template). |
|
Sure, thanks for pointing a hole. Just to clarify, there is already an integration test that compiles the project, change a file, then recompiles the project, but not using an annotation processor (it uses Groovy if I remember right). Is it the kind of test you had in mind? |
|
@desruisseaux yes, the main diff is that without annotation processors (or alike) you can just rerun |
|
In the new compiler plugin, the incremental compilation is controlled by an |
|
@desruisseaux I guess so (but have to admit I have no idea yet why it is made so complex for something so simple, either a change invalidates the compilation or it doesn't, no?) |
|
The problem is that neither the old or the new implementation can determine reliably what needs to be recompiled as a result of a change. For a true determination, we would need to build a graph of relationships between all Another reason is historical: the old implementation already provided a configuration option in the form of a simple boolean value. But neither the behaviour of the |
Hmm no, on that part the only valuable default is the safe one, heuristics (for speed) are ok but not as defaults and ultimately shouldn't be used in the xml (except in a profile) as a good practise IMHO.
technically this wouldn't work since you would need to know an annotation processor needs all classes (this is even outside the .class of the project even the .class of the processor doesn't even have a ref on others so the graph doesn't work). Several build systems did enable incremental annotation processors and/or toggles to configure that - looks like your new option at some extent. That said it doesn't change that the default should clear by default if any doubt and never assume it can do an incremental compilation, as a record idea just fixed it partially in a very recent release and it was really blocking the development and a hidden gem so hope maven doesnt reach that state for end users and keeps a safe default which is optimizable with your option explicitly if desired but not the opposite probably. So a good default would more look like: hope it makes sense |
|
Applied the above-proposed default (rebuild everything when an annotation processor is present) in commit c7535b5. |
…ess (forked compiler). Avoid too verbatim verifications of locale-sensitive compiler messages. apache#327
…ameter if an annotation processor is present.
This is a refactoring of the compiler plugin which separates the MOJO, which contains the configuration, from the code that configures and executes the
javax.tools.JavaCompiler. This refactoring splits one big class in two smaller classes for the compilation of main, and similarly for the compilation of test. This refactoring facilitates the development described in the next paragraph.This pull request implements the support of the
<include>,<exclude>,<release>and<module>elements of<source>. For now, the release and module elements can be used separately but not yet together.This pull request also contains opportunistic bug fixes in the incremental compilation. There is one change visible to users compared to 4.0.0-beta-2: in the
incrementalCompilationoption, theadditionsenumeration value has been renamedrebuild-on-addfor clarity and consistency withrebuild-on-change. These options did not existed in Maven 3.