Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
desruisseaux commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2072026430 The use of Eclipse compiler would not be mandated. If no tool chain or `compilerId` parameter is specified, the plugin uses whatever compiler is returned by `ToolProvider.getSystemJavaCompiler()`. The Eclipse compiler was mentioned only for saying that its use should still be possible. The `JavaCompiler` API seems to have some support for giving the output files with `JavaFileManager.getJavaFileForOutput(StandardLocation.CLASS_OUTPUT, , JavaFileObject.Kind.CLASS, )` (I just tested it). However, it does not return the `*.class` files for inner (nested) classes, so some heuristic would still be needed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
gnodet commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2071969652 > > remove plexus-compiler layer but what about other compilers? > > `javax.tools.JavaCompiler` is an interface. From a quick search on internet, I think (but did not verified closely) that the Eclipse compiler implements this interface. If there is still a need for the Plexus compiler, we can write a wrapper. The `javax.tools` API is preferred to the Plexus API because it has some JPMS specific methods that I didn't saw in Plexus API. It also has a method telling us whether an option is valid, and a caching mechanism. > > For languages other than Java, we would need a separated plugin. With JPMS support, multi-releases support, incremental build (even incomplete), etc., this plugin is very Java-specific. The point is that this interface may not be sufficient. Ideally, we'd have a list of output files, that would help with the heuristic. I'm not sure the eclipse compiler is required for full incremental build, I think the requirement for the eclipse compiler is when only exposing direct dependencies for compiled classes (i.e. hide transitive dependencies so that the user is forced to declare those when the code is actually using them) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
rmannibucau commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2071704037 @desruisseaux +1 to drop plexus but also +1 to keep the current incremental feature support by default - whatever name is given if incremental is not considered accurate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
desruisseaux commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2071683447 > remove plexus-compiler layer but what about other compilers? `javax.tools.JavaCompiler` is an interface. From a quick search on internet, I think (but did not verified closely) that the Eclipse compiler implements this interface. If there is still a need for the Plexus compiler, we can write a wrapper. The `javax.tools` API is preferred to the Plexus API because it has some JPMS specific methods that I didn't saw in Plexus API. It also has a method telling us whether an option is valid, and a caching mechanism. For languages other than Java, we would need a separated plugin. With JPMS support, multi-releases support, incremental build (even incomplete), etc., this plugin is very Java-specific. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
olamy commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2071121026 > > I can tell you with 100% certainty that only tracking file timestamps won't work and will be a bug farm. > > It is incomplete, but this is what the current plugin and the `javac` command line do. Implementing a better incremental build would probably be a multi-steps process. In medium term, my goal is to migrate the plugin from Maven 3 / Plexus API to Maven 4 / `javax.tools` API in order to improve JPMS support. Fixing some of the incremental build issues is a side effect of this work. The goal for now is only to preserve the existing functionalities (with bug fixes). > remove plexus-compiler layer but what about other compilers? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
hunterpayne commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2071110405 I agree it is a lot of work. But you can't say this implementation will work either. It will be a bug farm. Inner classes, enums, and interface inheritance will all probably have weird and unexpected issues in more complex cases. And if there are classes compiled by other plugins like scalac? That's why bugs like this exist, because the mapping between java files and class files isn't clean. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
desruisseaux commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2071095574 > I can tell you with 100% certainty that only tracking file timestamps won't work and will be a bug farm. It is incomplete, but this is what the current plugin and the `javac` command line do. Implementing a better incremental build would probably be a multi-steps process. In medium term, my goal is to migrate the plugin from Maven 3 / Plexus API to Maven 4 / `javax.tools` API in order to improve JPMS support. Fixing some of the incremental build issues is a side effect of this work. The goal for now is only to preserve the existing functionalities (with bug fixes). After a refactored compiler plugin become stable enough, it would be possible to investigate how to do better. The proposal to use comma-separated values in `incrementalCompilation` parameters leaves room for that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
hunterpayne commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070978534 I can tell you with 100% certainty that only tracking file timestamps won't work and will be a bug farm. You must know which .class files are generated from which .java files and in addition know which .class files depend upon which other .class files. If this isn't possible, then it isn't possible to implement this feature in a maintainable way. So either we need to track this additional information, or relax the requirement to so, module level or package level and track dependencies at that level. I am OK with either path but we shouldn't go ahead with only tracking file timestamps and acting like that is a working incremental build feature. On Monday, April 22, 2024 at 02:17:06 PM PDT, Martin Desruisseaux ***@***.***> wrote: While incremental builds is important, my reading of the source code inherited from Maven 3 suggests that in its current state, incremental build has bugs causing the opposite effect than what the plugin wanted to achieve. We could try to fix the Maven 3 compiler plugin, but I do not know if it is worth the effort given that the plugin is getting a significant rewrite for Maven 4. The current useIncrementalCompilation parameter is a boolean flag which is actually mixing at least two aspects: whether to check if a dependency changed, and how to detect which source files changed. I suggest to deprecate that parameter and replace it with a new incrementalCompilation parameter (same name but without use prefix). The value of that parameter would be a comma-separated list of the following: - dependencies: check whether a dependency changed. - sources: check whether a source file has been updated, or whether source files were added or removed. - classes: check whether a source file is more recent than its .class file. - modules (new): don't try to do incremental build in Maven, but delegate that task to javac instead. This is possible only with modular projects. It uses the --module compiler option, which is an alternative to enumerating all source files on the command-line. Equivalences with the current parameter: - useIncrementalCompilation = true is equivalent to incrementalCompilation = dependencies,sources (except maybe for the cases of newly added source files). - useIncrementalCompilation = false is equivalent to incrementalCompilation = classes. For an initial version of the Maven 4 compiler plugin, detection of changes would be based only on file timestamps. This is the same strategy than the current plugin. This approach would be unable to detect that a class needs to be recompiled because it depends on another class which has been changed. However, the current plugin cannot detect such cases neither, so we don't lost anything. If we support that feature in the future, it would be one new value in the above-cited list of comma-separated values. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
desruisseaux commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070967076 While incremental builds is important, my reading of the source code inherited from Maven 3 suggests that in its current state, incremental build has bugs causing the opposite effect than what the plugin wanted to achieve. We could try to fix the Maven 3 compiler plugin, but I do not know if it is worth the effort given that the plugin is getting a significant rewrite for Maven 4. The current `useIncrementalCompilation` parameter is a boolean flag which is actually mixing at least two aspects: whether to check if a dependency changed, and how to detect which source files changed. I suggest to deprecate that parameter and replace it with a new `incrementalCompilation` parameter (same name but without `use` prefix). The value of that parameter would be a comma-separated list of the following: * `dependencies`: check whether a dependency changed. * `sources`: check whether a source file has been updated, or whether source files were added or removed. * `classes`: check whether a source file is more recent than its `.class` file. * `modules` (new): don't try to do incremental build in Maven, but delegate that task to `javac` instead. This is possible only with modular projects. It uses the `--module` compiler option, which is an alternative to enumerating all source files on the command-line. Equivalences with the current parameter: * `useIncrementalCompilation = true` is equivalent to `incrementalCompilation = dependencies,sources` (except maybe for the cases of newly added source files). * `useIncrementalCompilation = false` is equivalent to `incrementalCompilation = classes`. For an initial version of the Maven 4 compiler plugin, detection of changes would be based only on file timestamps. This is the same strategy than the current plugin. This approach would be unable to detect that a class needs to be recompiled because it depends on another class which has been changed. However, the current plugin cannot detect such cases neither, so we don't lost anything. If we support that feature in the future, it would be one new value in the above-cited list of comma-separated values. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
hunterpayne commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070771711 Great, so we have 2 buckets to break these issues into. 1) issues with the difference in behavior with javac (vs JDT) and 2) issues with the difference in behavior with this plugin and the Takari one. I'm curious, what are the issues in the first bucket? Perhaps the target should be to get the maven compiler to work on JDT as a requirement instead of javac and JDT. Maybe that's a goal we can achieve. On Monday, April 22, 2024 at 12:21:17 PM PDT, Tamas Cservenak ***@***.***> wrote: Today, if one needs fully incremental (and build avoidance) with current stable Maven 3.9.x, one should use Takari Lifecycle plugin. But even then you should use Eclipse JDT not javac. But, in turn, with JDT, you get other nice things like enforcing dependency usage and so on... http://takari.io/book/index.html — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
cstamas commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070735050 Today, if one needs _fully incremental (and build avoidance) with current stable Maven 3.9.x, one should use Takari Lifecycle plugin_. But even then you should use Eclipse JDT not javac. But, in turn, with JDT, you get other nice things like enforcing dependency usage and so on... http://takari.io/book/index.html -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
hunterpayne commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070717803 I agree. I am asking if there is a way we can relax the feature/requirement to make it easier to implement and maintain. On Monday, April 22, 2024 at 12:07:46 PM PDT, Tamas Cservenak ***@***.***> wrote: I agree that "incremental" is very important feature. My stance is that this implementation is bad/wrong. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
cstamas commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070692585 I agree that "incremental" is very important feature. My stance is that _this implementation_ is bad/wrong. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [MCOMPILER-515] This plugin is not "incremental" [maven-compiler-plugin]
hunterpayne commented on PR #160: URL: https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-2070683052 Incremental building is an important feature. If you need it, you need it. Most projects don't but some very important projects do and they need something like this the most. I think the right solution is a compromise where we scale back how this works. The problem is that computing the right set of code to recompile is hard. So let's just make that part easy. Perhaps make the incremental based upon something higher level that where we are now. If that's not possible, then the solution is to actually maintain a map of files to classes to dependent classes/files from the last build and to use that to compute what is built. I think that is the buggy part as that that map is hard to maintain. Perhaps make that map manually editable by devs. We only need this feature in dev environments anyway. TLDR This is an important feature so let's find a way to reduce the complexity to make it work and be maintainable. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
