Hello Jason,
Please revert this change or make it backward comaptible. Removing
getArtifactHandler() from Artifact interface might be conceptually
right, but it breaks lots of thing.
1. a few tests are broken in the maven components.
2. More important is that every plugin have ever made that call will
break with a nasty NoSuchMethodException when run in 2.1.
A quick search in mojo.codehaus.org reveals that
appassembler-maven-plugin,
aspectj-maven-plugin,
cobertura-maven-plugin,
commons-attributes-maven-plugin,
native-maven-plugin,
taglist-maven-plugin
and webstart-maven-plugin are all affected.
(Done by grepping the current trunk, not counting stuff from sandbox)
3.You've also broken my code in the netbeans integration that uses the
Artifact's getArtifactHandler method as well. I can rewrite my own
code against the latest embedder, however point 2. still applies.
People won't be able to run many projects with the embedded version of
maven and will have to use some 2.0.x command line version. And of
course everyone will blame me :)
A more general note.
The light-heartedness of making non-backward compatible changes in
maven components and the core code in general gives me nightmares.
Plugins can reach any component from the maven core via the
${components.*} property, the MavenProject instance via ${project.*},
the MavenSession and Settings also via the property evaluation.
Whoever added this, effectively commited to making all those
components backward compatible public API. You might get away with
adding methods to the interfaces (it's plugins responsibility to track
when the method was added and setup <mavenPrerequisite>
approproately).
But the removal of any method or changing the signature is unacceptable.
Thanks.
Milos
On Feb 3, 2008 5:19 AM, <[EMAIL PROTECTED]> wrote:
> Author: jvanzyl
> Date: Sat Feb 2 20:19:44 2008
> New Revision: 617944
>
> URL: http://svn.apache.org/viewvc?rev=617944&view=rev
> Log:
> o The Artifact should serve as datum, or representation of resource in a
> Maven repository. It currently has way too much information, and
> also has references to components. In a first pass I have decoupled the
> ArtifactHandler from Artifact so that it is no longer required.
> This means that the primary requirement of the ArtifactFactory is gone and
> you can now use a simple constructor to create an
> Artifact.
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/Artifact.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/ArtifactUtils.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/DefaultArtifact.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/deployer/DefaultArtifactDeployer.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/factory/DefaultArtifactFactory.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/DefaultRepositoryLayout.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java
>
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/LegacyRepositoryLayout.java
>
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java
>
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/Artifact.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/Artifact.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/Artifact.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/Artifact.java
> Sat Feb 2 20:19:44 2008
> @@ -119,8 +119,6 @@
>
> void setDependencyFilter( ArtifactFilter artifactFilter );
>
> - ArtifactHandler getArtifactHandler();
> -
> List getDependencyTrail();
>
> void setDependencyTrail( List dependencyTrail );
> @@ -144,9 +142,6 @@
> boolean isResolved();
>
> void setResolvedVersion( String version );
> -
> - /** @todo remove, a quick hack for the lifecycle executor */
> - void setArtifactHandler( ArtifactHandler handler );
>
> boolean isRelease();
>
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/ArtifactUtils.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/ArtifactUtils.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/ArtifactUtils.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/ArtifactUtils.java
> Sat Feb 2 20:19:44 2008
> @@ -150,10 +150,10 @@
> {
> range = VersionRange.createFromVersion( artifact.getVersion() );
> }
> -
> +
> DefaultArtifact clone = new DefaultArtifact( artifact.getGroupId(),
> artifact.getArtifactId(), range.cloneOf(),
> - artifact.getScope(), artifact.getType(),
> artifact.getClassifier(),
> - artifact.getArtifactHandler(), artifact.isOptional() );
> + artifact.getScope(), artifact.getType(),
> artifact.getClassifier(), artifact.isOptional() );
> +
> clone.setRelease( artifact.isRelease() );
> clone.setResolvedVersion( artifact.getVersion() );
> clone.setResolved( artifact.isResolved() );
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/DefaultArtifact.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/DefaultArtifact.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/DefaultArtifact.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/DefaultArtifact.java
> Sat Feb 2 20:19:44 2008
> @@ -37,7 +37,7 @@
> import java.util.regex.Matcher;
>
> /**
> - * @author <a href="mailto:[EMAIL PROTECTED]">Jason van Zyl </a>
> + * @author Jason van Zyl
> * @version $Id$
> * @todo this should possibly be replaced by type handler
> */
> @@ -48,13 +48,6 @@
>
> private String artifactId;
>
> - /**
> - * The resolved version for the artifact after conflict resolution, that
> has not been transformed.
> - *
> - * @todo should be final
> - */
> - private String baseVersion;
> -
> private final String type;
>
> private final String classifier;
> @@ -63,28 +56,40 @@
>
> private File file;
>
> + // Why is this here? What repository is determined at runtime and is
> therefore a
> + // runtime charactistic. This needs to go. jvz.
> private ArtifactRepository repository;
>
> private String downloadUrl;
>
> + // Why is this here? jvz.
> private ArtifactFilter dependencyFilter;
>
> - private ArtifactHandler artifactHandler;
> -
> + // Why is this here? jvz?
> private List dependencyTrail;
>
> + /**
> + * The resolved version for the artifact after conflict resolution, that
> has not been transformed.
> + *
> + * @todo should be final
> + */
> + private String baseVersion;
> +
> private String version;
>
> private VersionRange versionRange;
>
> private boolean resolved;
>
> + // This is specific to maven. jvz.
> private boolean release;
>
> + // If the version is stored here (above), why on earth do we store the
> available versions here? jvz.
> private List availableVersions;
>
> private Map metadataMap;
>
> + // This is Maven specific. jvz/
> private boolean optional;
>
> public DefaultArtifact( String groupId,
> @@ -92,10 +97,9 @@
> VersionRange versionRange,
> String scope,
> String type,
> - String classifier,
> - ArtifactHandler artifactHandler )
> + String classifier )
> {
> - this( groupId, artifactId, versionRange, scope, type, classifier,
> artifactHandler, false );
> + this( groupId, artifactId, versionRange, scope, type, classifier,
> false );
> }
>
> public DefaultArtifact( String groupId,
> @@ -104,7 +108,6 @@
> String scope,
> String type,
> String classifier,
> - ArtifactHandler artifactHandler,
> boolean optional )
> {
> this.groupId = groupId;
> @@ -115,17 +118,10 @@
>
> selectVersionFromNewRangeIfAvailable();
>
> - this.artifactHandler = artifactHandler;
> -
> this.scope = scope;
>
> this.type = type;
>
> - if ( classifier == null )
> - {
> - classifier = artifactHandler.getClassifier();
> - }
> -
> this.classifier = classifier;
>
> this.optional = optional;
> @@ -474,11 +470,6 @@
> dependencyFilter = artifactFilter;
> }
>
> - public ArtifactHandler getArtifactHandler()
> - {
> - return artifactHandler;
> - }
> -
> public List getDependencyTrail()
> {
> return dependencyTrail;
> @@ -561,11 +552,6 @@
> {
> this.version = version;
> // retain baseVersion
> - }
> -
> - public void setArtifactHandler( ArtifactHandler artifactHandler )
> - {
> - this.artifactHandler = artifactHandler;
> }
>
> public void setRelease( boolean release )
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/deployer/DefaultArtifactDeployer.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/deployer/DefaultArtifactDeployer.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/deployer/DefaultArtifactDeployer.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/deployer/DefaultArtifactDeployer.java
> Sat Feb 2 20:19:44 2008
> @@ -20,6 +20,7 @@
> */
>
> import org.apache.maven.artifact.Artifact;
> +import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
> import org.apache.maven.artifact.manager.WagonManager;
> import org.apache.maven.artifact.metadata.ArtifactMetadata;
> import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException;
> @@ -63,6 +64,9 @@
> /** @plexus.requirement role-hint="default" */
> private ArtifactRepositoryLayout defaultLayout;
>
> + /** @plexus.requirement */
> + private ArtifactHandlerManager artifactHandlerManager;
> +
> /** @deprecated we want to use the artifact method only, and ensure
> artifact.file is set correctly. */
> public void deploy( String basedir,
> String finalName,
> @@ -71,7 +75,8 @@
> ArtifactRepository localRepository )
> throws ArtifactDeploymentException
> {
> - String extension = artifact.getArtifactHandler().getExtension();
> + String extension = artifactHandlerManager.getArtifactHandler(
> artifact.getType() ).getExtension();
> +
> File source = new File( basedir, finalName + "." + extension );
> deploy( source, artifact, deploymentRepository, localRepository );
> }
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/factory/DefaultArtifactFactory.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/factory/DefaultArtifactFactory.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/factory/DefaultArtifactFactory.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/factory/DefaultArtifactFactory.java
> Sat Feb 2 20:19:44 2008
> @@ -211,9 +211,6 @@
> desiredScope = Artifact.SCOPE_SYSTEM;
> }
>
> - ArtifactHandler handler = artifactHandlerManager.getArtifactHandler(
> type );
> -
> - return new DefaultArtifact( groupId, artifactId, versionRange,
> desiredScope, type, classifier, handler,
> - optional );
> + return new DefaultArtifact( groupId, artifactId, versionRange,
> desiredScope, type, classifier, optional );
> }
> }
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/installer/DefaultArtifactInstaller.java
> Sat Feb 2 20:19:44 2008
> @@ -20,6 +20,7 @@
> */
>
> import org.apache.maven.artifact.Artifact;
> +import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
> import org.apache.maven.artifact.metadata.ArtifactMetadata;
> import org.apache.maven.artifact.repository.ArtifactRepository;
> import
> org.apache.maven.artifact.repository.metadata.RepositoryMetadataInstallationException;
> @@ -46,6 +47,9 @@
> /** @plexus.requirement */
> private RepositoryMetadataManager repositoryMetadataManager;
>
> + /** @plexus.requirement */
> + private ArtifactHandlerManager artifactHandlerManager;
> +
> /** @deprecated we want to use the artifact method only, and ensure
> artifact.file is set correctly. */
> public void install( String basedir,
> String finalName,
> @@ -53,7 +57,8 @@
> ArtifactRepository localRepository )
> throws ArtifactInstallationException
> {
> - String extension = artifact.getArtifactHandler().getExtension();
> + String extension = artifactHandlerManager.getArtifactHandler(
> artifact.getType() ).getExtension();
> +
> File source = new File( basedir, finalName + "." + extension );
>
> install( source, artifact, localRepository );
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/DefaultRepositoryLayout.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/DefaultRepositoryLayout.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/DefaultRepositoryLayout.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/DefaultRepositoryLayout.java
> Sat Feb 2 20:19:44 2008
> @@ -21,6 +21,7 @@
>
> import org.apache.maven.artifact.Artifact;
> import org.apache.maven.artifact.handler.ArtifactHandler;
> +import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
> import org.apache.maven.artifact.metadata.ArtifactMetadata;
> import org.apache.maven.artifact.repository.ArtifactRepository;
>
> @@ -37,9 +38,12 @@
>
> private static final char ARTIFACT_SEPARATOR = '-';
>
> + /** @plexus.requirement */
> + private ArtifactHandlerManager artifactHandlerManager;
> +
> public String pathOf( Artifact artifact )
> {
> - ArtifactHandler artifactHandler = artifact.getArtifactHandler();
> + ArtifactHandler artifactHandler =
> artifactHandlerManager.getArtifactHandler( artifact.getType() );
>
> StringBuffer path = new StringBuffer();
>
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java
> Sat Feb 2 20:19:44 2008
> @@ -2,9 +2,9 @@
>
> import org.apache.maven.artifact.Artifact;
> import org.apache.maven.artifact.handler.ArtifactHandler;
> +import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
> import org.apache.maven.artifact.metadata.ArtifactMetadata;
> import org.apache.maven.artifact.repository.ArtifactRepository;
> -import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
>
> /**
> * The code in this class is taken from DefaultRepositorylayout, located at:
> @@ -20,9 +20,12 @@
>
> private static final char GROUP_SEPARATOR = '.';
>
> + /** @plexus.requirement */
> + private ArtifactHandlerManager artifactHandlerManager;
> +
> public String pathOf( Artifact artifact )
> {
> - ArtifactHandler artifactHandler = artifact.getArtifactHandler();
> + ArtifactHandler artifactHandler =
> artifactHandlerManager.getArtifactHandler( artifact.getType() );
>
> StringBuffer path = new StringBuffer();
>
>
> Modified:
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/LegacyRepositoryLayout.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/LegacyRepositoryLayout.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/LegacyRepositoryLayout.java
> (original)
> +++
> maven/artifact/trunk/src/main/java/org/apache/maven/artifact/repository/layout/LegacyRepositoryLayout.java
> Sat Feb 2 20:19:44 2008
> @@ -21,6 +21,7 @@
>
> import org.apache.maven.artifact.Artifact;
> import org.apache.maven.artifact.handler.ArtifactHandler;
> +import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
> import org.apache.maven.artifact.metadata.ArtifactMetadata;
> import org.apache.maven.artifact.repository.ArtifactRepository;
>
> @@ -33,9 +34,12 @@
> {
> private static final String PATH_SEPARATOR = "/";
>
> + /** @plexus.requirement */
> + private ArtifactHandlerManager artifactHandlerManager;
> +
> public String pathOf( Artifact artifact )
> {
> - ArtifactHandler artifactHandler = artifact.getArtifactHandler();
> + ArtifactHandler artifactHandler =
> artifactHandlerManager.getArtifactHandler( artifact.getType() );
>
> StringBuffer path = new StringBuffer();
>
>
> Modified:
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java
> (original)
> +++
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/DefaultArtifactTest.java
> Sat Feb 2 20:19:44 2008
> @@ -47,12 +47,11 @@
> throws Exception
> {
> super.setUp();
> - artifactHandler = new ArtifactHandlerMock();
> versionRange = VersionRange.createFromVersion( version );
> - artifact = new DefaultArtifact( groupId, artifactId, versionRange,
> scope, type, classifier, artifactHandler );
> + artifact = new DefaultArtifact( groupId, artifactId, versionRange,
> scope, type, classifier );
>
> snapshotVersionRange = VersionRange.createFromVersion(
> snapshotResolvedVersion );
> - snapshotArtifact = new DefaultArtifact( groupId, artifactId,
> snapshotVersionRange, scope, type, classifier, artifactHandler );
> + snapshotArtifact = new DefaultArtifact( groupId, artifactId,
> snapshotVersionRange, scope, type, classifier );
> }
>
> public void testGetVersionReturnsResolvedVersionOnSnapshot()
> @@ -78,7 +77,7 @@
>
> public void testGetDependencyConflictIdNullClassifier()
> {
> - artifact = new DefaultArtifact( groupId, artifactId, versionRange,
> scope, type, null, artifactHandler );
> + artifact = new DefaultArtifact( groupId, artifactId, versionRange,
> scope, type, null );
> assertEquals( groupId + ":" + artifactId + ":" + type,
> artifact.getDependencyConflictId() );
> }
>
> @@ -102,7 +101,7 @@
>
> public void testToStringNullClassifier()
> {
> - artifact = new DefaultArtifact( groupId, artifactId, versionRange,
> scope, type, null, artifactHandler );
> + artifact = new DefaultArtifact( groupId, artifactId, versionRange,
> scope, type, null );
> assertEquals( groupId + ":" + artifactId + ":" + type + ":" +
> version + ":" + scope, artifact.toString() );
> }
>
>
> Modified:
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java
> URL:
> http://svn.apache.org/viewvc/maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java?rev=617944&r1=617943&r2=617944&view=diff
> ==============================================================================
> ---
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java
> (original)
> +++
> maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java
> Sat Feb 2 20:19:44 2008
> @@ -138,7 +138,7 @@
> {
> tmpFile.deleteOnExit();
> Artifact artifact = new DefaultArtifact( "sample.group",
> "sample-art", VersionRange
> - .createFromVersion( "1.0" ), "artifactScope", "type",
> "classifier", null );
> + .createFromVersion( "1.0" ), "artifactScope", "type",
> "classifier" );
> artifact.setFile( tmpFile );
> ArtifactRepository repo = new DefaultArtifactRepository( "id",
> "noop://url",
> new
> ArtifactRepositoryLayoutStub() );
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
