Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to generate a module graph #271

Closed
jokoolle-183 opened this issue Aug 20, 2024 · 8 comments
Closed

Unable to generate a module graph #271

jokoolle-183 opened this issue Aug 20, 2024 · 8 comments

Comments

@jokoolle-183
Copy link
Contributor

jokoolle-183 commented Aug 20, 2024

After adding the dependency to the plugin and attempting to generate a dependency graph for the app module, I was confronted with generation errors.

Then I tried running a simple ./gradlew generateModulesGraphStatistics for the whole project and got this output:

Screenshot 2024-08-20 at 3 06 23 PM

Thinking I might have misconfigured something, I made sure I followed the implementation as in Google's Now in Android project (added plugin dependency in root level gradle file and set apply to 'true'). I tried generating the module graph in the NIA project, as well as running the statistics task and this is the result:

Screenshot 2024-08-20 at 3 08 18 PM

In order to debug the plugin I included it as a composite build and started digging.
It was obvious that all the subprojects weren't getting loaded or recognized. So I played around and changed plugin initialization a bit.

Screenshot 2024-08-20 at 3 17 46 PM

After applying this change, the project and all of its subprojects got loaded.

Screenshot 2024-08-20 at 2 59 50 PM

In order to test this on other projects such as NIA, I forked the repo and published some versions with the above-mentioned changes to Jitpack.
This change seems to not affect the NIA project, although I only tested out generating module graphs via their script and running the statistics task.

@jraska
What do you think about these changes?
Is there anything specific I'd need to take into consideration and verify?

I know this might seem like overkill for needs of a single project, so any feedback on how we might fix this locally in order to avoid applying changes to the plugin library would be appreciated!

P.S. Currently the project I'm working on is using the forked version of your plugin, since it's crucial to our restructuring and remodularization initiative, since we're having long build times.
Thank you for your motivating presentation on the Droidcon and Medium articles on this topic, it's been really inspiring and one of the driving forces to resolve technical debt in our project 💪

UPDATE 22.8.2024

Fixed in this PR --> #272

jokoolle-183 added a commit to jokoolle-183/modules-graph-assert that referenced this issue Aug 20, 2024
…uated block that waits for all projects and subprojects to get evaluated
jokoolle-183 added a commit to jokoolle-183/modules-graph-assert that referenced this issue Aug 20, 2024
@jraska
Copy link
Owner

jraska commented Aug 20, 2024

Hey, thanks for describing the issue into such detail and testing it on the way 👏

I actually think you found a way how it should be done correctly 🤔 and I also suspect this issue was related: #230

The bug you are experiencing is happening because the dependencies are not loaded at the moment the plugin is trying to read them.
My thinking is that gradle.projectsEvaluated is called after all projects are evaluated as it's name says, which is actually what the plugin assumed and it works with project.afterEvaluate in majority of cases, but clearly not in all of those.

Thank you very much for discovering this! Let's get that PR merged :)

jokoolle-183 added a commit to jokoolle-183/modules-graph-assert that referenced this issue Aug 21, 2024
jraska pushed a commit that referenced this issue Aug 21, 2024
* #271 Replaced single project evaluation block with projectsEvaluated block that waits for all projects and subprojects to get evaluated

* #271 Changed group name to com.github.jokoolle-183.modules-graph-assert

* #271 Reverted group name
@jraska
Copy link
Owner

jraska commented Aug 22, 2024

I will keep the issue open before the release goes out. :) not fixed until released.

@jraska jraska reopened this Aug 22, 2024
@jokoolle-183
Copy link
Contributor Author

Hello @jraska,
When can we expect the release, a rough estimation? 😄
Just asking for the sake of general information.

@jraska
Copy link
Owner

jraska commented Aug 27, 2024

Hey, sorry for the delay. I'm aware it's expected.

I hope max 2 weeks I need to figure out something in the background.

@jokoolle-183
Copy link
Contributor Author

Hey, sorry for the delay. I'm aware it's expected.

I hope max 2 weeks I need to figure out something in the background.

Okay, thanks!

@jraska
Copy link
Owner

jraska commented Aug 27, 2024

Solved. Thanks for the patience and the contribution :)

2.7.0

@jraska jraska closed this as completed Aug 27, 2024
@jokoolle-183
Copy link
Contributor Author

Solved. Thanks for the patience and the contribution :)

2.7.0

Awesome!
Thank you so much 🫶

@jraska
Copy link
Owner

jraska commented Aug 28, 2024

Hey, I will be reverting this change temporarily as it causes the tasks not to be found in many scenario without the flag --no-configure-on-demand

Current plan

  • Revert the change and release. Version 2.7.1 to come back to stable behaviour and not lose trust of the plugin users.
  • Keep the 2.7.0 out for experimenting - you can keep using it if it works for you
  • Get the full understanding without time pressure
  • Come up with final proposal - --no-configure-on-demand being mandatory is a likely option, but then I think it sould be version 3.0.0 as quite breaking change.

jraska added a commit that referenced this issue Aug 28, 2024
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

No branches or pull requests

2 participants