Skip to content

Commit 7078d92

Browse files
snazysnazy
authored
[MSHADE-413] Fix endless loop caused by manipulating shared objects (#124)
* Add IT * [MSHADE-413] Fix endless loop caused by manipulating shared objects Maven objects (like `Dependency` or `Model`) returned by Maven must not be modified. Doing so will result in endless loops or incorrect builds.
1 parent 0945bcb commit 7078d92

File tree

5 files changed

+296
-26
lines changed

5 files changed

+296
-26
lines changed

‎src/it/projects/MSHADE-413-parallel/invoker.properties

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
invoker.timeoutInSeconds=60
19+
invoker.goals = clean install -T2 -pl p1/ -pl p2/

‎src/it/projects/MSHADE-413-parallel/p1/pom.xml

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
Licensed to the Apache Software Foundation (ASF) under one
5+
or more contributor license agreements. See the NOTICE file
6+
distributed with this work for additional information
7+
regarding copyright ownership. The ASF licenses this file
8+
to you under the Apache License, Version 2.0 (the
9+
"License"); you may not use this file except in compliance
10+
with the License. You may obtain a copy of the License at
11+
12+
http://www.apache.org/licenses/LICENSE-2.0
13+
14+
Unless required by applicable law or agreed to in writing,
15+
software distributed under the License is distributed on an
16+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
KIND, either express or implied. See the License for the
18+
specific language governing permissions and limitations
19+
under the License.
20+
-->
21+
22+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
23+
<modelVersion>4.0.0</modelVersion>
24+
25+
<parent>
26+
<groupId>org.apache.maven.its.shade.parallel</groupId>
27+
<artifactId>mshade413-parent</artifactId>
28+
<version>1.0</version>
29+
</parent>
30+
31+
<artifactId>mshade413-p1</artifactId>
32+
<version>1.0</version>
33+
34+
<name>MSHADE-413-p1</name>
35+
36+
<dependencies>
37+
<dependency>
38+
<groupId>org.projectnessie</groupId>
39+
<artifactId>nessie-spark-extensions-base</artifactId>
40+
<version>0.22.0</version>
41+
</dependency>
42+
<dependency>
43+
<groupId>org.projectnessie</groupId>
44+
<artifactId>nessie-spark-extensions-base</artifactId>
45+
<version>0.22.0</version>
46+
<classifier>tests</classifier>
47+
<scope>test</scope>
48+
</dependency>
49+
<dependency>
50+
<groupId>org.apache.iceberg</groupId>
51+
<artifactId>iceberg-spark3-runtime</artifactId>
52+
<version>0.13.1</version>
53+
<scope>test</scope>
54+
</dependency>
55+
<dependency>
56+
<groupId>org.junit.jupiter</groupId>
57+
<artifactId>junit-jupiter-api</artifactId>
58+
<version>5.8.2</version>
59+
<scope>test</scope>
60+
</dependency>
61+
</dependencies>
62+
63+
<build>
64+
<plugins>
65+
<plugin>
66+
<groupId>org.apache.maven.plugins</groupId>
67+
<artifactId>maven-shade-plugin</artifactId>
68+
<version>@project.version@</version>
69+
<configuration>
70+
<artifactSet>
71+
<includes>
72+
<include>org.projectnessie</include>
73+
</includes>
74+
</artifactSet>
75+
</configuration>
76+
<executions>
77+
<execution>
78+
<phase>package</phase>
79+
<goals>
80+
<goal>shade</goal>
81+
</goals>
82+
</execution>
83+
</executions>
84+
</plugin>
85+
</plugins>
86+
</build>
87+
</project>

‎src/it/projects/MSHADE-413-parallel/p2/pom.xml

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
Licensed to the Apache Software Foundation (ASF) under one
5+
or more contributor license agreements. See the NOTICE file
6+
distributed with this work for additional information
7+
regarding copyright ownership. The ASF licenses this file
8+
to you under the Apache License, Version 2.0 (the
9+
"License"); you may not use this file except in compliance
10+
with the License. You may obtain a copy of the License at
11+
12+
http://www.apache.org/licenses/LICENSE-2.0
13+
14+
Unless required by applicable law or agreed to in writing,
15+
software distributed under the License is distributed on an
16+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
KIND, either express or implied. See the License for the
18+
specific language governing permissions and limitations
19+
under the License.
20+
-->
21+
22+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
23+
<modelVersion>4.0.0</modelVersion>
24+
25+
<parent>
26+
<groupId>org.apache.maven.its.shade.parallel</groupId>
27+
<artifactId>mshade413-parent</artifactId>
28+
<version>1.0</version>
29+
</parent>
30+
31+
<artifactId>mshade413-p2</artifactId>
32+
<version>1.0</version>
33+
34+
<name>MSHADE-413-p2</name>
35+
36+
<dependencies>
37+
<dependency>
38+
<groupId>org.projectnessie</groupId>
39+
<artifactId>nessie-spark-extensions-base</artifactId>
40+
<version>0.22.0</version>
41+
</dependency>
42+
<dependency>
43+
<groupId>org.projectnessie</groupId>
44+
<artifactId>nessie-spark-extensions-base</artifactId>
45+
<version>0.22.0</version>
46+
<classifier>tests</classifier>
47+
<scope>test</scope>
48+
</dependency>
49+
<dependency>
50+
<groupId>org.apache.iceberg</groupId>
51+
<artifactId>iceberg-spark3-runtime</artifactId>
52+
<version>0.13.1</version>
53+
<scope>test</scope>
54+
</dependency>
55+
<dependency>
56+
<groupId>org.junit.jupiter</groupId>
57+
<artifactId>junit-jupiter-api</artifactId>
58+
<version>5.8.2</version>
59+
<scope>test</scope>
60+
</dependency>
61+
</dependencies>
62+
63+
<build>
64+
<plugins>
65+
<plugin>
66+
<groupId>org.apache.maven.plugins</groupId>
67+
<artifactId>maven-shade-plugin</artifactId>
68+
<version>@project.version@</version>
69+
<configuration>
70+
<artifactSet>
71+
<includes>
72+
<include>org.projectnessie</include>
73+
</includes>
74+
</artifactSet>
75+
</configuration>
76+
<executions>
77+
<execution>
78+
<phase>package</phase>
79+
<goals>
80+
<goal>shade</goal>
81+
</goals>
82+
</execution>
83+
</executions>
84+
</plugin>
85+
</plugins>
86+
</build>
87+
</project>

‎src/it/projects/MSHADE-413-parallel/pom.xml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
Licensed to the Apache Software Foundation (ASF) under one
5+
or more contributor license agreements. See the NOTICE file
6+
distributed with this work for additional information
7+
regarding copyright ownership. The ASF licenses this file
8+
to you under the Apache License, Version 2.0 (the
9+
"License"); you may not use this file except in compliance
10+
with the License. You may obtain a copy of the License at
11+
12+
http://www.apache.org/licenses/LICENSE-2.0
13+
14+
Unless required by applicable law or agreed to in writing,
15+
software distributed under the License is distributed on an
16+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
KIND, either express or implied. See the License for the
18+
specific language governing permissions and limitations
19+
under the License.
20+
-->
21+
22+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
23+
<modelVersion>4.0.0</modelVersion>
24+
25+
<groupId>org.apache.maven.its.shade.parallel</groupId>
26+
<artifactId>mshade413-parent</artifactId>
27+
<version>1.0</version>
28+
<packaging>pom</packaging>
29+
30+
<name>MSHADE-413</name>
31+
<description>
32+
Test that shade works in two parallel project builds.
33+
</description>
34+
35+
<modules>
36+
<module>p1</module>
37+
<module>p2</module>
38+
</modules>
39+
</project>

‎src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java

Lines changed: 64 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,10 +1061,6 @@ private File shadedTestArtifactFile()
10611061
private void createDependencyReducedPom( Set<String> artifactsToRemove )
10621062
throws IOException, DependencyGraphBuilderException, ProjectBuildingException
10631063
{
1064-
List<Dependency> dependencies = new ArrayList<>();
1065-
1066-
boolean modified = false;
1067-
10681064
List<Dependency> transitiveDeps = new ArrayList<>();
10691065

10701066
// NOTE: By using the getArtifacts() we get the completely evaluated artifacts
@@ -1083,39 +1079,48 @@ private void createDependencyReducedPom( Set<String> artifactsToRemove )
10831079
// we'll figure out the exclusions in a bit.
10841080
transitiveDeps.add( dep );
10851081
}
1086-
List<Dependency> origDeps = project.getDependencies();
10871082

1088-
if ( promoteTransitiveDependencies )
1083+
Model model = project.getOriginalModel();
1084+
1085+
// MSHADE-413: Must not use objects (for example `Model` or `Dependency`) that are "owned
1086+
// by Maven" and being used by other projects/plugins. Modifying those will break the
1087+
// correctness of the build - or cause an endless loop.
1088+
List<Dependency> origDeps = new ArrayList<>();
1089+
List<Dependency> source = promoteTransitiveDependencies ? transitiveDeps : project.getDependencies();
1090+
for ( Dependency d : source )
10891091
{
1090-
origDeps = transitiveDeps;
1092+
origDeps.add( d.clone() );
10911093
}
1094+
model = model.clone();
10921095

1093-
Model model = project.getOriginalModel();
10941096
// MSHADE-185: We will remove all system scoped dependencies which usually
10951097
// have some kind of property usage. At this time the properties within
10961098
// such things are already evaluated.
10971099
List<Dependency> originalDependencies = model.getDependencies();
10981100
removeSystemScopedDependencies( artifactsToRemove, originalDependencies );
10991101

1102+
List<Dependency> dependencies = new ArrayList<>();
1103+
boolean modified = false;
11001104
for ( Dependency d : origDeps )
11011105
{
1102-
dependencies.add( d );
1103-
1104-
String id = getId( d );
1105-
1106-
if ( artifactsToRemove.contains( id ) )
1106+
if ( artifactsToRemove.contains( getId( d ) ) )
11071107
{
1108-
modified = true;
1109-
11101108
if ( keepDependenciesWithProvidedScope )
11111109
{
1112-
d.setScope( "provided" );
1110+
if ( !"provided".equals( d.getScope() ) )
1111+
{
1112+
modified = true;
1113+
d.setScope( "provided" );
1114+
}
11131115
}
11141116
else
11151117
{
1116-
dependencies.remove( d );
1118+
modified = true;
1119+
continue;
11171120
}
11181121
}
1122+
1123+
dependencies.add( d );
11191124
}
11201125

11211126
// MSHADE-155
@@ -1299,8 +1304,13 @@ public boolean updateExcludesInDeps( MavenProject project, List<Dependency> depe
12991304
boolean modified = false;
13001305
for ( DependencyNode n2 : node.getChildren() )
13011306
{
1307+
String artifactId2 = getId( n2.getArtifact() );
1308+
13021309
for ( DependencyNode n3 : n2.getChildren() )
13031310
{
1311+
Artifact artifact3 = n3.getArtifact();
1312+
String artifactId3 = getId( artifact3 );
1313+
13041314
// check if it really isn't in the list of original dependencies. Maven
13051315
// prior to 2.0.8 may grab versions from transients instead of
13061316
// from the direct deps in which case they would be marked included
@@ -1310,7 +1320,7 @@ public boolean updateExcludesInDeps( MavenProject project, List<Dependency> depe
13101320
boolean found = false;
13111321
for ( Dependency dep : transitiveDeps )
13121322
{
1313-
if ( getId( dep ).equals( getId( n3.getArtifact() ) ) )
1323+
if ( getId( dep ).equals( artifactId3 ) )
13141324
{
13151325
found = true;
13161326
break;
@@ -1321,18 +1331,31 @@ public boolean updateExcludesInDeps( MavenProject project, List<Dependency> depe
13211331
// note: MSHADE-31 introduced the exclusion logic for promoteTransitiveDependencies=true,
13221332
// but as of 3.2.1 promoteTransitiveDependencies has no effect for provided deps,
13231333
// which makes this fix even possible (see also MSHADE-181)
1324-
if ( !found && !"provided".equals( n3.getArtifact().getScope() ) )
1334+
if ( !found && !"provided".equals( artifact3.getScope() ) )
13251335
{
1336+
getLog().debug( String.format( "dependency %s (scope %s) not found in transitive dependencies",
1337+
artifactId3, artifact3.getScope() ) );
13261338
for ( Dependency dep : dependencies )
13271339
{
1328-
if ( getId( dep ).equals( getId( n2.getArtifact() ) ) )
1340+
if ( getId( dep ).equals( artifactId2 ) )
13291341
{
1330-
Exclusion exclusion = new Exclusion();
1331-
exclusion.setArtifactId( n3.getArtifact().getArtifactId() );
1332-
exclusion.setGroupId( n3.getArtifact().getGroupId() );
1333-
dep.addExclusion( exclusion );
1334-
modified = true;
1335-
break;
1342+
// MSHADE-413: First check whether the exclusion has already been added,
1343+
// because it's meaningless to add it more than once. Certain cases
1344+
// can end up adding the exclusion "forever" and cause an endless loop
1345+
// rewriting the whole dependency-reduced-pom.xml file.
1346+
if ( !dependencyHasExclusion( dep, artifact3 ) )
1347+
{
1348+
getLog().debug( String.format( "Adding exclusion for dependency %s (scope %s) "
1349+
+ "to %s (scope %s)",
1350+
artifactId3, artifact3.getScope(),
1351+
getId( dep ), dep.getScope() ) );
1352+
Exclusion exclusion = new Exclusion();
1353+
exclusion.setArtifactId( artifact3.getArtifactId() );
1354+
exclusion.setGroupId( artifact3.getGroupId() );
1355+
dep.addExclusion( exclusion );
1356+
modified = true;
1357+
break;
1358+
}
13361359
}
13371360
}
13381361
}
@@ -1347,6 +1370,21 @@ public boolean updateExcludesInDeps( MavenProject project, List<Dependency> depe
13471370
}
13481371
}
13491372

1373+
private boolean dependencyHasExclusion( Dependency dep, Artifact exclusionToCheck )
1374+
{
1375+
boolean containsExclusion = false;
1376+
for ( Exclusion existingExclusion : dep.getExclusions() )
1377+
{
1378+
if ( existingExclusion.getGroupId().equals( exclusionToCheck.getGroupId() )
1379+
&& existingExclusion.getArtifactId().equals( exclusionToCheck.getArtifactId() ) )
1380+
{
1381+
containsExclusion = true;
1382+
break;
1383+
}
1384+
}
1385+
return containsExclusion;
1386+
}
1387+
13501388
private List<ResourceTransformer> toResourceTransformers(
13511389
String shade, List<ResourceTransformer> resourceTransformers )
13521390
{

0 commit comments

Comments
 (0)