Skip to content

Commit

Permalink
Fix circular dependencies when project have the same name
Browse files Browse the repository at this point in the history
Before this commit, during dependency resolution, a synthetic
module version identifier was generated by project, using the
group and name of the project. However, it's possible for a
project in gradle to have the same name as another in the
same build, leading to duplicates. In this case the projects
were mixed together and lead to a circular dependency.

This commit fixes the problem by making sure we generate
distinct module version identifiers for such projects, by
using the full project path as the name instead of the short
name.

This also makes it possible to publish valid publications
when using the maven or ivy publish plugins. However, we detect
this problem early and warn the user that they should overwrite
the project identity in this case.
  • Loading branch information
melix committed Jan 13, 2020
1 parent 40ea3d3 commit 404c6cf
Show file tree
Hide file tree
Showing 15 changed files with 875 additions and 94 deletions.
@@ -0,0 +1,158 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.gradle.api.internal.artifacts;

import com.google.common.base.Splitter;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.gradle.api.Project;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

public class DefaultProjectModuleFactory implements ProjectModuleFactory {
private static final Splitter SPLITTER = Splitter.on(':')
.omitEmptyStrings();

private final Map<Project, Module> projectToModule = Maps.newConcurrentMap();

public DefaultProjectModuleFactory() {
}

private List<Project> findDuplicates(Project project) {
Set<Project> projects = project.getRootProject().getAllprojects();
String current = toGroupAndArtifact(project);
List<Project> duplicates = null;
for (Project projectIdentifier : projects) {
if (project != projectIdentifier) {
String ga = toGroupAndArtifact(projectIdentifier);
if (current.equals(ga)) {
if (duplicates == null) {
duplicates = Lists.newArrayList();
}
duplicates.add(projectIdentifier);
}
}
}
return duplicates == null ? Collections.emptyList() : duplicates;
}

private static String toGroupAndArtifact(Project projectIdentifier) {
return projectIdentifier.getGroup() + ":" + projectIdentifier.getName();
}

@Override
public Module getModule(Project project) {
return projectToModule.computeIfAbsent(project, this::createId);
}

private Module createId(Project project) {
return new DynamicDeduplicatingModuleProjectIdentifier(project);
}

private abstract class AbstractProjectBackedModule implements ProjectBackedModule {

private final Project project;

@Override
public Project getProject() {
return project;
}

public AbstractProjectBackedModule(Project project) {
this.project = project;
}

@Override
public List<Project> getProjectsWithSameCoordinates() {
List<Project> ids = findDuplicates(project);
if (ids.isEmpty()) {
return Collections.emptyList();
}
return ids.stream()
.filter(id -> id != project)
.collect(Collectors.toList());
}

@Override
public String getGroup() {
return String.valueOf(project.getGroup());
}

@Override
public String getVersion() {
return project.getVersion().toString();
}

@Override
public String getStatus() {
return project.getStatus().toString();
}

@Override
public String getProjectPath() {
return project.getPath();
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}

AbstractProjectBackedModule that = (AbstractProjectBackedModule) o;

if (!project.equals(that.project)) {
return false;
}

return true;
}

@Override
public int hashCode() {
return project.hashCode();
}
}

private class DynamicDeduplicatingModuleProjectIdentifier extends AbstractProjectBackedModule {
private final Project project;

private DynamicDeduplicatingModuleProjectIdentifier(Project project) {
super(project);
this.project = project;
}

@Override
public String getName() {
List<Project> duplicates = findDuplicates(project);
if (duplicates.isEmpty()) {
return project.getName();
}
List<String> strings = SPLITTER.splitToList(project.getPath());
if (strings.size() <= 1) {
return project.getName();
}
return String.join("-", strings.subList(0, strings.size() - 1)) + "-" + project.getName();
}
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2012 the original author or authors.
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -13,64 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.gradle.api.internal.artifacts;

import org.gradle.api.Project;

public class ProjectBackedModule implements Module {

private final Project project;

public ProjectBackedModule(Project project) {
this.project = project;
}

@Override
public String getGroup() {
return project.getGroup().toString();
}

@Override
public String getName() {
return project.getName();
}

@Override
public String getVersion() {
return project.getVersion().toString();
}

@Override
public String getStatus() {
return project.getStatus().toString();
}

@Override
public String getProjectPath() {
return project.getPath();
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}

ProjectBackedModule that = (ProjectBackedModule) o;

if (project != null ? !project.equals(that.project) : that.project != null) {
return false;
}

return true;
}
import java.util.List;

@Override
public int hashCode() {
return project != null ? project.hashCode() : 0;
}
public interface ProjectBackedModule extends Module {
Project getProject();
List<Project> getProjectsWithSameCoordinates();
}
@@ -0,0 +1,22 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.gradle.api.internal.artifacts;

import org.gradle.api.Project;

public interface ProjectModuleFactory {
Module getModule(Project project);
}
Expand Up @@ -27,8 +27,10 @@
import org.gradle.api.internal.DocumentationRegistry;
import org.gradle.api.internal.StartParameterInternal;
import org.gradle.api.internal.artifacts.DefaultModule;
import org.gradle.api.internal.artifacts.DefaultProjectModuleFactory;
import org.gradle.api.internal.artifacts.DependencyManagementServices;
import org.gradle.api.internal.artifacts.Module;
import org.gradle.api.internal.artifacts.ProjectModuleFactory;
import org.gradle.api.internal.artifacts.configurations.DependencyMetaDataProvider;
import org.gradle.api.internal.classpath.ModuleRegistry;
import org.gradle.api.internal.classpath.PluginModuleRegistry;
Expand Down Expand Up @@ -553,4 +555,8 @@ protected BuildScanUserInputHandler createBuildScanUserInputHandler(UserInputHan
protected BuildInvocationDetails createBuildInvocationDetails(BuildStartedTime buildStartedTime) {
return new DefaultBuildInvocationDetails(buildStartedTime);
}

protected ProjectModuleFactory createProjectModuleIdentifierFactory() {
return new DefaultProjectModuleFactory();
}
}
Expand Up @@ -24,7 +24,7 @@
import org.gradle.api.internal.MutationGuards;
import org.gradle.api.internal.artifacts.DependencyManagementServices;
import org.gradle.api.internal.artifacts.Module;
import org.gradle.api.internal.artifacts.ProjectBackedModule;
import org.gradle.api.internal.artifacts.ProjectModuleFactory;
import org.gradle.api.internal.artifacts.configurations.DependencyMetaDataProvider;
import org.gradle.api.internal.artifacts.dsl.dependencies.ProjectFinder;
import org.gradle.api.internal.collections.DefaultDomainObjectCollectionFactory;
Expand Down Expand Up @@ -279,8 +279,8 @@ public boolean isScript() {
}
}

protected DependencyMetaDataProvider createDependencyMetaDataProvider() {
return new ProjectBackedModuleMetaDataProvider();
protected DependencyMetaDataProvider createDependencyMetaDataProvider(ProjectModuleFactory projectModuleIdentifierFactory) {
return new ProjectBackedModuleMetaDataProvider(projectModuleIdentifierFactory);
}

protected ServiceRegistryFactory createServiceRegistryFactory(final ServiceRegistry services) {
Expand All @@ -301,9 +301,15 @@ protected TypeConverter createTypeConverter(PathToFileResolver fileResolver) {
}

private class ProjectBackedModuleMetaDataProvider implements DependencyMetaDataProvider {
private final ProjectModuleFactory projectModuleIdentifierFactory;

public ProjectBackedModuleMetaDataProvider(ProjectModuleFactory projectModuleIdentifierFactory) {
this.projectModuleIdentifierFactory = projectModuleIdentifierFactory;
}

@Override
public Module getModule() {
return new ProjectBackedModule(project);
return projectModuleIdentifierFactory.getModule(project);
}
}

Expand Down
Expand Up @@ -16,13 +16,15 @@

package org.gradle.api.internal.artifacts


import org.gradle.test.fixtures.AbstractProjectBuilderSpec

class ProjectBackedModuleTest extends AbstractProjectBuilderSpec {
ProjectModuleFactory factory = new DefaultProjectModuleFactory()

def "module exposes project properties"() {
given:
def module = new ProjectBackedModule(project)
def module = factory.getModule(project)

expect:
module.name == project.name
Expand Down
Expand Up @@ -39,8 +39,9 @@ import org.gradle.api.internal.CollectionCallbackActionDecorator
import org.gradle.api.internal.FactoryNamedDomainObjectContainer
import org.gradle.api.internal.GradleInternal
import org.gradle.api.internal.ProcessOperations
import org.gradle.api.internal.artifacts.DefaultProjectModuleFactory
import org.gradle.api.internal.artifacts.Module
import org.gradle.api.internal.artifacts.ProjectBackedModule
import org.gradle.api.internal.artifacts.ProjectModuleFactory
import org.gradle.api.internal.artifacts.configurations.DependencyMetaDataProvider
import org.gradle.api.internal.collections.DomainObjectCollectionFactory
import org.gradle.api.internal.file.DefaultProjectLayout
Expand Down Expand Up @@ -154,6 +155,7 @@ class DefaultProjectTest extends Specification {
CrossProjectConfigurator crossProjectConfigurator = new BuildOperationCrossProjectConfigurator(buildOperationExecutor)
ClassLoaderScope baseClassLoaderScope = new RootClassLoaderScope("root", getClass().classLoader, getClass().classLoader, new DummyClassLoaderCache(), Stub(ClassLoaderScopeRegistryListener))
ClassLoaderScope rootProjectClassLoaderScope = baseClassLoaderScope.createChild("root-project")
ProjectModuleFactory moduleFactory = new DefaultProjectModuleFactory()

def setup() {
rootDir = new File("/path/root").absoluteFile
Expand Down Expand Up @@ -953,7 +955,7 @@ def scriptMethod(Closure closure) {

def getModule() {
when:
Module moduleDummyResolve = new ProjectBackedModule(project)
Module moduleDummyResolve = moduleFactory.getModule(project)
dependencyMetaDataProviderMock.getModule() >> moduleDummyResolve
then:
project.getModule() == moduleDummyResolve
Expand Down

0 comments on commit 404c6cf

Please sign in to comment.