You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2021/10/16 12:56:50 UTC
[maven] branch MNG-7312 created (now 76dd2fe)
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a change to branch MNG-7312
in repository https://gitbox.apache.org/repos/asf/maven.git.
at 76dd2fe [MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251
This branch includes the following new commits:
new 76dd2fe [MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[maven] 01/01: [MNG-7312] Revert ThreadLocal approach from MNG-6843
and MNG-7251
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch MNG-7312
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 76dd2feb25ce8b3bc75d8d31f44bbbe41202d807
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sat Oct 16 14:32:24 2021 +0200
[MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251
Revert "[MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project"
This reverts commit a6e462b53a4b6c87ef43f6cfe7fbde0b0939e3d6.
Revert "[MNG-6843] Parallel build fails due to missing JAR artifacts in compilePath"
This reverts commit 73e00ed85df84ba0c557dd020740812b2453f2d3.
---
.../org/apache/maven/project/MavenProject.java | 87 +++++++++++-----------
.../org/apache/maven/project/MavenProjectTest.java | 43 -----------
2 files changed, 43 insertions(+), 87 deletions(-)
diff --git a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
index bbafcdf..57069a5 100644
--- a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
+++ b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
@@ -111,6 +111,10 @@ public class MavenProject
private Set<Artifact> resolvedArtifacts;
+ private ArtifactFilter artifactFilter;
+
+ private Set<Artifact> artifacts;
+
private Artifact parentArtifact;
private Set<Artifact> pluginArtifacts;
@@ -147,7 +151,8 @@ public class MavenProject
private Artifact artifact;
- private ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new );
+ // calculated.
+ private Map<String, Artifact> artifactMap;
private Model originalModel;
@@ -690,11 +695,10 @@ public class MavenProject
public void setArtifacts( Set<Artifact> artifacts )
{
- ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
- artifactsHolder.artifacts = artifacts;
+ this.artifacts = artifacts;
// flush the calculated artifactMap
- artifactsHolder.artifactMap = null;
+ artifactMap = null;
}
/**
@@ -707,36 +711,34 @@ public class MavenProject
*/
public Set<Artifact> getArtifacts()
{
- ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
- if ( artifactsHolder.artifacts == null )
+ if ( artifacts == null )
{
- if ( artifactsHolder.artifactFilter == null || resolvedArtifacts == null )
+ if ( artifactFilter == null || resolvedArtifacts == null )
{
- artifactsHolder.artifacts = new LinkedHashSet<>();
+ artifacts = new LinkedHashSet<>();
}
else
{
- artifactsHolder.artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 );
+ artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 );
for ( Artifact artifact : resolvedArtifacts )
{
- if ( artifactsHolder.artifactFilter.include( artifact ) )
+ if ( artifactFilter.include( artifact ) )
{
- artifactsHolder.artifacts.add( artifact );
+ artifacts.add( artifact );
}
}
}
}
- return artifactsHolder.artifacts;
+ return artifacts;
}
public Map<String, Artifact> getArtifactMap()
{
- ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
- if ( artifactsHolder.artifactMap == null )
+ if ( artifactMap == null )
{
- artifactsHolder.artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() );
+ artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() );
}
- return artifactsHolder.artifactMap;
+ return artifactMap;
}
public void setPluginArtifacts( Set<Artifact> pluginArtifacts )
@@ -1176,8 +1178,7 @@ public class MavenProject
{
throw new UnsupportedOperationException( e );
}
- // clone must have its own TL, otherwise the artifacts are intermingled!
- clone.threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new );
+
clone.deepCopy( this );
return clone;
@@ -1220,7 +1221,6 @@ public class MavenProject
// copy fields
file = project.file;
basedir = project.basedir;
- threadLocalArtifactsHolder.set( project.threadLocalArtifactsHolder.get().copy() );
// don't need a deep copy, they don't get modified or added/removed to/from - but make them unmodifiable to be
// sure!
@@ -1229,6 +1229,11 @@ public class MavenProject
setDependencyArtifacts( Collections.unmodifiableSet( project.getDependencyArtifacts() ) );
}
+ if ( project.getArtifacts() != null )
+ {
+ setArtifacts( Collections.unmodifiableSet( project.getArtifacts() ) );
+ }
+
if ( project.getParentFile() != null )
{
parentFile = new File( project.getParentFile().getAbsolutePath() );
@@ -1422,10 +1427,9 @@ public class MavenProject
*/
public void setResolvedArtifacts( Set<Artifact> artifacts )
{
- this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.emptySet();
- ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
- artifactsHolder.artifacts = null;
- artifactsHolder.artifactMap = null;
+ this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.<Artifact>emptySet();
+ this.artifacts = null;
+ this.artifactMap = null;
}
/**
@@ -1438,10 +1442,9 @@ public class MavenProject
*/
public void setArtifactFilter( ArtifactFilter artifactFilter )
{
- ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
- artifactsHolder.artifactFilter = artifactFilter;
- artifactsHolder.artifacts = null;
- artifactsHolder.artifactMap = null;
+ this.artifactFilter = artifactFilter;
+ this.artifacts = null;
+ this.artifactMap = null;
}
/**
@@ -1471,7 +1474,13 @@ public class MavenProject
// ----------------------------------------------------------------------------------------------------------------
//
- // D E P R E C A T E D - Everything below will be removed for Maven 4.0.0
+ //
+ // D E P R E C A T E D
+ //
+ //
+ // ----------------------------------------------------------------------------------------------------------------
+ //
+ // Everything below will be removed for Maven 4.0.0
//
// ----------------------------------------------------------------------------------------------------------------
@@ -1492,6 +1501,7 @@ public class MavenProject
if ( moduleFile != null )
{
File moduleDir = moduleFile.getCanonicalFile().getParentFile();
+
module = moduleDir.getName();
}
@@ -1812,6 +1822,7 @@ public class MavenProject
public void setReportArtifacts( Set<Artifact> reportArtifacts )
{
this.reportArtifacts = reportArtifacts;
+
reportArtifactMap = null;
}
@@ -1828,6 +1839,7 @@ public class MavenProject
{
reportArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getReportArtifacts() );
}
+
return reportArtifactMap;
}
@@ -1835,6 +1847,7 @@ public class MavenProject
public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
{
this.extensionArtifacts = extensionArtifacts;
+
extensionArtifactMap = null;
}
@@ -1851,6 +1864,7 @@ public class MavenProject
{
extensionArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getExtensionArtifacts() );
}
+
return extensionArtifactMap;
}
@@ -1868,6 +1882,7 @@ public class MavenProject
public Xpp3Dom getReportConfiguration( String pluginGroupId, String pluginArtifactId, String reportSetId )
{
Xpp3Dom dom = null;
+
// ----------------------------------------------------------------------
// I would like to be able to lookup the Mojo object using a key but
// we have a limitation in modello that will be remedied shortly. So
@@ -1971,20 +1986,4 @@ public class MavenProject
{
this.projectBuilderConfiguration = projectBuildingRequest;
}
-
- private static class ArtifactsHolder
- {
- private ArtifactFilter artifactFilter;
- private Set<Artifact> artifacts;
- private Map<String, Artifact> artifactMap;
-
- ArtifactsHolder copy()
- {
- ArtifactsHolder copy = new ArtifactsHolder();
- copy.artifactFilter = artifactFilter;
- copy.artifacts = artifacts != null ? new LinkedHashSet<>( artifacts ) : null;
- copy.artifactMap = artifactMap != null ? new LinkedHashMap<>( artifactMap ) : null;
- return copy;
- }
- }
}
diff --git a/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java b/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
index aff0dde..d2cba20 100644
--- a/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
+++ b/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
@@ -21,19 +21,14 @@ package org.apache.maven.project;
import java.io.File;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
-import org.apache.maven.artifact.Artifact;
import org.apache.maven.model.DependencyManagement;
import org.apache.maven.model.Model;
import org.apache.maven.model.Parent;
import org.apache.maven.model.Profile;
import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -209,44 +204,6 @@ public class MavenProjectTest
}
@Test
- public void testCloneWithArtifacts()
- throws InterruptedException
- {
- Artifact initialArtifact = Mockito.mock( Artifact.class, "initialArtifact" );
- MavenProject originalProject = new MavenProject();
- originalProject.setArtifacts( Collections.singleton( initialArtifact ) );
- assertEquals( Collections.singleton( initialArtifact ), originalProject.getArtifacts(),
- "Sanity check: originalProject returns artifact that has just been set" );
-
- final MavenProject clonedProject = originalProject.clone();
-
- assertEquals( Collections.singleton( initialArtifact ), clonedProject.getArtifacts(),
- "Cloned project returns the artifact that was set for the original project" );
-
- Artifact anotherArtifact = Mockito.mock( Artifact.class, "anotherArtifact" );
- clonedProject.setArtifacts( Collections.singleton( anotherArtifact ) );
- assertEquals( Collections.singleton( anotherArtifact ), clonedProject.getArtifacts(),
- "Sanity check: clonedProject returns artifact that has just been set" );
-
- assertEquals( Collections.singleton( initialArtifact ), originalProject.getArtifacts(),
- "Original project returns the artifact that was set initially (not the one for clonedProject)" );
-
- final AtomicReference<Set<Artifact>> artifactsFromThread = new AtomicReference<>();
- Thread thread = new Thread( new Runnable()
- {
- @Override
- public void run()
- {
- artifactsFromThread.set( clonedProject.getArtifacts() );
- }
- } );
- thread.start();
- thread.join();
-
- assertEquals( Collections.emptySet(), artifactsFromThread.get(),
- "Another thread does not see the same artifacts" );
- }
-
public void testUndefinedOutputDirectory()
throws Exception
{