Le mar. 15 avr. 2025 à 13:03, Martin Desruisseaux <
martin.desruisse...@geomatys.com> a écrit :
> Le 2025-04-15 à 11 h 53, Romain Manni-Bucau a écrit :
>
> >> If we add a new mechanism for allowing other plugins to access the
> >> configuration of the Java compiler plugin, it would make plugin
> >> evolution more risky since a change in the parameters could
> >> potentially impact an unknown number of plugins. A little bit like
> >> making all the private fields of a class public.
> >>
> > It is not different than using a patch file.
> >
> It is different in that `module-info-patch` is parsed by the compiler
> plugin only, and "compiled" in a set of JVM parameters that Surefire can
> use. Surefire does not need to know anything about `module-info-patch`.
>
Assuming you use javac or a standard compiler as known today, this got
proven fragile multiple times even before jpms until you totally open
module-info-patch which would lead to args opening.
But even assuming that, if it is 100% specific to compiler plugin - for me
it can't but i'll not fight much on that since i'll not push jpms too -
then it is trivial to solve -> compiler plugin config and be it, no need to
do anything new in maven.
>
>
> > Also there is no place for now in the pom as there was no place to set
> > a jpms module, we did half of the work there, not sure I understand
> > why you are reluctant to finish the work and try to use a quick a
> > dirty solution there 🤔
> >
> Because I disagree that it covers only half of the work and is a dirty
> solution. I do not see a more elegant solution at this time regarding
> integration with the existing practice of writing a `module-info.java`
> file.
>
I fully understand your point but where we disagree is that you priviledge
java over maven whereas maven already superseeds java by design (once again
dependencies example is a good one).
So you make it neat for a build system which wouldn't be maven but likely
not neat at all for maven land IMHO.
>
>
> > there it is more generic options, not JPMS specific, could even be a
> > -Xmx (likely a wrong example but you get the idea hopefully).
> >
> > <optionSets> <!-- or jvmOptionSets maybe
> > <optionSet>
> > <id>compilation-base</id>
> > <values>
> > <value>...</value>
> > </values>
> > </optionSet>
> > </optionSets>
> >
> > with such a config the plugin can consume
> >
> >
> <jvmOptionSetIds><jvmOptionSetId>compilation-base</jvmOptionSetId><jvmOptionSetId>compilation-j24</jvmOptionSetId></jvmOptionSetIds>
> But this approach is more verbose and tedious to write, both because XML
> and because it forces users to repeat the module names in --add-reads,
> --add-exports and --add-opens parameters, something that the
> `module-info-patch` approach handles automatically. It forces all
> plugins to support the `add-reads TEST-MODULE-PATH` special value, while
> the `module-info-patch` approach resolves that for them. Furthermore, I
> believe that there is value in treating the module configuration
> parameters separately from other parameters. They are special in that
> they modify a source code written by the developer: the content of
> `module-info`, and putting them in a `module-info-patch` file makes that
> fact clear. It does not mean that the <optionSet> proposal wouldn't be
> useful for other use cases, but not necessarily for module configuration.
>
Not really, your solution will end up being the same - maybe something I
didn't mention is that I assume we can use properties for module references
but this is something common for dependencies for ex.
With your patch option you need to invent a new interpolation system and so
on whereas it is built-in with the pom.
If you argue there is value having a specific set named testModulePatch it
can be a compromise, but limiting the feature this way without making it
maven friendly and ultimately dropping maven spirit to go lower level is
super weird from an user standpoint at least (likely worse from a dev
standpoint).
>
>
> > The disavantages are:
> >
> > - it is a file format which doesn't exist nor is supported by any tool
> As is the addition of <optionSets> element in POM, with the additional
> difficulty that tools would have to separate this element from the other
> Maven stuff in a POM, and then separate the module configuration options
> from other options mixed in that element.
>
This is 100% about the convention we bring there.
If you are right it means all your work about types for dependencies is as
wrong so not sure i'm following you there again, it is literally the same
solution.
>
>
> > - it will need a plus/minus mechanism ("diff") to be fully functional
> for advanced cases
>
> I do not see which advanced cases you are referring. Can you provide a
> concrete example, JPMS parameters included?
>
The projects with JPMS I worked on needed:
- to remove some encapsulation (open depending how you do it)
- drop some SPI (worked around with custom classloaders but it was ugly)
from module-info
- be able to implement a not exported module API for mock purposes
- be able to call an internal API ("partialStart" spirit)
The point is either you want to have a global solution and you need to
basically override the main module-info as we do today with plugin in the
pipe or hacks in test module, or you are fine with workarounds (classloader
or using classpath instead of modulepath in test for ex) and we do nothing
but a half backed solution just adds noise and I still have a hard time to
see your solution solving more that bringing issues (not sayingit doesnt
help some cases, just that it can be more confusing overall).
>
>
> > - it provides some configuration outside the source of truth (pom -
> > and you are right there is module-info file already but a lot of
> > project just generate it from code+pom already)
> >
> A module-info cannot be generated properly from the POM, as the latter
> has no information about which packages to export, which packages to
> opens, which services to declare, which services to implement, and which
> dependencies to make transitive ("transitive" here does not have the
> same meaning as Maven). The source of truth of `src/main/java` is the
> combination of the POM and module-info. The `module-info-patch` approach
> is an extension of that fact to `src/test/java`. In my opinion, this is
> the most natural extension as it reuses the existing pattern.
>
Just as a matter of fact several plugins we already discussed about do it.
Indeed, some meta are added in the plugin to complete it but it is a fact
this is solved for projects since years.
>
>
> > Hmm, maybe I'm not thinking to the same use case than you but
> > ultimately you need both files if test module provides is also an
> > actual module no?
> >
> No. if the tests are provided in their own module, there is no need to
> patch those tests. For example, there is no need to patch JUnit for
> using JUnit. We only patch the module which is *using* JUnit.
>
Ok so a no which means yes ;).
This is the current practise in src/test/java.
That said you comment brings me back to an old bug that surefire runs in
dir mode and not jar mode, maybe this is something we should enhanced and
enable surefire plugin which can already help.
>
>
> > On the habit point I'm also not sure, why is it smoother to not use
> > the pom for a system where you have only the pom? 🤔
> >
> Because we don't have only POM. We have POM + module-info.
>
> * Main code = POM + module-info
> * Test code = POM + module-info-patch
>
So test module can't be an applicative test module so you break maven
src/test layout (=you enforce to extract these modules now)?
> * Module-info-patch is mirror of module-info: it contains the stuff
> that modify the main module-info, and nothing else.
>
> Can you see the symmetry and how it fits nicely in the existing pattern?
> Putting the module configuration parameters in the POM is the dirty
> approach.
>
s/dirty/maven/, maven build is 100% about the pom, anything outside except
maven jvm itself config is dirty and we even make plugin with their own
configuration being configurable in the pom (from checkstyle to a plain
application server like apache tomee).
Once again I fail to see why we should use <dependencies> but not
<jvmOptions> in a plugin or reusable if you want to limit duplication since
it is the way maven is built.
Ultimately if you want a built system dedicated to jpms which is very neat
and limits the duplication you should go with a file aggregating
module-info+deps+options then generate all the files with duplication
instead.
But this is not how maven works so, once again, it looks like you are
merging maven with yet another build system but the cost to make maven
inconsistent is quite high IMHO.
>
> Martin
>
>
