Add opt-out to the duplicate project name detection

This commit reworks the project with duplicate names cycle
detection fix by adding an opt-out: because the new behavior
may force existing users to set both the artifactId and groupId
to publications even if they don't publish all projects, this
could be a potential breaking change.

subprojects/core/src/main/java/org/gradle/api/internal/artifacts/DefaultProjectModuleFactory.java

@@ -31,11 +31,18 @@
 import java.util.stream.Collectors;
 
 public class DefaultProjectModuleFactory implements ProjectModuleFactory {
+    public static final String DUPLICATE_DETECTION_SYSPROP = "org.gradle.dependency.duplicate.project.detection";
+
     private static final Splitter SPLITTER = Splitter.on(':')
         .omitEmptyStrings();
 
     private final Map<Project, Module> projectToModule = Maps.newConcurrentMap();
     private final Set<String> projectsWithSameName;
+    private final boolean detectDuplicates;
+
+    public static boolean isDuplicateDetectionEnabled() {
+        return Boolean.parseBoolean(System.getProperty(DUPLICATE_DETECTION_SYSPROP, "true"));
+    }
 
     public DefaultProjectModuleFactory(ProjectRegistry<? extends ProjectIdentifier> registry) {
         Map<String, Long> uniqueNames = registry.getAllProjects().stream()
@@ -48,6 +55,7 @@ public DefaultProjectModuleFactory(ProjectRegistry<? extends ProjectIdentifier>
             .map(Map.Entry::getKey)
             .forEach(builder::add);
         projectsWithSameName = builder.build();
+        detectDuplicates = isDuplicateDetectionEnabled();
     }
 
     private List<Project> findDuplicates(Project project) {
@@ -168,7 +176,7 @@ public String getName() {
 
         private void computeName() {
             String name = project.getName();
-            if (projectsWithSameName.contains(name)) {
+            if (detectDuplicates && projectsWithSameName.contains(name)) {
                 List<String> strings = SPLITTER.splitToList(project.getPath());
                 if (strings.size() > 1) {
                     lazyName = String.join("-", strings.subList(0, strings.size() - 1)) + "-" + name;

subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/ProjectDependencyResolveIntegrationTest.groovy

@@ -714,4 +714,39 @@ project(':b') {
             }
         }
     }
+
+    @Issue("https://github.com/gradle/gradle/issues/847")
+    def "can opt-out detection of circular dependencies with projects of same name"() {
+        given:
+        settingsFile << """
+            rootProject.name='duplicates'
+            include 'a:core'
+            include 'b:core'
+        """
+
+        buildFile << """
+            allprojects {
+                apply plugin: 'java-library'
+                group 'org.test'
+                version '1.0'
+            }
+
+            project(':a:core') {
+                dependencies {
+                    implementation project(':b:core')
+                }
+            }
+        """
+
+        def resolve = new ResolveTestFixture(buildFile, "compileClasspath")
+        resolve.prepare()
+
+        when:
+        fails 'a:core:checkDeps', '-Dorg.gradle.dependency.duplicate.project.detection=false'
+
+        then:
+        failure.assertHasDescription """Circular dependency between the following tasks:
+:a:core:compileJava
+\\--- :a:core:compileJava (*)"""
+    }
 }

subprojects/ivy/src/integTest/groovy/org/gradle/api/publish/ivy/IvyPublishMultiProjectIntegTest.groovy

@@ -343,9 +343,8 @@ $append
         """
     }
 
-    @ToBeFixedForInstantExecution
     @Issue("https://github.com/gradle/gradle/issues/847")
-    def "can publish projects with the same name should be considered different when building the graph"() {
+    def "fails publishing projects which share the same GAV coordinates"() {
         given:
         settingsFile << """
             rootProject.name='duplicates'
@@ -380,30 +379,52 @@ $append
         """
 
         when:
-        succeeds 'publishIvyPublicationToIvyRepo'
-        def project1 = javaLibrary(ivyRepo.module("org.gradle.test", "a-core", "1.0"))
-        def project2 = javaLibrary(ivyRepo.module("org.gradle.test", "b-core", "1.0"))
+        fails 'publishIvyPublicationToIvyRepo'
 
         then:
-        project1.assertPublishedAsJavaModule()
-        project2.assertPublishedAsJavaModule()
+        failure.assertHasCause "Project :b:core has the same (organisation, module name) as :a:core. You should set both the organisation and module name of the publication or opt out by adding the org.gradle.dependency.duplicate.project.detection system property to 'false'."
+    }
 
-        project1.parsedIvy.module == 'a-core'
-        project2.parsedIvy.module == 'b-core'
+    @ToBeFixedForInstantExecution
+    @Issue("https://github.com/gradle/gradle/issues/847")
+    def "fails publishing projects if they share the same GAV coordinates unless detection is disabled"() {
+        given:
+        settingsFile << """
+            rootProject.name='duplicates'
+            include 'a:core'
+            include 'b:core'
+        """
 
-        project1.parsedIvy.assertConfigurationDependsOn("runtime", "org.gradle.test:b-core:1.0")
+        buildFile << """
+            allprojects {
+                apply plugin: 'java-library'
+                apply plugin: 'ivy-publish'
+                group 'org.gradle.test'
+                version '1.0'
 
-        project1.parsedModuleMetadata.component.module == 'a-core'
-        project2.parsedModuleMetadata.component.module == 'b-core'
+                publishing {
+                    repositories {
+                        ivy { url "${ivyRepo.uri}" }
+                    }
+                    publications {
+                        ivy(IvyPublication) {
+                            from components.java
+                        }
+                    }
+                }
+            }
 
-        project1.parsedModuleMetadata.variant("runtimeElements") {
-            dependency("org.gradle.test", "b-core", "1.0")
-            noMoreDependencies()
-        }
+        """
 
-        and:
-        outputContains """Project :b:core has the same (organisation, module name) as :a:core. It has been assigned a synthetic Ivy module name of of b-core as a workaround, but you should set both the organisation and module name of the publication to be safe."""
-        outputContains """Project :a:core has the same (organisation, module name) as :b:core. It has been assigned a synthetic Ivy module name of of a-core as a workaround, but you should set both the organisation and module name of the publication to be safe."""
+        when:
+        succeeds 'publishIvyPublicationToIvyRepo', '-Dorg.gradle.dependency.duplicate.project.detection=false'
+        def project1 = javaLibrary(ivyRepo.module("org.gradle.test", "core", "1.0"))
+
+        then:
+        // this tests the current behavior, which is obviously wrong here as the user
+        // didn't overwrite the publication coordinates
+        project1.assertPublishedAsJavaModule()
+        project1.parsedIvy.module == 'core'
     }
 
     @ToBeFixedForInstantExecution
@@ -477,8 +498,8 @@ $append
         }
 
         and:
-        outputDoesNotContain "Project :a:core has the same (organisation, module name) as :b:core. It has been assigned a synthetic Ivy module name of of a-core as a workaround, but you should set both the organisation and module name of the publication to be safe."
-        outputDoesNotContain "Project :b:core has the same (organisation, module name) as :a:core. It has been assigned a synthetic Ivy module name of of b-core as a workaround, but you should set both the organisation and module name of the publication to be safe."
+        outputDoesNotContain "Project :a:core has the same (organisation, module name) as :b:core.  You should set both the organisation and module name of the publication or opt out by adding the org.gradle.dependency.duplicate.project.detection system property to 'false'."
+        outputDoesNotContain "Project :b:core has the same (organisation, module name) as :a:core.  You should set both the organisation and module name of the publication or opt out by adding the org.gradle.dependency.duplicate.project.detection system property to 'false'."
     }
 
     @ToBeFixedForInstantExecution
@@ -552,8 +573,8 @@ $append
         }
 
         and:
-        outputDoesNotContain "Project :a:core has the same (organisation, module name) as :b:core. It has been assigned a synthetic Ivy module name of of a-core as a workaround, but you should set both the organisation and module name of the publication to be safe."
-        outputDoesNotContain "Project :b:core has the same (organisation, module name) as :a:core. It has been assigned a synthetic Ivy module name of of b-core as a workaround, but you should set both the organisation and module name of the publication to be safe."
+        outputDoesNotContain "Project :a:core has the same (organisation, module name) as :b:core. You should set both the organisation and module name of the publication or opt out by adding the org.gradle.dependency.duplicate.project.detection system property to 'false'."
+        outputDoesNotContain "Project :b:core has the same (organisation, module name) as :a:core. You should set both the organisation and module name of the publication or opt out by adding the org.gradle.dependency.duplicate.project.detection system property to 'false'."
 
     }
 

subprojects/ivy/src/main/java/org/gradle/api/publish/ivy/internal/publication/DefaultIvyPublicationIdentity.java

@@ -16,11 +16,11 @@
 
 package org.gradle.api.publish.ivy.internal.publication;
 
+import org.gradle.api.InvalidUserCodeException;
 import org.gradle.api.Project;
+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.logging.Logger;
-import org.gradle.api.logging.Logging;
 import org.gradle.api.publish.ivy.internal.publisher.IvyPublicationIdentity;
 
 import javax.annotation.Nullable;
@@ -29,22 +29,19 @@
 import java.util.stream.Collectors;
 
 public class DefaultIvyPublicationIdentity implements IvyPublicationIdentity {
-    private final Logger logger;
     private Module delegate;
     private String organisation;
     private String module;
     private String revision;
 
-    public DefaultIvyPublicationIdentity(Module delegate, Logger logger) {
-        this.delegate = safeProjectCoordinatesProvider(delegate, logger);
-        this.logger = logger;
+    public DefaultIvyPublicationIdentity(Module delegate) {
+        this.delegate = safeProjectCoordinatesProvider(delegate);
     }
 
     public DefaultIvyPublicationIdentity(String organisation, String module, String revision) {
         this.organisation = organisation;
         this.module = module;
         this.revision = revision;
-        this.logger = Logging.getLogger(DefaultIvyPublicationIdentity.class);
     }
 
     @Override
@@ -77,21 +74,19 @@ public void setRevision(String revision) {
         this.revision = revision;
     }
 
-    private Module safeProjectCoordinatesProvider(Module module, Logger logger) {
+    private Module safeProjectCoordinatesProvider(Module module) {
         if (module instanceof ProjectBackedModule) {
-            return new LazyProjectModuleProvider((ProjectBackedModule) module, logger);
+            return new LazyProjectModuleProvider((ProjectBackedModule) module);
         }
         return module;
     }
 
     private static final class LazyProjectModuleProvider implements Module {
         private final ProjectBackedModule projectBackedModule;
-        private final Logger logger;
         private final AtomicBoolean warned = new AtomicBoolean();
 
-        private LazyProjectModuleProvider(ProjectBackedModule module, Logger logger) {
+        private LazyProjectModuleProvider(ProjectBackedModule module) {
             this.projectBackedModule = module;
-            this.logger = logger;
         }
 
         @Nullable
@@ -126,16 +121,15 @@ private void maybeWarnAboutDuplicates() {
             if (!warned.getAndSet(true)) {
                 Project project = projectBackedModule.getProject();
                 List<Project> projectsWithSameId = projectBackedModule.getProjectsWithSameCoordinates();
-                if (!projectsWithSameId.isEmpty()) {
+                if (!projectsWithSameId.isEmpty() && DefaultProjectModuleFactory.isDuplicateDetectionEnabled()) {
                     StringBuilder sb = new StringBuilder();
                     sb.append("Project ")
                         .append(project.getPath())
                         .append(" has the same (organisation, module name) as ")
                         .append(projectsWithSameId.stream().map(Project::getPath).collect(Collectors.joining(" and ")))
-                        .append(". It has been assigned a synthetic Ivy module name of of ")
-                        .append(projectBackedModule.getName())
-                        .append(" as a workaround, but you should set both the organisation and module name of the publication to be safe.");
-                    logger.warn(sb.toString());
+                        .append(". You should set both the organisation and module name of the publication")
+                        .append(" or opt out by adding the " + DefaultProjectModuleFactory.DUPLICATE_DETECTION_SYSPROP + " system property to 'false'.");
+                    throw new InvalidUserCodeException(sb.toString());
                 }
             }
         }

subprojects/ivy/src/main/java/org/gradle/api/publish/ivy/plugins/IvyPublishPlugin.java

@@ -27,7 +27,6 @@
 import org.gradle.api.internal.artifacts.Module;
 import org.gradle.api.internal.artifacts.configurations.DependencyMetaDataProvider;
 import org.gradle.api.internal.file.FileResolver;
-import org.gradle.api.logging.Logger;
 import org.gradle.api.model.ObjectFactory;
 import org.gradle.api.plugins.ExtensionContainer;
 import org.gradle.api.plugins.JavaPlatformPlugin;
@@ -89,8 +88,8 @@ public void apply(final Project project) {
                 objectFactory,
                 fileResolver,
                 project.getPluginManager(),
-                project.getExtensions(),
-                project.getLogger()));
+                project.getExtensions()
+            ));
             createTasksLater(project, extension, project.getLayout().getBuildDirectory());
         });
     }
@@ -169,23 +168,21 @@ private static class IvyPublicationFactory implements NamedDomainObjectFactory<I
         private final FileResolver fileResolver;
         private final PluginManager plugins;
         private final ExtensionContainer extensionContainer;
-        private final Logger logger;
 
         private IvyPublicationFactory(DependencyMetaDataProvider dependencyMetaDataProvider, Instantiator instantiator, ObjectFactory objectFactory, FileResolver fileResolver,
-                                      PluginManager plugins, ExtensionContainer extensionContainer, Logger logger) {
+                                      PluginManager plugins, ExtensionContainer extensionContainer) {
             this.dependencyMetaDataProvider = dependencyMetaDataProvider;
             this.instantiator = instantiator;
             this.objectFactory = objectFactory;
             this.fileResolver = fileResolver;
             this.plugins = plugins;
             this.extensionContainer = extensionContainer;
-            this.logger = logger;
         }
 
         @Override
         public IvyPublication create(String name) {
             Module module = dependencyMetaDataProvider.getModule();
-            IvyPublicationIdentity publicationIdentity = new DefaultIvyPublicationIdentity(module, logger);
+            IvyPublicationIdentity publicationIdentity = new DefaultIvyPublicationIdentity(module);
             NotationParser<Object, IvyArtifact> notationParser = new IvyArtifactNotationParserFactory(instantiator, fileResolver, publicationIdentity).create();
             VersionMappingStrategyInternal versionMappingStrategy = objectFactory.newInstance(DefaultVersionMappingStrategy.class);
             configureDefaultConfigurationsUsedWhenMappingToResolvedVersions(versionMappingStrategy);