Rework exclude rule merging

subprojects/core/src/main/java/org/gradle/api/internal/file/archive/TarFileTree.java

@@ -104,11 +104,12 @@ private void visitImpl(FileVisitor visitor, InputStream inputStream) throws IOEx
         NoCloseTarInputStream tar = new NoCloseTarInputStream(inputStream);
         TarEntry entry;
         File expandedDir = getExpandedDir();
+        boolean isFirstTimeVisit = !expandedDir.exists();
         while (!stopFlag.get() && (entry = tar.getNextEntry()) != null) {
             if (entry.isDirectory()) {
-                visitor.visitDir(new DetailsImpl(resource, expandedDir, entry, tar, stopFlag, chmod));
+                visitor.visitDir(new DetailsImpl(resource, isFirstTimeVisit, expandedDir, entry, tar, stopFlag, chmod));
             } else {
-                visitor.visitFile(new DetailsImpl(resource, expandedDir, entry, tar, stopFlag, chmod));
+                visitor.visitFile(new DetailsImpl(resource, isFirstTimeVisit, expandedDir, entry, tar, stopFlag, chmod));
             }
         }
     }
@@ -189,13 +190,15 @@ private static class DetailsImpl extends AbstractFileTreeElement implements File
         private final NoCloseTarInputStream tar;
         private final AtomicBoolean stopFlag;
         private final ReadableResourceInternal resource;
+        private final boolean isFirstTimeVisit;
         private final File expandedDir;
         private File file;
         private boolean read;
 
-        public DetailsImpl(ReadableResourceInternal resource, File expandedDir, TarEntry entry, NoCloseTarInputStream tar, AtomicBoolean stopFlag, Chmod chmod) {
+        public DetailsImpl(ReadableResourceInternal resource, boolean isFirstTimeVisit, File expandedDir, TarEntry entry, NoCloseTarInputStream tar, AtomicBoolean stopFlag, Chmod chmod) {
             super(chmod);
             this.resource = resource;
+            this.isFirstTimeVisit = isFirstTimeVisit;
             this.expandedDir = expandedDir;
             this.entry = entry;
             this.tar = tar;
@@ -213,7 +216,7 @@ public void stopVisiting() {
         public File getFile() {
             if (file == null) {
                 file = new File(expandedDir, entry.getName());
-                if (!file.exists()) {
+                if (isFirstTimeVisit) {
                     copyTo(file);
                 }
             }

subprojects/core/src/main/java/org/gradle/api/internal/file/archive/ZipFileTree.java

@@ -79,6 +79,7 @@ public void visit(FileVisitor visitor) {
         try {
             ZipFile zip = new ZipFile(zipFile);
             File expandedDir = getExpandedDir();
+            boolean isFirstTimeVisit = !expandedDir.exists();
             try {
                 // The iteration order of zip.getEntries() is based on the hash of the zip entry. This isn't much use
                 // to us. So, collect the entries in a map and iterate over them in alphabetical order.
@@ -92,9 +93,9 @@ public void visit(FileVisitor visitor) {
                 while (!stopFlag.get() && sortedEntries.hasNext()) {
                     ZipEntry entry = sortedEntries.next();
                     if (entry.isDirectory()) {
-                        visitor.visitDir(new DetailsImpl(zipFile, expandedDir, entry, zip, stopFlag, chmod));
+                        visitor.visitDir(new DetailsImpl(zipFile, isFirstTimeVisit, expandedDir, entry, zip, stopFlag, chmod));
                     } else {
-                        visitor.visitFile(new DetailsImpl(zipFile, expandedDir, entry, zip, stopFlag, chmod));
+                        visitor.visitFile(new DetailsImpl(zipFile, isFirstTimeVisit, expandedDir, entry, zip, stopFlag, chmod));
                     }
                 }
             } finally {
@@ -116,15 +117,17 @@ private File getExpandedDir() {
 
     private static class DetailsImpl extends AbstractFileTreeElement implements FileVisitDetails {
         private final File originalFile;
+        private final boolean isFirstTimeVisit;
         private final File expandedDir;
         private final ZipEntry entry;
         private final ZipFile zip;
         private final AtomicBoolean stopFlag;
         private File file;
 
-        public DetailsImpl(File originalFile, File expandedDir, ZipEntry entry, ZipFile zip, AtomicBoolean stopFlag, Chmod chmod) {
+        public DetailsImpl(File originalFile, boolean isFirstTimeVisit, File expandedDir, ZipEntry entry, ZipFile zip, AtomicBoolean stopFlag, Chmod chmod) {
             super(chmod);
             this.originalFile = originalFile;
+            this.isFirstTimeVisit = isFirstTimeVisit;
             this.expandedDir = expandedDir;
             this.entry = entry;
             this.zip = zip;
@@ -142,7 +145,7 @@ public void stopVisiting() {
         public File getFile() {
             if (file == null) {
                 file = new File(expandedDir, entry.getName());
-                if (!file.exists()) {
+                if (isFirstTimeVisit) {
                     copyTo(file);
                 }
             }

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

@@ -830,7 +830,7 @@ class DependencySubstitutionRulesIntegrationTest extends AbstractIntegrationSpec
         resolve.expectGraph {
             root(":impl", "depsub:impl:") {
                 module("org.utils:api:2.0")
-                edge("project :api", "org.utils:api:2.0").byConflictResolution("between versions 1.6 and 2.0").selectedByRule()
+                edge("project :api", "org.utils:api:2.0").byConflictResolution("between versions 2.0 and 1.6").selectedByRule()
             }
         }
     }
@@ -952,7 +952,7 @@ class DependencySubstitutionRulesIntegrationTest extends AbstractIntegrationSpec
                 module("org.utils:b:1.3") {
                     module("org.utils:a:1.3")
                 }
-                edge("org.utils:a:1.2", "org.utils:a:1.3").byConflictResolution("between versions 1.2.1 and 1.3")
+                edge("org.utils:a:1.2", "org.utils:a:1.3").byConflictResolution("between versions 1.3 and 1.2.1")
             }
         }
     }
@@ -1278,7 +1278,7 @@ Required by:
         resolve.expectGraph {
             root(":", ":depsub:") {
                 edge("org:a:1.0", "org:a:2.0") {
-                    byConflictResolution("between versions 1.0 and 2.0")
+                    byConflictResolution("between versions 2.0 and 1.0")
                     module("org:c:1.0")
                 }
                 edge("foo:b:1.0", "org:b:1.0") {
@@ -1318,7 +1318,7 @@ Required by:
         resolve.expectGraph {
             root(":", ":depsub:") {
                 edge("org:a:1.0", "org:a:2.0") {
-                    byConflictResolution("between versions 1.0 and 2.0")
+                    byConflictResolution("between versions 2.0 and 1.0")
                     module("org:c:1.0")
                 }
                 edge("foo:bar:baz", "org:b:1.0") {
@@ -1377,7 +1377,7 @@ Required by:
         resolve.expectGraph {
             root(":", ":depsub:") {
                 edge("org:a:1.0", "org:c:2.0") {
-                    byConflictResolution("between versions 1.1 and 2.0")
+                    byConflictResolution("between versions 2.0 and 1.1")
                 }
                 module("org:a:2.0") {
                     module("org:b:2.0") {
@@ -1449,7 +1449,7 @@ Required by:
         resolve.expectGraph {
             root(":", ":depsub:") {
                 edge("org:a:1.0", "org:a:2.0") {
-                    byConflictResolution("between versions 1.0 and 2.0")
+                    byConflictResolution("between versions 2.0 and 1.0")
                     module("org:c:1.0")
                 }
                 edge("foo:bar:baz", "org:b:1.0") {

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

@@ -16,9 +16,11 @@
 
 package org.gradle.integtests.resolve
 
+
 import org.gradle.integtests.fixtures.executer.GradleContextualExecuter
 import spock.lang.IgnoreIf
 import spock.lang.Issue
+import spock.lang.Unroll
 
 /**
  * Demonstrates the resolution of dependency excludes in published module metadata.
@@ -109,7 +111,8 @@ task check(type: Sync) {
      *
      * Exclude is applied to dependency a->b
      */
-    def "dependency exclude for group or module applies to child module of dependency"() {
+    @Unroll
+    def "dependency exclude for group or module applies to child module of dependency (#excluded)"() {
         given:
         def expectResolved = ['a', 'b', 'c', 'd', 'e'] - expectExcluded
         repository {
@@ -202,7 +205,8 @@ task check(type: Sync) {
      *
      * Selective exclusions are applied to dependency a->b
      */
-    def "can exclude transitive dependencies"() {
+    @Unroll
+    def "can exclude transitive dependencies (#condition)"() {
         repository {
             'a:a:1.0' {
                 dependsOn group: 'b', artifact: 'b', version: '1.0', exclusions: excludes

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

@@ -156,7 +156,7 @@ class PublishedDependencyConstraintsIntegrationTest extends AbstractModuleDepend
                     }
                 }
                 if (available) {
-                    edge("org:foo:1.0","org:foo:1.1").byConflictResolution("between versions 1.0 and 1.1")
+                    edge("org:foo:1.0","org:foo:1.1").byConflictResolution("between versions 1.1 and 1.0")
                 } else {
                     module("org:foo:1.0")
                 }
@@ -221,7 +221,7 @@ class PublishedDependencyConstraintsIntegrationTest extends AbstractModuleDepend
                 }
                 module("org:first-level2:1.0") {
                     if (available) {
-                        edge("org:foo:1.0","org:foo:1.1").byConflictResolution("between versions 1.0 and 1.1")
+                        edge("org:foo:1.0","org:foo:1.1").byConflictResolution("between versions 1.1 and 1.0")
                     } else {
                         module("org:foo:1.0")
                     }
@@ -395,7 +395,7 @@ class PublishedDependencyConstraintsIntegrationTest extends AbstractModuleDepend
                 module("org:first-level:1.0") {
                     if (available) {
                         constraint("org:bar:1.1", "org:foo:1.1").selectedByRule()
-                        edge("org:foo:1.0", "org:foo:1.1").byConflictResolution("between versions 1.0 and 1.1")
+                        edge("org:foo:1.0", "org:foo:1.1").byConflictResolution("between versions 1.1 and 1.0")
                     } else {
                         module("org:foo:1.0")
                     }

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

@@ -398,7 +398,7 @@ class ResolveConfigurationDependenciesBuildOperationIntegrationTest extends Abst
         op.details.configurationName == "compile"
         op.failure == "org.gradle.api.artifacts.ResolveException: Could not resolve all dependencies for configuration ':compile'."
         failure.assertHasCause("""Conflict(s) found for the following module(s):
-  - org:leaf between versions 1.0 and 2.0""")
+  - org:leaf between versions 2.0 and 1.0""")
         op.result != null
         op.result.resolvedDependenciesCount == 2
     }

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

@@ -601,7 +601,7 @@ class RichVersionConstraintsIntegrationTest extends AbstractModuleDependencyReso
    Dependency path ':test:unspecified' --> 'org:foo:{strictly [0,1]}'""")
     }
 
-    void "should pass if strict version ranges overlap"() {
+    void    "should pass if strict version ranges overlap"() {
         given:
         repository {
             'org:foo' {

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

@@ -160,11 +160,11 @@ project(':tool') {
             root(":tool", "test:tool:") {
                 project(":api", "test:api:") {
                     configuration = "runtimeElements"
-                    edge("org:foo:1.3.3", "org:foo:1.4.4").byConflictResolution("between versions 1.3.3 and 1.4.4")
+                    edge("org:foo:1.3.3", "org:foo:1.4.4").byConflictResolution("between versions 1.4.4 and 1.3.3")
                 }
                 project(":impl", "test:impl:") {
                     configuration = "runtimeElements"
-                    module("org:foo:1.4.4").byConflictResolution("between versions 1.3.3 and 1.4.4")
+                    module("org:foo:1.4.4").byConflictResolution("between versions 1.4.4 and 1.3.3")
                 }
             }
         }
@@ -210,10 +210,10 @@ task resolve {
         resolve.expectGraph {
             root(":", "org:test:1.0") {
                 module("org:bar:1.0") {
-                    edge("org:foo:1.3.3", "org:foo:1.4.4").byConflictResolution("between versions 1.3.3 and 1.4.4")
+                    edge("org:foo:1.3.3", "org:foo:1.4.4").byConflictResolution("between versions 1.4.4 and 1.3.3")
                 }
                 module("org:baz:1.0") {
-                    module("org:foo:1.4.4").byConflictResolution("between versions 1.3.3 and 1.4.4")
+                    module("org:foo:1.4.4").byConflictResolution("between versions 1.4.4 and 1.3.3")
                 }
             }
         }
@@ -262,7 +262,7 @@ dependencies {
                 }
                 module("org:two:1.0") {
                     module("org:dep-2.0-bringer:1.0") {
-                        edge("org:dep:2.0", "org:dep:2.5").byConflictResolution("between versions 2.0 and 2.5")
+                        edge("org:dep:2.0", "org:dep:2.5").byConflictResolution("between versions 2.5 and 2.0")
                     }
                     module("org:control-1.2-bringer:1.0") {
                         module("org:control:1.2")

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

@@ -107,7 +107,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec {
                     module('org:core:1.1')
                 }
                 module("outside:module:1.0") {
-                    edge('org:core:1.0', 'org:core:1.1').byConflictResolution("between versions 1.0 and 1.1")
+                    edge('org:core:1.0', 'org:core:1.1').byConflictResolution("between versions 1.1 and 1.0")
                 }
             }
         }
@@ -187,7 +187,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec {
             root(":", ":test:") {
                 module("org:xml:1.0") {
                     edge('org:core:1.0', 'org:core:1.1')
-                        .byConflictResolution("between versions 1.0 and 1.1")
+                        .byConflictResolution("between versions 1.1 and 1.0")
                         .byConstraint("belongs to platform org:platform:1.1")
                 }
                 module("org:json:1.1") {
@@ -529,7 +529,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec {
                 }
                 module('org2:foo:1.0') {
                     edge('org4:a:1.0', 'org4:a:1.1') {
-                        byConflictResolution("between versions 1.0 and 1.1")
+                        byConflictResolution("between versions 1.1 and 1.0")
                     }
                 }
                 module('org3:bar:1.0') {
@@ -596,7 +596,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec {
                     byConstraint("belongs to platform org:platform:1.1")
                     // byReason("version 1.1 is buggy") // TODO CC: uncomment when we collect rejection from component selection rule
                     edge('org:core:1.0', 'org:core:1.1') {
-                        byConflictResolution("between versions 1.0 and 1.1")
+                        byConflictResolution("between versions 1.1 and 1.0")
                     }
                 }
                 module("org:json:1.1") {
@@ -731,7 +731,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec {
                     module("org.apache.groovy:core:2.5")
                 }
                 edge("org.springframework:core:1.0", "org.springframework:core:1.1") {
-                    byConflictResolution("between versions 1.0 and 1.1")
+                    byConflictResolution("between versions 1.1 and 1.0")
                 }
             }
         }
@@ -813,7 +813,7 @@ class AlignmentIntegrationTest extends AbstractAlignmentSpec {
                     module("org.apache.groovy:core:2.5")
                 }
                 edge("org.springframework:core:1.0", "org.springframework:core:1.1") {
-                    byConflictResolution("between versions 1.0 and 1.1")
+                    byConflictResolution("between versions 1.1 and 1.0")
                 }
             }
         }

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

@@ -78,7 +78,7 @@ class ResolutionResultApiIntegrationTest extends AbstractDependencyResolutionTes
         then:
         output.contains """
 cool-project:5.0 root
-foo:1.0 between versions 0.5 and 1.0
+foo:1.0 between versions 1.0 and 0.5
 leaf:2.0 forced
 bar:1.0 requested
 baz:1.0 requested
@@ -129,8 +129,8 @@ baz:1.0 requested
                             descriptions.each {
                                 println "\$it.cause : \$it.description"
                             }
-                            def descriptor = descriptions.find { it.cause == ComponentSelectionCause.REQUESTED }
-                            assert descriptor?.description == 'second reason'
+                            def descriptors = descriptions.findAll { it.cause == ComponentSelectionCause.REQUESTED }
+                            assert descriptors.description == ['first reason', 'second reason']
                         }
                     }
                 }
@@ -200,8 +200,8 @@ baz:1.0 requested
                             descriptions.each {
                                 println "\$it.cause : \$it.description"
                             }
-                            def descriptor = descriptions.find { it.cause == ComponentSelectionCause.REQUESTED }
-                            assert descriptor?.description == 'second reason'
+                            def descriptors = descriptions.findAll { it.cause == ComponentSelectionCause.REQUESTED }
+                            assert descriptors.description == ['requested', 'second reason']
                         }
                     }
                 }
@@ -217,14 +217,14 @@ baz:1.0 requested
             root(":", ":test:") {
                 module('org.test:a:1.0:runtime') {
                     edge('org.test:leaf:0.9', 'org.test:leaf:1.1')
-                        .byConflictResolution("between versions 1.0 and 1.1") // conflict with the version requested by 'b'
+                        .byConflictResolution("between versions 1.1 and 1.0") // conflict with the version requested by 'b'
                         .byReason('second reason') // this comes from 'b'
                         .selectedByRule("substitute 0.9 with 1.0")
                 }
                 module('org.test:b:1.0:runtime') {
                     module('org.test:leaf:1.1')
                         .selectedByRule("substitute 0.9 with 1.0")
-                        .byConflictResolution("between versions 1.0 and 1.1")
+                        .byConflictResolution("between versions 1.1 and 1.0")
                         .byReason('second reason')
                 }
             }

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

@@ -355,7 +355,7 @@ class MultipleVariantSelectionIntegrationTest extends AbstractModuleDependencyRe
         resolve.expectGraph {
             root(":", ":test:") {
                 edge('org:foo:1.0', 'org:foo:1.1') {
-                    byConflictResolution('between versions 1.0 and 1.1')
+                    byConflictResolution('between versions 1.1 and 1.0')
                     // the following assertion is true but limitations to the test fixtures make it hard to check
                     //variant('altruntime', [custom: 'c3', 'org.gradle.status': defaultStatus()])
                     variant('runtime', [custom: 'c2', 'org.gradle.status': defaultStatus(), 'org.gradle.usage': 'java-runtime-jars'])
@@ -586,7 +586,7 @@ class MultipleVariantSelectionIntegrationTest extends AbstractModuleDependencyRe
         resolve.expectGraph {
             root(":", ":test:") {
                 edge('org:foo:1.0', 'org:foo:1.1') {
-                    byConflictResolution('between versions 1.0 and 1.1')
+                    byConflictResolution('between versions 1.1 and 1.0')
                     variant('runtime', [custom: 'c2', 'org.gradle.status': defaultStatus(), 'org.gradle.usage': 'java-runtime-jars'])
                     artifact group: 'org', module: 'foo', version: '1.0', classifier: 'c2'
                 }

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

@@ -91,7 +91,7 @@ class DependencyConstraintsAndResolutionStrategiesIntegrationTest extends Abstra
 
         then:
         failure.assertHasCause """Conflict(s) found for the following module(s):
-  - org:foo between versions 1.0 and 1.1"""
+  - org:foo between versions 1.1 and 1.0"""
     }
 
     void "dependency substitution rules are applied to dependency constraints"() {