Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove old and deprecated items, use Resolver API #336

Merged
merged 5 commits into from
Mar 17, 2023
Merged

Remove old and deprecated items, use Resolver API #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
da0439a
Make up some years of debt
cstamas Feb 16, 2023
e2395de
One IT fails
cstamas Mar 10, 2023
16a31e6
Merge remote-tracking branch 'origin/master' into make-up-10-years
slawekjaranowski Mar 16, 2023
20b8c1d
Use also project artifacts in createFlattenedDependenciesAll
slawekjaranowski Mar 17, 2023
968bbd7
Skip IT flatten-dependency-all-depmgnt-optional for Maven < 3.5.2
slawekjaranowski Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,20 +185,15 @@
</dependency>

<!-- other -->
<dependency>
<groupId>org.apache.maven.shared</groupId>
<artifactId>maven-dependency-tree</artifactId>
<version>3.2.1</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.5.1</version>
</dependency>
<dependency>
<groupId>org.apache.maven.shared</groupId>
<artifactId>maven-artifact-transfer</artifactId>
<version>0.13.1</version>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-interpolation</artifactId>
<version>1.26</version>
</dependency>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cstamas I use maven-artifact-transfer in one of the Maven plugins maintained by the Jenkins project and I was unaware that it was deprecated. If it is, what is its replacement and how should I migrate?

Copy link
Contributor Author

@cstamas cstamas Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@basil MAT was introduced to make transition from org.sonatype.aether Java package to org.eclipse.aether possible, and did it by use of Java reflection. This change happened when Aether (today Resolver) moved from Sonatype to Eclipse, between Maven 3.0 and Maven 3.1 (then it moved to ASF, but ASF got permission to use org.eclipse.aether package). The resolver packages will not be changed (for example to org.apache.maven.resolver) UNTIL resolver is also made "hidden" by Maven (like all of it's internals), and the new Maven4 API becomes the ONLY way to interact with Maven (and Resolver).

This is to happen in far future, as similarly like Maven 3.x supported 2.x and 3.x plugins, Maven 4.x will support 4.x (the new API) and 3.x (current, 3.1+ "API") plugins, hence the cut-oif is expected maybe in Maven 5, still undecided (but also is safely far in future).

Hence, if you do NOT support Maven 3.0.x anymore (hopefully not, even ASF raised the bar to 3.2.5, that is still almost 10 years of history), all you have to do is to use Resolver classes "directly", just like this PR does.

Ironically, this "direct use" of resolver classes was considered "bad practice" due Maven 3.0/Maven 3.1 transition fiasco (as plugins became "maven 3.0 only" or "maven 3.1 only", depending on which packages they used), and this got "imprinted" into maven devs, while this is actually NOT true from the moment you declare "supported" Maven versions as [3.1,) (3.1 and onwards). It was almost full 13 years since release of 3.0, Maven Project dropped support for Maven 3.0.x since long time as well. And similar resolver package change is not gonna happen in long term, again, probably in Maven 5 or so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we release mat? I found it useful for code simplification

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, our minimum Maven version is 3.81 so I filed jenkinsci/maven-hpi-plugin#469. I appreciate your explanation. I have inherited several old Maven plugins and have been trying to modernize them, but it has been very challenging for me since I am an outsider to the Maven developer community and these transitions are not documented very well. That makes your explanation particularly helpful. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you accept to not support Maven 3.0.x line (and anything older), it is totally safe to go for "direct use" of Resolver classes. Maven 4 introduces "new API" that will coexists with "old Maven 3.1+ API (am including resolver API here as well)" during Maven 4.x lifespan to offer transition period. It will be "cut off" (still undecided) probably in Maven 5, when the "new API" becomes the only way to interact with Maven, while all of it's internals and guts (unlike today, where you can access or inject just anything from Maven) becomes hidden from plugins.

Still, to be on safe side, please read this https://maven.apache.org/resolver/api-compatibility.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we release mat? I found it useful for code simplification

Problem with MAT is that it uses Maven2 ArtifactRepository type for a local repository of "modern maven3 library" (org.sonatype or org.eclipse, does not matter). See issues:

https://issues.apache.org/jira/browse/MNG-7706
https://issues.apache.org/jira/browse/MNG-7663


<!-- testing -->
Expand Down
2 changes: 2 additions & 0 deletions src/it/projects/flatten-dependency-all-depmgnt-optional/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Test fails on Maven < 3.5.2
invoker.maven.version = 3.5.2+
262 changes: 135 additions & 127 deletions src/main/java/org/codehaus/mojo/flatten/FlattenMojo.java
View file
Open in desktop

Large diffs are not rendered by default.

61 changes: 22 additions & 39 deletions src/main/java/org/codehaus/mojo/flatten/cifriendly/CiInterpolatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,31 +34,29 @@
import org.codehaus.plexus.interpolation.SimpleRecursionInterceptor;
import org.codehaus.plexus.interpolation.ValueSource;

/*
/**
* Based on StringSearchInterpolator from plexus-interpolation,
* see {@link org.codehaus.plexus.interpolation.StringSearchInterpolator}.
* This interpolates only the Maven CI Friendly variables revision, sha1 and changelist.
*/
public class CiInterpolatorImpl implements Interpolator
{

private Map existingAnswers = new HashMap();
private final Map existingAnswers = new HashMap();

private List<ValueSource> valueSources = new ArrayList<>();
private final List<ValueSource> valueSources = new ArrayList<>();

private List<InterpolationPostProcessor> postProcessors = new ArrayList<>();
private final List<InterpolationPostProcessor> postProcessors = new ArrayList<>();

private boolean cacheAnswers = false;

public static final String DEFAULT_START_EXPR = "${";

public static final String DEFAULT_END_EXPR = "}";

private String startExpr;

private String endExpr;
private final String startExpr;

private String escapeString;
private final String endExpr;

public CiInterpolatorImpl()
{
Expand All @@ -76,6 +74,7 @@ public CiInterpolatorImpl( String startExpr, String endExpr )
/**
* {@inheritDoc}
*/
@Override
public void addValueSource( ValueSource valueSource )
{
valueSources.add( valueSource );
Expand All @@ -84,6 +83,7 @@ public void addValueSource( ValueSource valueSource )
/**
* {@inheritDoc}
*/
@Override
public void removeValuesSource( ValueSource valueSource )
{
valueSources.remove( valueSource );
Expand All @@ -92,6 +92,7 @@ public void removeValuesSource( ValueSource valueSource )
/**
* {@inheritDoc}
*/
@Override
public void addPostProcessor( InterpolationPostProcessor postProcessor )
{
postProcessors.add( postProcessor );
Expand All @@ -100,23 +101,27 @@ public void addPostProcessor( InterpolationPostProcessor postProcessor )
/**
* {@inheritDoc}
*/
@Override
public void removePostProcessor( InterpolationPostProcessor postProcessor )
{
postProcessors.remove( postProcessor );
}

@Override
public String interpolate( String input, String thisPrefixPattern )
throws InterpolationException
{
return interpolate( input, new SimpleRecursionInterceptor() );
}

@Override
public String interpolate( String input, String thisPrefixPattern, RecursionInterceptor recursionInterceptor )
throws InterpolationException
{
return interpolate( input, recursionInterceptor );
}

@Override
public String interpolate( String input )
throws InterpolationException
{
Expand All @@ -127,6 +132,7 @@ public String interpolate( String input )
* Entry point for recursive resolution of an expression and all of its
* nested expressions.
*/
@Override
public String interpolate( String input, RecursionInterceptor recursionInterceptor )
throws InterpolationException
{
Expand Down Expand Up @@ -168,21 +174,6 @@ private String interpolate( String input, RecursionInterceptor recursionIntercep
final String wholeExpr = input.substring( startIdx, endIdx + endExpr.length() );
String realExpr = wholeExpr.substring( startExpr.length(), wholeExpr.length() - endExpr.length() );

if ( startIdx >= 0 && escapeString != null && escapeString.length() > 0 )
{
int startEscapeIdx = startIdx == 0 ? 0 : startIdx - escapeString.length();
if ( startEscapeIdx >= 0 )
{
String escape = input.substring( startEscapeIdx, startIdx );
if ( escape != null && escapeString.equals( escape ) )
{
result.append( wholeExpr );
result.replace( startEscapeIdx, startEscapeIdx + escapeString.length(), "" );
continue;
}
}
}

boolean resolved = false;
if ( !unresolvable.contains( wholeExpr ) )
{
Expand Down Expand Up @@ -235,7 +226,7 @@ private String interpolate( String input, RecursionInterceptor recursionIntercep
{
value = interpolate( String.valueOf( value ), recursionInterceptor, unresolvable );

if ( postProcessors != null && !postProcessors.isEmpty() )
if ( !postProcessors.isEmpty() )
{
for ( InterpolationPostProcessor postProcessor : postProcessors )
{
Expand All @@ -252,7 +243,7 @@ private String interpolate( String input, RecursionInterceptor recursionIntercep
// result = matcher.replaceFirst( stringValue );
// but this could result in multiple lookups of stringValue, and replaceAll is not correct
// behaviour
result.append( String.valueOf( value ) );
result.append( value );
resolved = true;
}
else
Expand All @@ -271,10 +262,7 @@ private String interpolate( String input, RecursionInterceptor recursionIntercep
result.append( wholeExpr );
}

if ( endIdx > -1 )
{
endIdx += endExpr.length() - 1;
}
endIdx += endExpr.length() - 1;
}

if ( endIdx == -1 && startIdx > -1 )
Expand All @@ -298,6 +286,7 @@ else if ( endIdx < input.length() )
* @return a {@link List} that may be interspersed with {@link String} and
* {@link Throwable} instances.
*/
@Override
public List getFeedback()
{
List<?> messages = new ArrayList();
Expand All @@ -316,6 +305,7 @@ public List getFeedback()
/**
* Clear the feedback messages from previous interpolate(..) calls.
*/
@Override
public void clearFeedback()
{
for ( ValueSource vs : valueSources )
Expand All @@ -324,30 +314,23 @@ public void clearFeedback()
}
}

@Override
public boolean isCacheAnswers()
{
return cacheAnswers;
}

@Override
public void setCacheAnswers( boolean cacheAnswers )
{
this.cacheAnswers = cacheAnswers;
}

@Override
public void clearAnswers()
{
existingAnswers.clear();
}

public String getEscapeString()
{
return escapeString;
}

public void setEscapeString( String escapeString )
{
this.escapeString = escapeString;
}

}

47 changes: 20 additions & 27 deletions src/main/java/org/codehaus/mojo/flatten/cifriendly/CiModelInterpolator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -47,8 +45,6 @@
import org.apache.maven.model.interpolation.ModelInterpolator;
import org.apache.maven.model.path.PathTranslator;
import org.apache.maven.model.path.UrlNormalizer;
import org.codehaus.plexus.component.annotations.Component;
import org.codehaus.plexus.component.annotations.Requirement;
import org.codehaus.plexus.interpolation.AbstractValueSource;
import org.codehaus.plexus.interpolation.InterpolationException;
import org.codehaus.plexus.interpolation.InterpolationPostProcessor;
Expand All @@ -62,23 +58,22 @@
import org.codehaus.plexus.interpolation.ValueSource;
import org.codehaus.plexus.interpolation.util.ValueSourceUtils;

/*
* Based on StringSearchModelInterpolator in maven-model-builder
/**
* Based on StringSearchModelInterpolator in maven-model-builder.
*
* IMPORTANT: this is a legacy Plexus component, with manually authored descriptor in src/main/resources!
*/
@Component( role = CiInterpolator.class )
public class CiModelInterpolator implements CiInterpolator, ModelInterpolator
{

public CiModelInterpolator()
{
interpolator = createInterpolator();
recursionInterceptor = new PrefixAwareRecursionInterceptor( PROJECT_PREFIXES );
}

private static final List<String> PROJECT_PREFIXES = Arrays.asList( "pom.", "project." );

private static final Collection<String> TRANSLATED_PATH_EXPRESSIONS;

private static final Map<Class<?>, InterpolateObjectAction.CacheItem> CACHED_ENTRIES = new ConcurrentHashMap<>(
80, 0.75f, 2 );
// Empirical data from 3.x, actual =40

static
{
Collection<String> translatedPrefixes = new HashSet<>();
Expand All @@ -101,18 +96,20 @@ public CiModelInterpolator()
TRANSLATED_PATH_EXPRESSIONS = translatedPrefixes;
}

private Interpolator interpolator;
private final Interpolator interpolator;

// There is a protected setter for this one?
private RecursionInterceptor recursionInterceptor;

@Requirement
private PathTranslator pathTranslator;

@Requirement
private UrlNormalizer urlNormalizer;

private static final Map<Class<?>, InterpolateObjectAction.CacheItem> CACHED_ENTRIES = new ConcurrentHashMap<>(
80, 0.75f, 2 );
// Empirical data from 3.x, actual =40
public CiModelInterpolator()
{
this.interpolator = createInterpolator();
this.recursionInterceptor = new PrefixAwareRecursionInterceptor( PROJECT_PREFIXES );
}

public Model interpolateModel( Model model, File projectDir, ModelBuildingRequest config,
ModelProblemCollector problems )
Expand All @@ -131,10 +128,7 @@ protected void interpolateObject( Object obj, Model model, File projectDir, Mode
List<? extends InterpolationPostProcessor> postProcessors =
createPostProcessors( model, projectDir, config );

InterpolateObjectAction action = new InterpolateObjectAction( obj, valueSources, postProcessors, this,
problems );

AccessController.doPrivileged( action );
new InterpolateObjectAction( obj, valueSources, postProcessors, this, problems ).run();
}
finally
{
Expand Down Expand Up @@ -205,7 +199,7 @@ protected Interpolator createInterpolator()
return interpolator;
}

private static final class InterpolateObjectAction implements PrivilegedAction<Object>
private static final class InterpolateObjectAction implements Runnable
{

private final LinkedList<Object> interpolationTargets;
Expand Down Expand Up @@ -234,16 +228,15 @@ private static final class InterpolateObjectAction implements PrivilegedAction<O
this.problems = problems;
}

public Object run()
@Override
public void run()
{
while ( !interpolationTargets.isEmpty() )
{
Object obj = interpolationTargets.removeFirst();

traverseObjectWithParents( obj.getClass(), obj );
}

return null;
}

private String interpolate( String value )
Expand Down
Loading