Re: Replacement proposal for

2024-04-23 Thread Martin Desruisseaux

Le 2024-04-23 à 10 h 21, sebb a écrit :


Minor correction: the classes method will necessarily *detect* added 
sources, because they won't have a class file. What occurs as a result 
is another matter./ It might be worth considering forcing a full build 
as an option in such cases if there is any way that adding a new 
source file can affect the compilation of others.


Yes, I was thinking about one more value for , 
maybe "rebuildOnAdd". The current Maven 3 plugin has an implicit 
"rebuild on add" behavior when  is true, but 
not when it is false.


    Martin



-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org



Re: Replacement proposal for

2024-04-23 Thread sebb
Minor correction: the classes method will necessarily *detect* added
sources, because they won't have a class file.
What occurs as a result is another matter./
It might be worth considering forcing a full build as an option in
such cases if there is any way that adding a new source file can
affect the compilation of others.

On Mon, 22 Apr 2024 at 13:52, Martin Desruisseaux
 wrote:
>
> Hello all
>
> The Maven compiler plugin has an  boolean
> parameter with `true` as the default value. This parameter has issues
> discussed in MCOMPILER-209 [1], which has 61 votes. In short, builds are
> much faster when this parameter is set to `false`, which is
> counter-intuitive. During the refactoring for the Maven 4 compiler
> plugin, by looking at the source code, I saw the following issues with
> the current implementation:
>
>   * When  is enabled (which is the default),
> the plugin compares the list of source files in the current build
> with a list saved after the previous build in order to detect
> changes. But it seems to me that the plugin uses relative paths in
> one case, and absolute paths in the other cases. Consequently, the
> paths do not match and the plugin always thinks that all source
> files changed. I did not double-checked or tested (maybe I missed
> some `toAbsoluteFile()` calls). But if confirmed, it would explain
> the MCOMPILER-209 issue.
>   * The ways that the two lists are compared has a performance cost of
> O(N²). It could easily be O(N) instead by replacing one list by an
> hash set.
>   * The plugin tries to detect changes in dependencies, but the
> algorithm works only if the compilation never fail. Consider the
> following scenario:
>   o One project has 3 modules: A, B and C.
>   o Module B and C depends on A.
>   o Developer makes a change in A and run `mvn install`.
>   o Module A compiles successfully, but B fails because of the
> change in A.
>   o Developer fixes the build failure in B and run `mvn install`
> again. In this scenario, the incremental compilation will not
> see that C also needs to be rebuilt, because when
>  is enabled, the plugin compares the
> JAR timestamps with the build start time. In this scenario, the
> second build start time is after the `A.jar` creation time.
> Maybe this issue was unnoticed because the issue explained in
> the first bullet point caused the plugin to always recompile
> everything anyway.
>
> I propose to deprecate  in Maven 4 and
> replace it by a new parameter: . Its value would
> be a comma-separated list of values instead of a single boolean flag.
> Those values specify which methods to use for deciding which files to
> recompile:
>
>   * "sources": check for changes in source files, including whether a
> file has been deleted. This method uses a file saved by the previous
> build in the "target/maven-status"  directory (as done by the
> current implementation).
>   * "classes": check whether the source file is more recent than the
> class file. This method does not need any "maven-status" file from
> the previous build, but does not detect whether a file has been
> added or removed.
>   * "dependencies": check whether a dependency has changed (using a more
> robust algorithm than the current one).
>   * "modules": do not specify any file to the compiler and use the
> `--module` option instead, in which case the compiler will scan the
> directories itself and decides itself which files to recompile based
> on their timestamp. This option is available for modular projects only.
>
> The current  boolean flag maps to above
> proposal as below:
>
>   * `true` is approximately equivalent to "sources,dependencies", except
> that I propose to not rebuild everything when a file is added (it is
> usually not needed).
>   * `false` is equivalent to "classes".
>
> As seen from above, the current  actually
> mixes two aspects that could be treated separately: whether to check if
> a dependency changed, and whether to use a list saved from the previous
> build for checking if a source file changed. The comma-separated value
> proposal would allow users to control those aspects independently.
>
>  Martin
>
> [1]https://issues.apache.org/jira/browse/MCOMPILER-209

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org



Re: Replacement proposal for

2024-04-22 Thread Tamás Cservenák
+1

And I'd just add this to the discussion:
https://github.com/apache/maven-compiler-plugin/pull/160

:D

T

On Mon, Apr 22, 2024 at 2:54 PM Martin Desruisseaux <
martin.desruisse...@geomatys.com> wrote:

> Hello all
>
> The Maven compiler plugin has an  boolean
> parameter with `true` as the default value. This parameter has issues
> discussed in MCOMPILER-209 [1], which has 61 votes. In short, builds are
> much faster when this parameter is set to `false`, which is
> counter-intuitive. During the refactoring for the Maven 4 compiler
> plugin, by looking at the source code, I saw the following issues with
> the current implementation:
>
>   * When  is enabled (which is the default),
> the plugin compares the list of source files in the current build
> with a list saved after the previous build in order to detect
> changes. But it seems to me that the plugin uses relative paths in
> one case, and absolute paths in the other cases. Consequently, the
> paths do not match and the plugin always thinks that all source
> files changed. I did not double-checked or tested (maybe I missed
> some `toAbsoluteFile()` calls). But if confirmed, it would explain
> the MCOMPILER-209 issue.
>   * The ways that the two lists are compared has a performance cost of
> O(N²). It could easily be O(N) instead by replacing one list by an
> hash set.
>   * The plugin tries to detect changes in dependencies, but the
> algorithm works only if the compilation never fail. Consider the
> following scenario:
>   o One project has 3 modules: A, B and C.
>   o Module B and C depends on A.
>   o Developer makes a change in A and run `mvn install`.
>   o Module A compiles successfully, but B fails because of the
> change in A.
>   o Developer fixes the build failure in B and run `mvn install`
> again. In this scenario, the incremental compilation will not
> see that C also needs to be rebuilt, because when
>  is enabled, the plugin compares the
> JAR timestamps with the build start time. In this scenario, the
> second build start time is after the `A.jar` creation time.
> Maybe this issue was unnoticed because the issue explained in
> the first bullet point caused the plugin to always recompile
> everything anyway.
>
> I propose to deprecate  in Maven 4 and
> replace it by a new parameter: . Its value would
> be a comma-separated list of values instead of a single boolean flag.
> Those values specify which methods to use for deciding which files to
> recompile:
>
>   * "sources": check for changes in source files, including whether a
> file has been deleted. This method uses a file saved by the previous
> build in the "target/maven-status"  directory (as done by the
> current implementation).
>   * "classes": check whether the source file is more recent than the
> class file. This method does not need any "maven-status" file from
> the previous build, but does not detect whether a file has been
> added or removed.
>   * "dependencies": check whether a dependency has changed (using a more
> robust algorithm than the current one).
>   * "modules": do not specify any file to the compiler and use the
> `--module` option instead, in which case the compiler will scan the
> directories itself and decides itself which files to recompile based
> on their timestamp. This option is available for modular projects only.
>
> The current  boolean flag maps to above
> proposal as below:
>
>   * `true` is approximately equivalent to "sources,dependencies", except
> that I propose to not rebuild everything when a file is added (it is
> usually not needed).
>   * `false` is equivalent to "classes".
>
> As seen from above, the current  actually
> mixes two aspects that could be treated separately: whether to check if
> a dependency changed, and whether to use a list saved from the previous
> build for checking if a source file changed. The comma-separated value
> proposal would allow users to control those aspects independently.
>
>  Martin
>
> [1]https://issues.apache.org/jira/browse/MCOMPILER-209
>


Replacement proposal for

2024-04-22 Thread Martin Desruisseaux

Hello all

The Maven compiler plugin has an  boolean 
parameter with `true` as the default value. This parameter has issues 
discussed in MCOMPILER-209 [1], which has 61 votes. In short, builds are 
much faster when this parameter is set to `false`, which is 
counter-intuitive. During the refactoring for the Maven 4 compiler 
plugin, by looking at the source code, I saw the following issues with 
the current implementation:


 * When  is enabled (which is the default),
   the plugin compares the list of source files in the current build
   with a list saved after the previous build in order to detect
   changes. But it seems to me that the plugin uses relative paths in
   one case, and absolute paths in the other cases. Consequently, the
   paths do not match and the plugin always thinks that all source
   files changed. I did not double-checked or tested (maybe I missed
   some `toAbsoluteFile()` calls). But if confirmed, it would explain
   the MCOMPILER-209 issue.
 * The ways that the two lists are compared has a performance cost of
   O(N²). It could easily be O(N) instead by replacing one list by an
   hash set.
 * The plugin tries to detect changes in dependencies, but the
   algorithm works only if the compilation never fail. Consider the
   following scenario:
 o One project has 3 modules: A, B and C.
 o Module B and C depends on A.
 o Developer makes a change in A and run `mvn install`.
 o Module A compiles successfully, but B fails because of the
   change in A.
 o Developer fixes the build failure in B and run `mvn install`
   again. In this scenario, the incremental compilation will not
   see that C also needs to be rebuilt, because when
is enabled, the plugin compares the
   JAR timestamps with the build start time. In this scenario, the
   second build start time is after the `A.jar` creation time.
   Maybe this issue was unnoticed because the issue explained in
   the first bullet point caused the plugin to always recompile
   everything anyway.

I propose to deprecate  in Maven 4 and 
replace it by a new parameter: . Its value would 
be a comma-separated list of values instead of a single boolean flag. 
Those values specify which methods to use for deciding which files to 
recompile:


 * "sources": check for changes in source files, including whether a
   file has been deleted. This method uses a file saved by the previous
   build in the "target/maven-status"  directory (as done by the
   current implementation).
 * "classes": check whether the source file is more recent than the
   class file. This method does not need any "maven-status" file from
   the previous build, but does not detect whether a file has been
   added or removed.
 * "dependencies": check whether a dependency has changed (using a more
   robust algorithm than the current one).
 * "modules": do not specify any file to the compiler and use the
   `--module` option instead, in which case the compiler will scan the
   directories itself and decides itself which files to recompile based
   on their timestamp. This option is available for modular projects only.

The current  boolean flag maps to above 
proposal as below:


 * `true` is approximately equivalent to "sources,dependencies", except
   that I propose to not rebuild everything when a file is added (it is
   usually not needed).
 * `false` is equivalent to "classes".

As seen from above, the current  actually 
mixes two aspects that could be treated separately: whether to check if 
a dependency changed, and whether to use a list saved from the previous 
build for checking if a source file changed. The comma-separated value 
proposal would allow users to control those aspects independently.


    Martin

[1]https://issues.apache.org/jira/browse/MCOMPILER-209