gradlex-org / java-module-dependencies

A Gradle plugin to use dependencies from 'module-info.java' files.
Apache License 2.0
48 stars 9 forks source link

Support for defining hierarchical project paths in the Settings DSL #136

Open TheGoesen opened 2 weeks ago

TheGoesen commented 2 weeks ago

The project name is the identity via which the project is addressed ...

That statement is incorrect. The name of the project is not an identity. as the javadoc of project.name states:

p>Returns the name of this project. The project's name is not necessarily unique within a project hierarchy. You

  • should use the {@link #getPath()} method for a unique identifier for the project.

As mentioned by my work account in #108: I have a lot of projects that have the same name, but not the same path. I guess i have also no clue why I have not been affected by https://github.com/gradle/gradle/issues/847 in the slightest, for me multiple projects with the same name work just fine.

So currently i would not use the settings syntax because it would cause the ApplyPluginsAction to run on unintended projects, because they have the same name as intended projects. (But different paths)

jjohannes commented 2 weeks ago

What I tried to convey when I answered this is the other issue is that in my opinion, in the context of a Java Project, it is good practice to give a unique name to each project. The Javadoc you cited is true as a general statement about Gradle's Project type, which can be used in all kinds of contexts. And then yes, in general multiple projects with the same name are possible.

Since you have a project where you use the same name multiple times, I am happy to discuss which feature you would like to see to support your setup better.

We would need more features in the Settings plugin approach. Probably an additional configuration option for the project path if it differs from the "artifact". As I myself have no use for it, I can't really test it out in a real large project, but I could imagine something like this:

javaModules {
  module("module-b") {
    parentPath = "..." // complete path is '<parentath>:<artifact>'
  }
  directory("modules") { 
    parentPath = "..." // set parent path for all module in the directory
  }
}

Would that work for you? Maybe you want to give adding something like that a try?

tg-freigmbh commented 2 weeks ago

My naive idea: using the default gradle syntax -> include("zInstaller:Installer-Prepare:iasetup:Tools") would be plainly adapted into-> javaModules { moduleFromPath("zInstaller:Installer-Prepare:iasetup:Tools") } but the idea to use the parentPath would work just as well. Can give that a try when i find some time

jjohannes commented 2 weeks ago

My idea with the parentPath approach is to allow to define paths, but still allow you to use the following functionality that already exists:

If we are fine with saying: If you have a path then the path needs to match the directory hierarchy Then your idea might be better from the usability perspective. In that case, we could consider to also add a directoryFromPath, which includes all modules in the folder that matches the given path.

Maybe you can try different ideas in you projects to get a feeling for what has the best usability for your use cases.

TheGoesen commented 1 week ago

So a few thoughts so far:

  1. I am not 100% on the current way of "replacing/wrapping" the include command with another definition. If another gradle plugin exists that for some reason also want you their own versions of include, a user will not be able to combine these two plugins.

  2. The logic of the directory instruction and its auto include as it is currently -> include every directory which is cool because you dont even need a build file. However the logic that I use in my big project-> include every build.gradle.kts you will find, because thats how you know its something thats actually ported to gradle. (the build file can still be empty)

  3. So its a bit of either I introduce a bit of sugarcoded 'do The Thing that @TheGoesen does' flag, which would be

  4. Have a configurable search depth for build files

  5. Include every build file that gets picked up

  6. But only those project that have a module-info.java in their src/main get this plugin applied. Because there is a lot of stuff that isnt even java. Doing this assumption either hardcoded or configurable would not be hard to implement but might be awkward to document/maintain/explain.

As an alternative I see the following

  1. let the user "include" whatever he wants
  2. The user then somehow tells the settings plugin that they want this project to the jpms projects.

Thoughts?

jjohannes commented 1 week ago

Good point @TheGoesen. The idea behind the Settings DSL is certainly opinionated towards certain use cases.

I understand that it is not what you want in some existing projects, which follow different patterns/conventions already that you would like to keep for whatever reason. Therefore, I think it makes sense to split the tow aspects if we can:

  1. The DSL for defining the project structure
  2. The mechanism that finds the module-info.java files at initialization time to parse them early

Then you could basically exchange (1) with you own mechanism and still profit from (2).

A problem why it can't be completely detached right now is that the mechanism also needs to know the group of the project for the capability mapping. Gradle 8.11 will have a new method to select a capability without needing to know the group: https://docs.gradle.org/8.11-rc-1/release-notes.html#api-for-selecting-variants-by-feature-name With this new feature, it may be possible to avoid the "knowledge about group" aspect. I haven't tried it yet, but I am confident.

Assuming that we do not need the group anymore, we can offer something for this:

  1. The user then somehow tells the settings plugin that they want this project to the jpms projects.

For example:

include(":some-project") // include yourself with Gradle's default mechanism
javaModules.module(project(":some-project")) // tell the plugin about a project that contains Java Modules

This can then be embedded in you own code that auto-includes by looking for build files etc.

Maybe such a general mechanism is also a good solution for the original issue? That is, we don't extend the DSL for including directories with more specifics, but instead allow you to do your own thing if needed by using Gradle's default include plus javaModules.module(project(":project:with:path:already:included")).

tg-freigmbh commented 1 week ago

Hmm can this new feature be used for something that doesnt have its own "sourceSet"? like the way its documented I would asume you need to create a sourceSet for your "feature"/jar file, which I avoided in some case of generated code.

jjohannes commented 1 week ago

I am pretty sure it works with all java { registerFeature("foo") { ... } } setups. It's just an example in the release notes.

tg-freigmbh commented 1 week ago

ok but If i am not registering a sourceSet with a feature... then how can I use a feature at all?

jjohannes commented 1 week ago

In combination with Java Modules it makes little sense to use it without its own Source Set, and it's own module-info, in my opinion.

In traditional Java, this is a way to formalize what is called "Optional Dependency" in Maven. Which, in Maven, is just useless metadata the tools ignore.

But here you can say "I have transitive dependency X which is only needed if you use a certain piece of my code". If you use that piece, please tell by "using" the feature.

The feature points to the "main" source set but still has additional dependencies only in the scope of that feature.

Here is an article on that topic: https://blog.gradle.org/optional-dependencies

But I think this is pretty unrelated to this issue. :)