Skip to content

Conversation

@TheGoesen
Copy link
Contributor

Hello,

Ok So this change adresses 3 thing.

  1. ModuleDependencyReport uses getModuleArtifacts().unset(); but that doesn't seem exist before gradle 8.6 (or 8.7?), which can create some unexpected problems..
  2. As mentioned in Support projects with same name but different project paths #108 : Lets not full explode with no chance of continuing the build when multiple projects share the same name. Some functionality might be reduced, warnings are printed..
  3. Lets just skip the path heuristic entirely if this plugin is applied to the corresponding project and we have already parsed the module file and we absolutely know the modulename. This allows the plugin to support arbitrary modulenames. Modulenamecheck must be disabled for that to work, or else it will complain..

The last point could also be hidden behind config/feature switch, but gradle test shows all good.
Copy and pasted some other sample project and adjusted the modulenames, looks good to me

jjohannes pushed a commit that referenced this pull request Jun 4, 2024
@jjohannes
Copy link
Member

Thank you for the suggestions and in particular for bringing the issue with "unset" to our attention.

I picked the two fixes from this PR into separate PRs and will release them with the next patch (#111, #113). I do not print a warning for the duplicated projects for now, as one could use this setup with custom mappings and then you won't be able to get rid of the warnings. But this topic may be improved further #108. And...

...this goes into your third proposal. Which is that we should get rid of the magic that tries to "guess" the module names of other projects. The reason it is not done yet as you propose is that we do not want to access the state of another project. This is not guaranteed to work because of the order in which projects are configured. It would probably work in most cases here, because we do it late inside a Provider, but some projects might still run into trouble. More importantly, this kind of cross-project access will be forbidden in the future with project isolation.

We already have a problem here with the getGroup, which I also only use reluctantly (related Gradle issue: gradle/gradle#13672 (comment))

I changed the test setup to activate -Dorg.gradle.unsafe.isolated-projects=true. If you run the updated tests against your proposal it'll now show the issue.

I won't integrate this change, but I have other ideas to tackle this. I also want to get rid of this "magic" (or at least allow users to opt-out of it). This PR motivated me to write it down: #114

I'll close this one. Please feel free to share feedback and ideas in #114.

@jjohannes jjohannes closed this Jun 4, 2024
@jjohannes
Copy link
Member

The fixes have been released as part in 1.6.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants