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

Support for Isolated Projects #380

Open
michaelbull opened this issue Apr 5, 2024 · 12 comments
Open

Support for Isolated Projects #380

michaelbull opened this issue Apr 5, 2024 · 12 comments

Comments

@michaelbull
Copy link

Gradle has introduced the isolated projects feature that is currently in pre-alpha.

Enabling this pre-alpha feature with the io.spring.dependency-management plugin applied reports the following problems:

image

org.gradle.api.InvalidUserCodeException: Cannot access project ':file' from project ':www'
	at org.gradle.configurationcache.ProblemReportingCrossProjectModelAccess$ProblemReportingProject.getGroup(ProblemReportingCrossProjectModelAccess.kt:272)
	at io.spring.gradle.dependencymanagement.internal.VersionConfiguringAction$StandardLocalProjects.getNames(VersionConfiguringAction.java:124)
	at io.spring.gradle.dependencymanagement.internal.VersionConfiguringAction$CachingLocalProjects.getNames(VersionConfiguringAction.java:145)
	at io.spring.gradle.dependencymanagement.internal.VersionConfiguringAction.isDependencyOnLocalProject(VersionConfiguringAction.java:95)
	at io.spring.gradle.dependencymanagement.internal.VersionConfiguringAction.execute(VersionConfiguringAction.java:63)
	at io.spring.gradle.dependencymanagement.internal.VersionConfiguringAction.execute(VersionConfiguringAction.java:37)
	at java.base/java.lang.Thread.run(Thread.java:829)

The stacktrace points towards the VersionConfiguringAction.

		@Override
		public Set<String> getNames() {
			Set<String> names = new HashSet<>();
			for (Project localProject : this.project.getRootProject().getAllprojects()) {
				names.add(localProject.getGroup() + ":" + localProject.getName());
			}
			return names;
		}

This code ends up only being used by this method:

	private boolean isDependencyOnLocalProject(Project project, DependencyResolveDetails details) {
		return this.localProjects.getNames().contains(getId(details));
	}

	@Override
	public void execute(DependencyResolveDetails details) {
		logger.debug("Processing dependency '{}'", details.getRequested());
		if (isDependencyOnLocalProject(this.project, details)) {
			logger.debug("'{}' is a local project dependency. Dependency management has not been applied",
					details.getRequested());
			return;
		}

                ...

It seems like it is collecting all dependency names by asking the root project for all subproject names and then comparing their names to ensure that the plugin does not apply to local subproject dependency resolution. Collecting all the names via the root project is breaking the principal of project isolation.

I wonder if there is a simpler way to determine whether the plugin is a local project dependency, to accordingly ignore it when applying a controlled version? To comply with the project isolation feature, the plugin fundamentally cannot break out of the subproject it's applied to and query the root project for its subprojects, however this may not be feasible with the design of the plugin.

Let me know if you have any ideas and I'd be happy to help make the changes necessary to support the feature.

@wilkinsona
Copy link
Contributor

A couple of options come to mind:

  1. Try using Settings#getRootProject and ProjectDescriptor#getChildren to figure out the names of all of the projects.
  2. Disable the skipping of "local" projects when isolated projects are enabled

Assuming that it works, one is probably preferable.

@michaelbull
Copy link
Author

michaelbull commented Apr 5, 2024

With regards to option 2, I think we can still have it analysing local projects, but only those ones that are subprojects of the project the plugin is applied to. According to the build logic constraints, we can still safely call Project.getAllprojects, which returns "the set containing this project and its subprojects". This does however result in different behaviour for projects that are siblings of the one you've applied the plugin to, vs ones that are children (the siblings aren't checked, the children are).

I'll test a local project with a call to getRootProject and see if it complains.

@wilkinsona
Copy link
Contributor

I'll test a local project with a call to getRootProject and see if it complains.

How did you get on with this, @michaelbull?

@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@seregamorph
Copy link

seregamorph commented Jan 2, 2025

+1 for this feature request. In general, the CONFIGURING stage of Gradle can be quite long for projects that have hundreds of modules, so this new Isolated Projects feature looks promising.
I created a duplicate issue #402 where you can find more details and a demo project that reproduces the problem.

Regarding the violations of current plugin implementation I see there are some comments already in the thread. AFAIK it's okay to iterate other projects, but there are methods that cannot be called, e.g. otherProject.getGroup() (because it's not final).

I made some research on possible approaches. WDYT changing this

private boolean isDirectDependency(ModuleVersionSelector selector) {
	if (this.directDependencies == null) {
		Set<String> directDependencies = new HashSet<>();
		for (Dependency dependency : this.configuration.getAllDependencies()) {
			directDependencies.add(dependency.getGroup() + ":" + dependency.getName());
		}
		this.directDependencies = directDependencies;
	}
	return this.directDependencies.contains(getId(selector));
}

to this

import org.gradle.api.artifacts.ExternalDependency;
...
private boolean isDirectExternalDependency(ModuleVersionSelector selector) {
	if (this.directDependencies == null) {
		Set<String> directDependencies = new HashSet<>();
		for (Dependency dependency : this.configuration.getAllDependencies()) {
			if (dependency instanceof ExternalDependency) {
				directDependencies.add(dependency.getGroup() + ":" + dependency.getName());
			}
		}
		this.directDependencies = directDependencies;
	}
	return this.directDependencies.contains(getId(selector));
}

AFAIK it's possible to access other module of the same project by groupId:artifactId, but should this case be covered by the plugin? I believe this simple filter of dependency interface type can cover at least one violation (there are others).

@alllex
Copy link

alllex commented Jan 3, 2025

Iterating over projects and getting their name is not a violation of Isolated Projects constraints. Only accessing the group is a violation, because the group is a mutable field that can be changed during project configuration (e.g. when project's build script is executed).

Because of this Project.getGroup() or ProjectDependency.getGroup(), or ProjectDependency.getVersion() (version is also a mutable Project state) are all violations under Isolated Projects. Because ProjectDependency relies on the backing Project instance to get the group and version information, it's also unsafe. Therefore, using the suggested ExternalDependency filtering should be sufficient to avoid violations. However, I can't judge if it will be enough for what the plugin is trying to achieve.

@wilkinsona
Copy link
Contributor

I think only considering ExternalDependency instances may be enough for that case, however there are other violations that I cannot see how to resolve.

One example is detecting local projects as described in this issue's opening description. Unfortunately, using Settings#getRootProject and ProjectDescriptor#getChildren as I'd suggested above doesn't look like a viable approach as there's no link, AFAIK, from a Project to the Settings of the whole build.

Another problem is a violation that's reported when calling hasProperty on a child project when Gradle searches up to the project's parent. This is similar to gradle/gradle#28857.

@seregamorph
Copy link

@alllex is it possible to consider guarantee in Gradle initialization cycle that parent project is configured before current project (I can submit a Gradle project feature request if needed) with Isolated Projects enabled?

@wilkinsona
Copy link
Contributor

wilkinsona commented Jan 3, 2025

The changes in this branch should address all of this plugin's violations, at least in @seregamorph's sample project. However, they come at the cost of some changes in behaviour when project isolation is enabled as property resolution is different and detection of local projects is disabled. As such, I'm reluctant to merge them as those differences are likely to cause confusion and increase the support burden. Instead, I see them as away to highlight where alternative approaches in this plugin or improvements in Gradle are needed.

@seregamorph
Copy link

I checked out the commit and built locally via include-build and I still see failures:

 ./gradlew build -Dorg.gradle.unsafe.isolated-projects=true --include-build ../spring/dependency-management-plugin 
Isolated projects is an incubating feature.
Calculating task graph as no cached configuration is available for tasks: build
[Incubating] Problems report is available at: file:///Users/morph/Projects/demo-gradle-spring-multi-module/build/reports/problems/problems-report.html

FAILURE: Build failed with an exception.

* What went wrong:
Configuration cache problems found in this build.

4 problems were found storing the configuration cache, 2 of which seem unique.
- Plugin class 'org.gradle.api.plugins.BasePlugin': Project ':app' cannot access 'Project.group' functionality on subprojects of project ':'
- Plugin class 'org.gradle.api.plugins.BasePlugin': Project ':app' cannot access 'Project.version' functionality on subprojects of project ':'

See the complete report at file:///Users/morph/Projects/demo-gradle-spring-multi-module/build/reports/configuration-cache/5pnmb3rxo2cce3oi32s0o9aot/9g9gch1gy3uivw0kswcjli5zg/configuration-cache-report.html
> Project ':app' cannot access 'Project.group' functionality on subprojects of project ':'
> Project ':app' cannot access 'Project.version' functionality on subprojects of project ':'

It's much better than it was and I really appreciate the fix is ready that fast.

@wilkinsona
Copy link
Contributor

If you look at the report, I think you’ll see that the remaining problems are caused by Spring Boot’s plugin, not this one.

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

No branches or pull requests

5 participants