gradle / gradle-native

The home of Gradle's support for natively compiled languages
https://blog.gradle.org/introducing-the-new-cpp-plugins
Apache License 2.0
92 stars 8 forks source link

Init script plugins should be able to call `allProjects()` in a `projectLoaded()` lifecycle hook #962

Closed ghale closed 5 years ago

ghale commented 5 years ago

The allprojects() hook uses project mutation locking to guard against cross-project configuration. Furthermore, we register projects in the project state registry via a projectsLoaded() lifecycle hook. When an initscript plugin is executed via a script in the distribution or user home directory, it is executed early in the build lifecycle. If the initscript plugin tries to insert a projectsLoaded() hook that then in turn calls allProjects(), the hook will be inserted before the hook to register projects in the project state registry. This means that the mutation locking in allProjects() will not be able to find the project in the state registry and will throw an exception.

A couple of options to address this:

adammurdoch commented 5 years ago

The last option is the way to go. Disabling the mutation locks should be very much a last resort, and we shouldn't be using the public hooks to do internal work.

I would look at either registering the projects at the end of settings loading, or when each project is created, or at the end of project loading.

lacasseio commented 5 years ago

This issue became a non-issue in 5.1 where commit https://github.com/gradle/gradle/commit/887ce3ca1030cd43931a99c186e0e5c7729642a8#diff-8b051be3965b9fc23dff9cf113dd518fL112 seems to have fixed the problem. It looks like test coverage wasn't added and is in face a side effect of the change. I will add test coverage and see if we can clean up this code as well https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/project/BuildOperationCrossProjectConfigurator.java#L59

big-guy commented 5 years ago

@lacasseio can you also reply to the original reporter that this is fixed in 5.1?

lacasseio commented 5 years ago

Done. I'm validating the test coverage and a small debt cleanup.

lacasseio commented 5 years ago

PR: https://github.com/gradle/gradle/pull/8122